creation of table having enum columns fails

[MolbioUnige]

At some point between version 4.6.6 and the current version, migrations of table with enum columns fails to generate the correct query.

SQLSTATE[HY000]: General error: 4161 Unknown data type: 'NOT'
Query: CREATE TABLE `documents` (`id` CHAR(36) NOT NULL, `application_id` INTEGER UNSIGNED NOT NULL, `type` NOT NULL,

The table has been defined as such:

$this->table('documents')
            ->addColumn('id', 'uuid', [
                'default' => null,
                'limit' => null,
                'null' => false,
            ])
            ->addPrimaryKey(['id'])
            ->addColumn('application_id', 'integer', [
                'default' => null,
                'limit' => 10,
                'null' => false,
                'signed' => false,
            ])
            ->addColumn('type', 'enum', [
                'default' => null,
                'limit' => null,
                'null' => false,
                'values' => ['BSc','MSc','GRE','CV'],
            ])....

[dereuromark]

If you use the old backend it should still work, apparently, when switching to the internal backend with Cake ORM there might be some legacy support missing now.
Let me take a quick look.

In general, it is safer to use a more agnostic approach (and you should be able to easily upgrade to that) using PHP backed enums and normal varchar strings in DB.

[dereuromark]

Can you check if #896 solves it for you?

[MolbioUnige]

Sorry, it didn't, and yes, I agree, backed enum are much better. This happens to be what I'm working on, changing enums to backed enums. So for me, it doesn't really matter if enum support get's dropped, or not. This said, I'll try to figure out where it fails. I see that some enums tests have been dropped in b068c33

[markstory]

As part of 4.7.0, migrations uses the builtin backend by default which doesn't support enum column types. You can continue using the phinx backend which does support enums by setting Migrations.backend to phinx in your application's configuration.

[MolbioUnige]

Great, so to continue using phinx, migrations must extend AbstractMigration and not BaseMigration or else I get the following error

PHPUnit 10.5.53 by Sebastian Bergmann and contributors.

Error in bootstrap script: TypeError:
Migrations\BaseMigration::__construct(): Argument #1 ($version) must be of type int, string given, called in /application/apply/vendor/robmorgan/phinx/src/Phinx/Migration/Manager.php on line 851
#0 /application/apply/vendor/robmorgan/phinx/src/Phinx/Migration/Manager.php(851): Migrations\BaseMigration->__construct()
#1 /application/apply/vendor/cakephp/migrations/src/CakeManager.php(73): Phinx\Migration\Manager->getMigrations()
#2 [internal function]: Migrations\CakeManager->printStatus()
#3 /application/apply/vendor/cakephp/migrations/src/Migration/PhinxBackend.php(287): call_user_func_array()
#4 /application/apply/vendor/cakephp/migrations/src/Migration/PhinxBackend.php(144): Migrations\Migration\PhinxBackend->run()
#5 /application/apply/vendor/cakephp/migrations/src/Migrations.php(165): Migrations\Migration\PhinxBackend->status()
#6 /application/apply/vendor/cakephp/migrations/src/TestSuite/Migrator.php(179): Migrations\Migrations->status()
#7 /application/apply/vendor/cakephp/migrations/src/TestSuite/Migrator.php(102): Migrations\TestSuite\Migrator->shouldDropTables()
#8 /application/apply/vendor/cakephp/migrations/src/TestSuite/Migrator.php(58): Migrations\TestSuite\Migrator->runMany()
#9 /application/apply/tests/bootstrap.php(49): Migrations\TestSuite\Migrator->run()
#10 /application/apply/vendor/phpunit/phpunit/src/TextUI/Application.php(337): include_once('...')
#11 /application/apply/vendor/phpunit/phpunit/src/TextUI/Application.php(109): PHPUnit\TextUI\Application->loadBootstrapScript()
#12 /application/apply/vendor/phpunit/phpunit/phpunit(104): PHPUnit\TextUI\Application->run()
#13 /application/apply/vendor/bin/phpunit(122): include('...')
#14 {main}

[markstory]

Great, so to continue using phinx, migrations must extend AbstractMigration and not BaseMigration or else I get the following error.

Yes, or you can pin to migrations 4.6 as a solution until your migrations can be updated. I admit these aren't ideal solutions though.

[dereuromark]

What about
#896

Does this shim it to work again?

[MolbioUnige]

@dereuromark sorry, the answer is no. It doesn't fix the generated CREATE sql

[MolbioUnige]

It seems the problem is not in the migrations plugin but in cakephp. Running 5.2 with builtin backend

here below is the value of $column here

migrations/src/Db/Adapter/MysqlAdapter.php

Line 233 in 69732d9

foreach ($columns as $column) {

object(Migrations\Db\Table\Column) id:0 {
  protected name => 'type'
  protected type => 'enum'
  protected limit => null
  protected null => false
  protected default => null
  protected identity => false
  protected generated => 'BY DEFAULT'
  protected seed => null
  protected increment => null
  protected scale => null
  protected after => null
  protected update => null
  protected comment => null
  protected signed => true
  protected timezone => false
  protected properties => []
  protected collation => null
  protected encoding => null
  protected srid => null
  protected values => [
    (int) 0 => 'BSc',
    (int) 1 => 'MSc',
    (int) 2 => 'GRE',
    (int) 3 => 'CV'
  ]
}

here below the content of $columnData at

migrations/src/Db/Adapter/MysqlAdapter.php

Line 234 in 69732d9

$columnData = $this->mapColumnData($column->toArray());

[
  'name' => 'type',
  'type' => 'enum',
  'length' => null,
  'null' => false,
  'default' => null,
  'unsigned' => false,
  'onUpdate' => null,
  'collate' => null,
  'precision' => null,
  'srid' => null,
  'timezone' => false,
  'comment' => null,
  'autoIncrement' => false,
  'values' => [
    (int) 0 => 'BSc',
    (int) 1 => 'MSc',
    (int) 2 => 'GRE',
    (int) 3 => 'CV'
  ]
]

here below the content of $sql at

migrations/src/Db/Adapter/MysqlAdapter.php

Line 235 in 69732d9

$sql .= $dialect->columnDefinitionSql($columnData) . ', ';

'CREATE TABLE `documents` (`id` CHAR(36) NOT NULL, `application_id` INTEGER UNSIGNED NOT NULL, `type` NOT NULL, '

$sql is malformed at the type column, and this is the result of the $dialect->columnDefinitionSql($columnData) function.

In my case, it's https://github.com/cakephp/cakephp/blob/fff9fb8aedf2348631ea08dfce38ff8104114535/src/Database/Schema/MysqlSchemaDialect.php#L608

There, a column with a type = enum get's mapped to nothing and indeed the result of the function is at line 814 the following

'`type` NOT NULL'

[MolbioUnige]

And the problem is not due to my column being named type. It happens with any enum defined columns, whatever their name is

[markstory]

It seems the problem is not in the migrations plugin but in cakephp. Running 5.2 with builtin backend

Yes, CakePHP has never had support for enum columns in either column generation, or reflection. As migrations becomes aligned with CakePHP's database abstraction layer there are some gaps between what was possible with phinx (which had its own separate database abstraction layer) and what is possible with Cakephp's database abstraction layer. Enum column support is one gap that hasn't been addressed.

I'm very reluctant to add support for enum columns as they create long-term maintenance issues and in my experience should be avoided.