New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Compatibility with MySQL Group Replication and MySQL InnoDB Cluster #11885
Conversation
…ve a primary key)
Besides changing the schema, there also needs to be an update script, that performs the changes while updating. |
…INTEGER is simply too small."
Integer was changed to BIGINT. Where would we place the update script? Which file is best as an example? |
See https://github.com/piwik/piwik/tree/3.x-dev/core/Updates |
Update script was created core/Updates/3.1.0.php. Hopefully we could be considered for 3.1.0. |
Tested the update script and it works well. Retested and everything checks out and we are ready for merge. |
core/Db/Schema/Mysql.php
Outdated
@@ -81,6 +81,8 @@ public function getTablesCreateSql() | |||
`setting_name` VARCHAR(255) NOT NULL, | |||
`setting_value` LONGTEXT NOT NULL, | |||
`user_login` VARCHAR(100) NOT NULL DEFAULT '', | |||
`plugin_setting_id` BIGINT UNSIGNED NOT NULL AUTO_INCREMENT, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to make the spacing consistent with the lines above (ie. use spaces not tabs)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, tabs were changed to spaces in Mysql.php.
Ok, tabs were changed to spaces. |
core/Updates/3.2.0.php
Outdated
*/ | ||
private function getPluginSettingsMigrations($queries) | ||
{ | ||
$queries[] = $this->migration->db->addColumn($this->pluginSettingsTable, 'plugin_setting_id', 'BIGINT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT', 'user_login'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Aardvark8 fyi you missed a few tabs here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, tabs were replaced with spaced in 3.2.0.php as well.
Hi @Aardvark8 When trying to run the upgrade it fails because there is already one field with auto increment in
|
Hi Matt, We do not receive that error. Looking at the file, there does not appear to be any other column in that table with AUTO_INCREMENT. Is it possible, that your tester ran the code twice and received the error on the second run (maybe after adding the primary key column by the original name)? |
@mattab merging since it works for me & build passes. Thanks for submitting your first PR @Aardvark8 ! |
On demo2 we're getting the error
so this is not working as expected? cc @diosmosis edit: table schema on demo2
|
The problem is that the update script for creating the |
Looks like this problem should have been fixed by |
…tion and MySQL InnoDB Cluster (matomo-org#11885) * Columns added to support mysql group replication (every table must have a primary key) * changed integer to bigint as "there are issues at other places where INTEGER is simply too small." * Update script created for 3.1.0 to Fix matomo-org#11596 * corrected php syntax and made the columns primary keys on add * updated the db upgrade to reflect the inclusion in version 3.2 instead of 3.1 * changed tabs to spaces * replaced tabs with spaces * Use consistent column names * Use consistent column names * Will be in 3.2.1 * Change Update version + bump version. * Version bump fail.
Columns added to support MySQL group replication (every table must have a primary key)