fix(MySQL): keep auto-increment columns out of the primary key

src/Driver/MySQL/MySQLHandler.php

@@ -64,6 +64,19 @@ public function eraseTable(AbstractTable $table): void
         );
     }
 
+    #[\Override]
+    public function createTable(AbstractTable $table): void
+    {
+        $this->run($this->createStatement($table));
+
+        $inline = $this->autoIncrementIndexes($table);
+        foreach ($table->getIndexes() as $index) {
+            if (!\in_array($index, $inline, true)) {
+                $this->createIndex($table, $index);
+            }
+        }
+    }
+
     public function alterColumn(
         AbstractTable $table,
         AbstractColumn $initial,
@@ -130,7 +143,57 @@ protected function createStatement(AbstractTable $table): string
     {
         $table instanceof MySQLTable or throw new SchemaException('MySQLHandler can process only MySQL tables');
 
-        return parent::createStatement($table) . " ENGINE {$table->getEngine()}";
+        $statement = parent::createStatement($table);
+
+        // MySQL rejects an AUTO_INCREMENT column without a key, so an index created after the table is too late.
+        $indexes = \array_map(
+            fn(AbstractIndex $index): string => $index->sqlStatement($this->getDriver(), false),
+            $this->autoIncrementIndexes($table),
+        );
+        if ($indexes !== []) {
+            // The parent statement ends with "\n)".
+            $statement = \substr($statement, 0, -2) . ",\n    " . \implode(",\n    ", $indexes) . "\n)";
+        }
+
+        return $statement . " ENGINE {$table->getEngine()}";
+    }
+
+    /**
+     * Indexes covering AUTO_INCREMENT columns that do not lead the primary key.
+     *
+     * @return list<AbstractIndex>
+     *
+     * @throws SchemaException When such a column is neither in the primary key nor in an index.
+     */
+    protected function autoIncrementIndexes(AbstractTable $table): array
+    {
+        $primaryKeys = $table->getPrimaryKeys();
+
+        $result = [];
+        foreach ($table->getColumns() as $column) {
+            if (
+                ($column->getAttributes()['autoIncrement'] ?? false) !== true
+                || ($primaryKeys[0] ?? null) === $column->getName()
+            ) {
+                continue;
+            }
+
+            $found = \in_array($column->getName(), $primaryKeys, true);
+            foreach ($table->getIndexes() as $index) {
+                if (\in_array($column->getName(), $index->getColumns(), true)) {
+                    $found = true;
+                    \in_array($index, $result, true) or $result[] = $index;
+                }
+            }
+
+            $found or throw new SchemaException(\sprintf(
+                'AUTO_INCREMENT column `%s` of table `%s` must be in the primary key or in an index',
+                $column->getName(),
+                $table->getFullName(),
+            ));
+        }
+
+        return $result;
     }
 
     /**

src/Driver/MySQL/Schema/MySQLColumn.php

@@ -42,7 +42,7 @@ class MySQLColumn extends AbstractColumn
      */
     public const DATETIME_NOW = 'CURRENT_TIMESTAMP';
 
-    public const EXCLUDE_FROM_COMPARE = ['size', 'timezone', 'userType', 'attributes', 'first', 'after', 'unknownSize', 'charset', 'collation'];
+    public const EXCLUDE_FROM_COMPARE = ['size', 'timezone', 'userType', 'attributes', 'first', 'after', 'unknownSize', 'charset', 'collation', 'isPrimary'];
     protected const INTEGER_TYPES = ['tinyint', 'smallint', 'mediumint', 'int', 'bigint'];
     protected const STRING_TYPES = ['varchar', 'char', 'text', 'tinytext', 'mediumtext', 'longtext', 'enum', 'set'];
 
@@ -53,18 +53,21 @@ class MySQLColumn extends AbstractColumn
             'size'          => 11,
             'autoIncrement' => true,
             'nullable'      => false,
+            'isPrimary'     => true,
         ],
         'smallPrimary'  => [
             'type'          => 'smallint',
             'size'          => 6,
             'autoIncrement' => true,
             'nullable'      => false,
+            'isPrimary'     => true,
         ],
         'bigPrimary'  => [
             'type'          => 'bigint',
             'size'          => 20,
             'autoIncrement' => true,
             'nullable'      => false,
+            'isPrimary'     => true,
         ],
 
         //Enum type (mapped via method)
