Regression: length null for int(2)/int(3) etc

[dereuromark]

This is a (multiple allowed):

• [x] bug

• [ ] enhancement

• [ ] feature-discussion (RFC)

• CakePHP Version: latest

• Migrations plugin version: latest - 3.1.0

• Phinx version: latest - 0.12.7

• Database server (MySQL, SQLite, Postgres): Mysql

• PHP Version: 8

• Platform / OS: Linux

What you did

Rebake fixture:

bin/cake bake fixture users -f

Expected Behavior

'shop_type' => ['type' => 'integer', 'length' => 2, 'unsigned' => false, 'null' => false, 'default' => '0', 'comment' => '', 'precision' => null, 'autoIncrement' => null],

stays the same

Actual Behavior

'length' => null, suddenly, as for other int values (no matter what length: 2, 3, 10, 11).

[markstory]

This could be related to MySQL no longer storing integer lengths and instead only having 'display width' now.

[othercorey]

This could be related to MySQL no longer storing integer lengths and instead only having 'display width' now.

It's the other way. Mysql no longer has display width.

[dereuromark]

Do we need to fix in phinx or in migrations plugin? I am not so concerned about the (10/11) keys and foreign keys, but the (2) enums as shown above.

[dereuromark]

How can we resolve this? If there is indeed no way in MySQL with display width, can we use a different way to avoid the regressions? They are now affecting all systems, baked code, validation rules, migration snippets, ... How about using comment in a formal way here as fallback?

E.g.: If length is null and comment has length:int;... we can use the length integer value here as fallback.

[othercorey]

What does the number represent for migrations?

[dereuromark]

We lose the "semantic type", so bool/enum is not possible anymore, as shown above, also some other type specific meanings are lost.

[dereuromark]

So both bake and migrations (fixtures) need it to make proper generated files from DB. What do you think? Can we try to go into that formalized comment field here as fallback solution for Mysql?

[markstory]

Using a comment could work. What would we do if the column already has a comment?

[dereuromark]

Ignore anything that doesnt fit the very narrow regex IMO.

#\b(length|...):\d+;#

So no BC issue for anyone who is using them differently.

[markstory]

That could work, making it MySQL specific inside bake/migrations is going to be fun :smile:

[dereuromark]

Would https://github.com/cakephp/phinx/pull/2174 enable this?

[dereuromark]

We now need to actually reactivate this ASAP And also include this directly into CakeORM

Anyone who is upgrading from Mysql 5 to 8.0.19+ will run into this as a very serious regression/issue. The length as such will make boolean (tinyint1) not possible anymore. And all queries that use e.g.

$this->getAlias() . '.expires' => false/true

would have to be

$this->getAlias() . '.expires' => 0/1

to continue working in Mysql 8 it seems.

If all tinyint/int/... fields get a length as comment meta data, we can re-add those in the ORM schema layer and therefore shim this back to the behavior that is expected and needed within the framework.

[dereuromark]

Also note that Cake ORM Schema does not see the unsigned part in Mysql 5 for boolean type. It is always the same for both when reading:

[
  'type' => 'boolean',
  'length' => null,
  'null' => false,
  'default' => '0',
  'comment' => '',
  'precision' => null
]

So any detection and fix of this might have to be done through pure DB schema reading without Cake interpretation.