diff --git a/src/Driver/MySQL/MySQLHandler.php b/src/Driver/MySQL/MySQLHandler.php index f9b1169d..38af4f80 100644 --- a/src/Driver/MySQL/MySQLHandler.php +++ b/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 + * + * @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; } /** diff --git a/src/Driver/MySQL/Schema/MySQLColumn.php b/src/Driver/MySQL/Schema/MySQLColumn.php index cf3e9cd2..04ed1bbf 100644 --- a/src/Driver/MySQL/Schema/MySQLColumn.php +++ b/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']; diff --git a/src/Driver/MySQL/Schema/MySQLTable.php b/src/Driver/MySQL/Schema/MySQLTable.php index 93de8547..5d397224 100644 --- a/src/Driver/MySQL/Schema/MySQLTable.php +++ b/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, ); } diff --git a/tests/Database/Functional/Driver/MySQL/Schema/AutoIncrementColumnTest.php b/tests/Database/Functional/Driver/MySQL/Schema/AutoIncrementColumnTest.php new file mode 100644 index 00000000..e3bc8a5f --- /dev/null +++ b/tests/Database/Functional/Driver/MySQL/Schema/AutoIncrementColumnTest.php @@ -0,0 +1,162 @@ + ['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()); + } +}