yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.24k stars 6.9k forks source link

MySQL integer pk overflow vector #11541

Closed kathysledge closed 8 years ago

kathysledge commented 8 years ago

yii\db\mysql\QueryBuilder

public $typeMap = [
    Schema::TYPE_PK => 'int(11) NOT NULL AUTO_INCREMENT PRIMARY KEY',
    Schema::TYPE_UPK => 'int(11) UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY',
];

Max value of int column is 4,294,967,295 (unsigned) (http://dev.mysql.com/doc/refman/5.7/en/integer-types.html).

That's only 10 digits, so the type map should be changed to int(10).

cebe commented 8 years ago

https://github.com/yiisoft/yii2/pull/12072#issuecomment-237392712

rob006 commented 8 years ago

@cebe

mysql> CREATE TABLE `m1197_testing`.`inttest` ( `a1` INT NOT NULL , `a2` INT UNSIGNED NOT NULL ) ENGINE = InnoDB;
Query OK, 0 rows affected (0,04 sec)

mysql> DESCRIBE inttest;
+-------+------------------+------+-----+---------+-------+
| Field | Type             | Null | Key | Default | Extra |
+-------+------------------+------+-----+---------+-------+
| a1    | int(11)          | NO   |     | NULL    |       |
| a2    | int(10) unsigned | NO   |     | NULL    |       |
+-------+------------------+------+-----+---------+-------+
2 rows in set (0,01 sec)

MySQL 5.6

cebe commented 8 years ago

@rob006 thanks for checking! fixed.

cebe commented 8 years ago

that's actually a breaking change.

cebe commented 8 years ago

reverted the change: 144d78e

I'm setting this to 2.1 now but I am unsure if we should change anything as it is a subtle break that has no real value imo.

rob006 commented 8 years ago

that's actually a breaking change.

Why?

cebe commented 8 years ago

see https://github.com/yiisoft/yii2/commit/85d89e489311e20c626bcb90129c0c86978fcee4#commitcomment-18556122 it will break foreign keys pointing to the pk.

rob006 commented 8 years ago

@cebe Did you test this? I have no problems with creating FK between INT(10) UNSIGNED and INT(11) UNSIGNED.

cebe commented 8 years ago

according to @samdark it fails. Which MySQL version are you using?

rob006 commented 8 years ago

Tested on MySQL 5.6.

samdark commented 8 years ago

Hmm... seems I was wrong and it's indeed possible. Singned/unsigned should match but MySQL is OK w/ referencing int(10) PK from int(11) FK.

samdark commented 8 years ago

Sorry for confusion.

nkovacs commented 8 years ago

Hmm... seems I was wrong and it's indeed possible. Singned/unsigned should match but MySQL is OK w/ referencing int(10) PK from int(11) FK.

That's because the display width does not limit the range of values that can be stored in the field: http://dev.mysql.com/doc/refman/5.7/en/numeric-type-attributes.html So it's effectively the same type. The display width is only metadata that tells the mysql client to pad it with spaces so that numbers display nicely under each other. On the other hand, unsigned changes the type, it can no longer store values larger than 2^31-1, so if you had a signed int referencing an unsigned int, it would be possible to have ids that you cannot store in the foreign key.

samdark commented 8 years ago

Yes, I'm aware of the fact. Just remember that some incompatibilities were not tolerated and was sure it was display width as well.