Make unique index compatible with database without long key support #18723
Conversation
sgiehl
added
not-in-changelog
|
This pull request has been mentioned on Matomo forums. There might be relevant details there: https://forum.matomo.org/t/4-7-0-b2-update-fails-with-mysql-5-5/44603/10 |
core/Db/Schema/Mysql.php
Outdated
| @@ -362,14 +362,14 @@ public function getTablesCreateSql() | |||
| 'changes' => "CREATE TABLE `{$prefixTables}changes` ( | |||
| `idchange` INTEGER UNSIGNED NOT NULL AUTO_INCREMENT, | |||
| `created_time` DATETIME NOT NULL, | |||
| `plugin_name` VARCHAR(255) NOT NULL, | |||
| `plugin_name` VARCHAR(40) NOT NULL, | |||
There was a problem hiding this comment.
@sgiehl seems in other tables we have a 60 character limit in the DB like plugin_setting, site_setting. Does it make sense to apple this here as well? We could create a follow up issue for Matomo 5 (to not break anything) to adjust isValidPluginName to limit to max 60 characters. Checked our longest plugin name so far is around 30 characters.
There was a problem hiding this comment.
the developer docs say the max length should be 40. But we can also use 60. No preference there.
There was a problem hiding this comment.
OK we can leave it at 40. I will create a follow up issue then to change the other columns to 40 as well.
There was a problem hiding this comment.
or maybe let's use 60 @sgiehl for consistency with the other tables. I'm sure it be fine to use 40, but better have consistency for now. I created follow up issue for Matomo 5 to decide on a number.
There was a problem hiding this comment.
I've updated the plugin_name length to 60
core/Db/Schema/Mysql.php
Outdated
| `version` VARCHAR(20) NOT NULL, | ||
| `title` VARCHAR(255) NOT NULL, | ||
| `description` TEXT NULL, | ||
| `link_name` VARCHAR(255) NULL, | ||
| `link` VARCHAR(255) NULL, | ||
| PRIMARY KEY(`idchange`), | ||
| UNIQUE KEY unique_plugin_version_title (`plugin_name`, `version`, `title`) | ||
| UNIQUE KEY unique_plugin_version_title (`plugin_name`, `version`, `title`(131)) |
There was a problem hiding this comment.
Should we make this like 100 then? For a title it's already very long.
There was a problem hiding this comment.
If we extend the plugin name to 60, guess we can also lower the title to 100
There was a problem hiding this comment.
I've updated the title index limit to 100
|
This pull request has been mentioned on Matomo forums. There might be relevant details there: |
Description:
Not sure if that is the most elegant solution. But can't think of something better right now.
fixes #18721
Review