@@ -118,8 +121,9 @@ class MySQLColumn extends AbstractColumn
         'uuid'        => ['type' => 'varchar', 'size' => 36],
     ];
     protected array $reverseMapping = [
-        'primary'     => [['type' => 'int', 'autoIncrement' => true]],
-        'bigPrimary'  => ['serial', ['type' => 'bigint', 'size' => 20, 'autoIncrement' => true]],
+        'primary'     => [['type' => 'int', 'autoIncrement' => true, 'isPrimary' => true]],
+        'smallPrimary' => [['type' => 'smallint', 'autoIncrement' => true, 'isPrimary' => true]],
+        'bigPrimary'  => ['serial', ['type' => 'bigint', 'size' => 20, 'autoIncrement' => true, 'isPrimary' => true]],
         'enum'        => ['enum'],
         'set'         => ['set'],
         'boolean'     => ['bool', 'boolean', ['type' => 'tinyint', 'size' => 1]],
@@ -218,11 +222,23 @@ class MySQLColumn extends AbstractColumn
     protected bool $first = false;
 
     /**
+     * Internal field to determine if the auto-increment column is PK.
+     */
+    protected bool $isPrimary = false;
+
+    /**
+     * @param array $primaryKeys Primary key columns of the table.
+     *
      * @psalm-param non-empty-string $table
      */
-    public static function createInstance(string $table, array $schema, ?\DateTimeZone $timezone = null): self
-    {
+    public static function createInstance(
+        string $table,
+        array $schema,
+        ?\DateTimeZone $timezone = null,
+        array $primaryKeys = [],
+    ): self {
         $column = new self($table, $schema['Field'], $timezone);
+        $column->isPrimary = \in_array($schema['Field'], $primaryKeys, true);
 
         $column->type = $schema['Type'];
         $column->comment = $schema['Comment'];

src/Driver/MySQL/Schema/MySQLTable.php

@@ -106,12 +106,15 @@ protected function fetchColumns(): array
     {
         $query = "SHOW FULL COLUMNS FROM {$this->driver->identifier($this->getFullName())}";
 
+        $primaryKeys = $this->fetchPrimaryKeys();
+
         $result = [];
         foreach ($this->driver->query($query) as $schema) {
             $result[] = MySQLColumn::createInstance(
                 $this->getFullName(),
                 $schema,
                 $this->driver->getTimezone(),
+                $primaryKeys,
             );
         }
 

tests/Database/Functional/Driver/MySQL/Schema/AutoIncrementColumnTest.php

@@ -0,0 +1,162 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Cycle\Database\Tests\Functional\Driver\MySQL\Schema;
+
+// phpcs:ignore
+use Cycle\Database\Exception\SchemaException;
+use Cycle\Database\Tests\Functional\Driver\Common\BaseTest;
+
+/**
+ * @group driver
+ * @group driver-mysql
+ */
+final class AutoIncrementColumnTest extends BaseTest
+{
+    public const DRIVER = 'mysql';
+
+    public static function primaryTypesProvider(): iterable
+    {
+        yield 'primary' => ['primary', 'primary'];
+        yield 'smallPrimary' => ['smallPrimary', 'smallPrimary'];
+        yield 'bigPrimary' => ['bigPrimary', 'bigPrimary'];
+    }
+
+    public function testAutoIncrementColumnIsNotAddedToPrimaryKey(): void
+    {
+        $schema = $this->schema('auto_increment');
+        $schema->string('id', 36)->nullable(false);
+        $schema->integer('number', autoIncrement: true);
+        $schema->bigInteger('big_number', autoIncrement: true);
+        $schema->setPrimaryKeys(['id']);
+
+        $this->assertSame(['id'], $schema->getPrimaryKeys());
+        $this->assertSame('integer', $schema->column('number')->getAbstractType());
+        $this->assertSame('bigInteger', $schema->column('big_number')->getAbstractType());
+    }
+
+    public function testCreateTableWithAutoIncrementColumnOutsidePrimaryKey(): void
+    {
+        $schema = $this->schema('auto_increment');
+        $schema->string('id', 36)->nullable(false);
+        $schema->integer('number', autoIncrement: true)->nullable(false);
+        $schema->string('title')->nullable(true);
+        $schema->setPrimaryKeys(['id']);
+        $schema->index(['number'])->unique();
+        $schema->save();
+
+        $this->assertSameAsInDB($schema);
+
+        $saved = $this->schema('auto_increment');
+        $this->assertSame(['id'], $saved->getPrimaryKeys());
+        $this->assertSame('integer', $saved->column('number')->getAbstractType());
+        $this->assertTrue($saved->hasIndex(['number']));
+
+        $table = $this->database->table('auto_increment');
+        $table->insertOne(['id' => 'a', 'title' => 'first']);
+        $table->insertOne(['id' => 'b', 'title' => 'second']);
+        $this->assertSame(
+            [['id' => 'a', 'number' => 1], ['id' => 'b', 'number' => 2]],
+            $table->select('id', 'number')->orderBy('id')->fetchAll(),
+        );
+    }
+
+    public function testCreateTableWithNonUniqueIndexOnAutoIncrementColumn(): void
+    {
+        $schema = $this->schema('auto_increment');
+        $schema->string('id', 36)->nullable(false);
+        $schema->bigInteger('number', autoIncrement: true)->nullable(false);
+        $schema->setPrimaryKeys(['id']);
+        $schema->index(['number']);
+        $schema->save();
+
+        $this->assertSameAsInDB($schema);
+        $this->assertSame(['id'], $this->schema('auto_increment')->getPrimaryKeys());
+    }
+
+    public function testCreateTableWithAutoIncrementColumnNotFirstInPrimaryKey(): void
+    {
+        $schema = $this->schema('auto_increment');
+        $schema->string('id', 36)->nullable(false);
+        $schema->integer('number', autoIncrement: true)->nullable(false);
+        $schema->setPrimaryKeys(['id', 'number']);
+        $schema->index(['number'])->unique();
+        $schema->save();
+
+        $this->assertSameAsInDB($schema);
+        $this->assertSame(['id', 'number'], $this->schema('auto_increment')->getPrimaryKeys());
+    }
+
+    public function testAutoIncrementColumnWithoutKeyThrowsException(): void
+    {
+        $schema = $this->schema('auto_increment');
+        $schema->string('id', 36)->nullable(false);
+        $schema->integer('number', autoIncrement: true)->nullable(false);
+        $schema->setPrimaryKeys(['id']);
+
+        $this->expectException(SchemaException::class);
+        $this->expectExceptionMessage('`number`');
+
+        $schema->save();
+    }
+
+    public function testExistingTableIsReflectedWithItsPrimaryKey(): void
+    {
+        $this->database->execute(
+            'CREATE TABLE `auto_increment` (
+                `id` varchar(36) NOT NULL,
+                `number` int NOT NULL AUTO_INCREMENT,
+                PRIMARY KEY (`id`),
+                UNIQUE KEY `auto_increment_number` (`number`)
+            )',
+        );
+
+        $schema = $this->schema('auto_increment');
+        $this->assertSame(['id'], $schema->getPrimaryKeys());
+        $this->assertSame('integer', $schema->column('number')->getAbstractType());
+
+        $schema->string('id', 36)->nullable(false);
+        $schema->integer('number', autoIncrement: true)->nullable(false);
+        $schema->index(['number'])->unique()->setName('auto_increment_number');
+        $schema->setPrimaryKeys(['id']);
+
+        $this->assertFalse($schema->getComparator()->hasChanges());
+    }
+
+    public function testPrimaryKeyListedExplicitlyIsReflectedWithoutChanges(): void
+    {
+        $schema = $this->schema('auto_increment');
+        $schema->string('id', 36)->nullable(false);
+        $schema->integer('number', autoIncrement: true)->nullable(false);
+        $schema->setPrimaryKeys(['number', 'id']);
+        $schema->save();
+
+        $this->assertSameAsInDB($schema);
+        $this->assertSame(['number', 'id'], $this->schema('auto_increment')->getPrimaryKeys());
+
+        $schema = $this->schema('auto_increment');
+        $schema->string('id', 36)->nullable(false);
+        $schema->integer('number', autoIncrement: true)->nullable(false);
+        $schema->setPrimaryKeys(['number', 'id']);
+
+        $this->assertFalse($schema->getComparator()->hasChanges());
+    }
+
+    /**
+     * @dataProvider primaryTypesProvider
+     */
+    public function testPrimaryTypesStayInPrimaryKey(string $type, string $abstractType): void
+    {
+        $schema = $this->schema('auto_increment');
+        $schema->$type('id');
+        $schema->string('title');
+        $schema->save();
+
+        $this->assertSameAsInDB($schema);
+
+        $saved = $this->schema('auto_increment');
+        $this->assertSame(['id'], $saved->getPrimaryKeys());
+        $this->assertSame($abstractType, $saved->column('id')->getAbstractType());
+    }
+}