Skip to content
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

Change default Type from int(11) to int #10418

Closed
2 tasks
emteknetnz opened this issue Jul 21, 2022 · 3 comments
Closed
2 tasks

Change default Type from int(11) to int #10418

emteknetnz opened this issue Jul 21, 2022 · 3 comments

Comments

@emteknetnz
Copy link
Member

emteknetnz commented Jul 21, 2022

The (11) in int(11) is only used to denote display length, not the stored length - https://stackoverflow.com/a/27519793. It's used for things that probably aren't used a whole lots like ZEROFILL. Stored length is altered by using different types such as smallint

MySQL 8.0.17 deprecated ZEROFILL - https://dev.mysql.com/doc/refman/8.0/en/numeric-type-attributes.html

The default width for int is 11 - https://dev.mysql.com/doc/refman/8.0/en/integer-types.html - minimum value is -2147483648 which is 10 characters + 1 character for the negative sign.

Read more about numeric-type syntax - https://dev.mysql.com/doc/refman/8.0/en/numeric-type-syntax.html

framework 4.11 defaults to int(11), though it is persisted differently in MySQL 5.7 and MySQL 8

describe Page;

MySQL 5.7

+----------+---------+------+-----+---------+----------------+
| Field    | Type    | Null | Key | Default | Extra          |
+----------+---------+------+-----+---------+----------------+
| ID       | int(11) | NO   | PRI | NULL    | auto_increment |
| MyFileID | int(11) | NO   | MUL | 0       |                |
+----------+---------+------+-----+---------+----------------+

MySQL 8

+----------+------+------+-----+---------+----------------+
| Field    | Type | Null | Key | Default | Extra          |
+----------+------+------+-----+---------+----------------+
| ID       | int  | NO   | PRI | NULL    | auto_increment |
| MyFileID | int  | NO   | MUL | 0       |                |
+----------+------+------+-----+---------+----------------+

Notably, when running dev/build?flush in mysql 8, no matter how many times we do it, we get the following for every ID column in every table:
+ Field ErrorPage.ID: changed to int(11) not null auto_increment (from int not null auto_increment)

It's also likely what' causing this unit test to fail only in MySQL 8
1) SilverStripe\ORM\Tests\DataObjectSchemaGenerationTest::testFieldsDontRerequestChanges Failed asserting that true is false. /home/runner/work/silverstripe-framework/silverstripe-framework/tests/php/ORM/DataObjectSchemaGenerationTest.php:88 /home/runner/work/silverstripe-framework/silverstripe-framework/src/ORM/Connect/DBSchemaManager.php:164 /home/runner/work/silverstripe-framework/silverstripe-framework/tests/php/ORM/DataObjectSchemaGenerationTest.php:89 /home/runner/work/silverstripe-framework/silverstripe-framework/vendor/bin/phpunit:123
https://github.com/silverstripe/silverstripe-framework/runs/7441905051?check_suite_focus=true#step:12:113

Change the default the default for int columns from int(11) to int. While this should be a safe - we should probably release make this update in either 4.12 or 5.0

ACs

  • 4.10 branch is merged up in 4.11
  • 4.10 build hacks is removed

PRs

@maxime-rainville
Copy link
Contributor

This was already fixed in 4.11 by #9453

It's probably not worth backporting to 4.10.

@GuySartorelli
Copy link
Member

Closing - all actions are complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants