Skip to content

make default PKs for mysql unsigned by default #1899

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

Merged
merged 1 commit into from
Oct 23, 2020
Merged

make default PKs for mysql unsigned by default #1899

merged 1 commit into from
Oct 23, 2020

Conversation

MasterOdin
Copy link
Member

closes #1780

This changes it so that the default PK for a table (either by not specifying one at all, or just setting the id option for the table) is unsigned by default, instead of signed. This gives users extra space to store IDs in their databases, and is generally considered the "right" definition to have for an auto incrementing column for mysql, with some amount of other ORM / DB stuff out there doing this as well.

The principle downside is that foreign key columns will have to be specified with "signed" => false now, but I think is balanced out by the upsides of doing this.

Signed-off-by: Matthew Peveler <matt.peveler@gmail.com>
@othercorey
Copy link
Member

The principle downside is that foreign key columns will have to be specified with "signed" => false now, but I think is balanced out by the upsides of doing this.

This seems like a significant change. Why don't users who want an unsigned range specify it instead of adding this dependency? The default INT range is very large.

@dereuromark
Copy link
Member

Is there any downside of this?
This seems like the right way to approach this for most keys, if not explicitly configured otherwise.

@dereuromark dereuromark merged commit 616d104 into cakephp:0.next Oct 23, 2020
@MasterOdin MasterOdin deleted the unsigned_pk_mysql branch October 23, 2020 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants