From d01babe48fdee6434a912888564eeac7b28129fc Mon Sep 17 00:00:00 2001 From: mscherer Date: Mon, 10 Nov 2025 17:55:17 +0100 Subject: [PATCH 1/3] Add ALGORITHM and LOCK support for MySQL ALTER TABLE operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements support for MySQL's ALGORITHM and LOCK clauses in ALTER TABLE operations, enabling zero-downtime schema migrations for compatible operations. Key additions: - Class constants for ALGORITHM (DEFAULT, INSTANT, INPLACE, COPY) and LOCK (DEFAULT, NONE, SHARED, EXCLUSIVE) options to avoid magic strings - Column class now supports algorithm and lock options via setAlgorithm()/setLock() - MysqlAdapter validates and applies algorithm/lock clauses to ALTER operations - Batched operations detect conflicts and throw clear error messages - Comprehensive test coverage (11 new test cases) Benefits: - Near-zero downtime migrations on large tables with ALGORITHM=INSTANT - Production-friendly migrations with explicit locking control - Improved performance for compatible schema changes on MySQL 8.0+/MariaDB 10.3+ Usage: ```php use Migrations\Db\Adapter\MysqlAdapter; $table->addColumn('status', 'string', [ 'null' => true, 'algorithm' => MysqlAdapter::ALGORITHM_INSTANT, 'lock' => MysqlAdapter::LOCK_NONE, ])->update(); ``` Closes #2323 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/Db/Adapter/MysqlAdapter.php | 369 +++++++++++++++++- src/Db/Table/Column.php | 58 +++ .../TestCase/Db/Adapter/MysqlAdapterTest.php | 171 ++++++++ 3 files changed, 597 insertions(+), 1 deletion(-) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index ca79798f..338828ab 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -14,6 +14,18 @@ use Cake\Database\Schema\SchemaDialect; use Cake\Database\Schema\TableSchema; use InvalidArgumentException; +use Migrations\Db\Action\AddColumn; +use Migrations\Db\Action\AddForeignKey; +use Migrations\Db\Action\AddIndex; +use Migrations\Db\Action\ChangeColumn; +use Migrations\Db\Action\ChangeComment; +use Migrations\Db\Action\ChangePrimaryKey; +use Migrations\Db\Action\DropForeignKey; +use Migrations\Db\Action\DropIndex; +use Migrations\Db\Action\DropTable; +use Migrations\Db\Action\RemoveColumn; +use Migrations\Db\Action\RenameColumn; +use Migrations\Db\Action\RenameTable; use Migrations\Db\AlterInstructions; use Migrations\Db\Table\CheckConstraint; use Migrations\Db\Table\Column; @@ -107,6 +119,65 @@ class MysqlAdapter extends AbstractAdapter public const FIRST = 'FIRST'; + /** + * MySQL ALTER TABLE ALGORITHM options + * + * These constants control how MySQL performs ALTER TABLE operations: + * - ALGORITHM_DEFAULT: Let MySQL choose the best algorithm + * - ALGORITHM_INSTANT: Instant operation (no table copy, MySQL 8.0+ / MariaDB 10.3+) + * - ALGORITHM_INPLACE: In-place operation (no full table copy) + * - ALGORITHM_COPY: Traditional table copy algorithm + * + * Usage: + * ```php + * use Migrations\Db\Adapter\MysqlAdapter; + * + * $table->addColumn('status', 'string', [ + * 'algorithm' => MysqlAdapter::ALGORITHM_INSTANT, + * 'lock' => MysqlAdapter::LOCK_NONE, + * ]); + * ``` + * + * Note: ALGORITHM_INSTANT requires MySQL 8.0+ or MariaDB 10.3+ and only works for + * compatible operations (adding nullable columns, dropping columns, etc.). + * If the operation cannot be performed instantly, MySQL will return an error. + * + * @see https://dev.mysql.com/doc/refman/8.0/en/alter-table.html + * @see https://mariadb.com/kb/en/alter-table/#algorithm + */ + public const ALGORITHM_DEFAULT = 'DEFAULT'; + public const ALGORITHM_INSTANT = 'INSTANT'; + public const ALGORITHM_INPLACE = 'INPLACE'; + public const ALGORITHM_COPY = 'COPY'; + + /** + * MySQL ALTER TABLE LOCK options + * + * These constants control the locking behavior during ALTER TABLE operations: + * - LOCK_DEFAULT: Let MySQL choose the appropriate lock level + * - LOCK_NONE: Allow concurrent reads and writes (least restrictive) + * - LOCK_SHARED: Allow concurrent reads, block writes + * - LOCK_EXCLUSIVE: Block all concurrent access (most restrictive) + * + * Usage: + * ```php + * use Migrations\Db\Adapter\MysqlAdapter; + * + * $table->changeColumn('name', 'string', [ + * 'limit' => 500, + * 'algorithm' => MysqlAdapter::ALGORITHM_INPLACE, + * 'lock' => MysqlAdapter::LOCK_NONE, + * ]); + * ``` + * + * @see https://dev.mysql.com/doc/refman/8.0/en/alter-table.html + * @see https://mariadb.com/kb/en/alter-table/#lock + */ + public const LOCK_DEFAULT = 'DEFAULT'; + public const LOCK_NONE = 'NONE'; + public const LOCK_SHARED = 'SHARED'; + public const LOCK_EXCLUSIVE = 'EXCLUSIVE'; + /** * @inheritDoc */ @@ -576,6 +647,7 @@ protected function getAddColumnInstructions(TableMetadata $table, Column $column ); $alter .= $this->afterClause($column); + $alter .= $this->algorithmClause($column); return new AlterInstructions([$alter]); } @@ -600,6 +672,62 @@ protected function afterClause(Column $column): string return ' AFTER ' . $this->quoteColumnName($after); } + /** + * Generates the ALGORITHM and LOCK clauses for MySQL ALTER TABLE statements. + * + * @param \Migrations\Db\Table\Column $column The column being altered. + * @return string The appropriate SQL fragment. + * @throws \InvalidArgumentException If an invalid algorithm or lock value is provided. + */ + protected function algorithmClause(Column $column): string + { + $clause = ''; + $algorithm = $column->getAlgorithm(); + $lock = $column->getLock(); + + if ($algorithm !== null) { + $validAlgorithms = [ + self::ALGORITHM_DEFAULT, + self::ALGORITHM_INSTANT, + self::ALGORITHM_INPLACE, + self::ALGORITHM_COPY, + ]; + $upperAlgorithm = strtoupper($algorithm); + + if (!in_array($upperAlgorithm, $validAlgorithms, true)) { + throw new InvalidArgumentException(sprintf( + 'Invalid algorithm "%s". Valid options: %s', + $algorithm, + implode(', ', $validAlgorithms), + )); + } + + $clause .= ', ALGORITHM=' . $upperAlgorithm; + } + + if ($lock !== null) { + $validLocks = [ + self::LOCK_DEFAULT, + self::LOCK_NONE, + self::LOCK_SHARED, + self::LOCK_EXCLUSIVE, + ]; + $upperLock = strtoupper($lock); + + if (!in_array($upperLock, $validLocks, true)) { + throw new InvalidArgumentException(sprintf( + 'Invalid lock "%s". Valid options: %s', + $lock, + implode(', ', $validLocks), + )); + } + + $clause .= ', LOCK=' . $upperLock; + } + + return $clause; + } + /** * {@inheritDoc} * @@ -671,10 +799,11 @@ protected function getChangeColumnInstructions(string $tableName, string $column $dialect = $this->getSchemaDialect(); $alter = sprintf( - 'CHANGE %s %s%s', + 'CHANGE %s %s%s%s', $this->quoteColumnName($columnName), $this->columnDefinitionSql($dialect, $newColumn), $this->afterClause($newColumn), + $this->algorithmClause($newColumn), ); return new AlterInstructions([$alter]); @@ -1164,4 +1293,242 @@ protected function isMariaDb(): bool return stripos($version, 'mariadb') !== false; } + + /** + * {@inheritDoc} + * + * Overridden to support ALGORITHM and LOCK clauses for MySQL ALTER TABLE operations. + * + * @throws \InvalidArgumentException + * @return void + */ + public function executeActions(TableMetadata $table, array $actions): void + { + // Extract algorithm and lock specifications from all actions + $algorithm = null; + $lock = null; + + foreach ($actions as $action) { + if (!method_exists($action, 'getColumn')) { + continue; + } + + $column = $action->getColumn(); + if (!($column instanceof Column)) { + continue; + } + + $colAlgorithm = $column->getAlgorithm(); + $colLock = $column->getLock(); + + if ($colAlgorithm !== null) { + if ($algorithm !== null && $algorithm !== $colAlgorithm) { + throw new InvalidArgumentException(sprintf( + 'Conflicting algorithm specifications in batched operations: "%s" and "%s". ' . + 'All operations in a batch must use the same algorithm, or specify it on only one operation.', + $algorithm, + $colAlgorithm, + )); + } + $algorithm = $colAlgorithm; + } + + if ($colLock !== null) { + if ($lock !== null && $lock !== $colLock) { + throw new InvalidArgumentException(sprintf( + 'Conflicting lock specifications in batched operations: "%s" and "%s". ' . + 'All operations in a batch must use the same lock mode, or specify it on only one operation.', + $lock, + $colLock, + )); + } + $lock = $colLock; + } + } + + // If no algorithm/lock specified, use parent implementation + if ($algorithm === null && $lock === null) { + parent::executeActions($table, $actions); + + return; + } + + // Otherwise, execute with custom algorithm/lock support + $this->executeActionsWithAlgorithmAndLock($table, $actions, $algorithm, $lock); + } + + /** + * Executes actions with ALGORITHM and LOCK clauses. + * + * @param \Migrations\Db\Table\TableMetadata $table The table metadata + * @param array $actions The actions to execute + * @param string|null $algorithm The algorithm to use + * @param string|null $lock The lock mode to use + * @throws \InvalidArgumentException + * @return void + */ + protected function executeActionsWithAlgorithmAndLock( + TableMetadata $table, + array $actions, + ?string $algorithm, + ?string $lock, + ): void { + $instructions = new AlterInstructions(); + + // Build instructions (copied from AbstractAdapter::executeActions) + foreach ($actions as $action) { + switch (true) { + case $action instanceof AddColumn: + $instructions->merge($this->getAddColumnInstructions($table, $action->getColumn())); + break; + + case $action instanceof AddIndex: + $instructions->merge($this->getAddIndexInstructions($table, $action->getIndex())); + break; + + case $action instanceof AddForeignKey: + $instructions->merge($this->getAddForeignKeyInstructions($table, $action->getForeignKey())); + break; + + case $action instanceof ChangeColumn: + $instructions->merge($this->getChangeColumnInstructions( + $table->getName(), + $action->getColumnName(), + $action->getColumn(), + )); + break; + + case $action instanceof DropForeignKey && !$action->getForeignKey()->getName(): + $instructions->merge($this->getDropForeignKeyByColumnsInstructions( + $table->getName(), + $action->getForeignKey()->getColumns(), + )); + break; + + case $action instanceof DropForeignKey && $action->getForeignKey()->getName(): + $instructions->merge($this->getDropForeignKeyInstructions( + $table->getName(), + (string)$action->getForeignKey()->getName(), + )); + break; + + case $action instanceof DropIndex && $action->getIndex()->getName(): + $instructions->merge($this->getDropIndexByNameInstructions( + $table->getName(), + (string)$action->getIndex()->getName(), + )); + break; + + case $action instanceof DropIndex && !$action->getIndex()->getName(): + $instructions->merge($this->getDropIndexByColumnsInstructions( + $table->getName(), + (array)$action->getIndex()->getColumns(), + )); + break; + + case $action instanceof DropTable: + $instructions->merge($this->getDropTableInstructions($table->getName())); + break; + + case $action instanceof RemoveColumn: + $instructions->merge($this->getDropColumnInstructions( + $table->getName(), + (string)$action->getColumn()->getName(), + )); + break; + + case $action instanceof RenameColumn: + $instructions->merge($this->getRenameColumnInstructions( + $table->getName(), + (string)$action->getColumn()->getName(), + $action->getNewName(), + )); + break; + + case $action instanceof RenameTable: + $instructions->merge($this->getRenameTableInstructions( + $table->getName(), + $action->getNewName(), + )); + break; + + case $action instanceof ChangePrimaryKey: + $instructions->merge($this->getChangePrimaryKeyInstructions( + $table, + $action->getNewColumns(), + )); + break; + + case $action instanceof ChangeComment: + $instructions->merge($this->getChangeCommentInstructions( + $table, + $action->getNewComment(), + )); + break; + + default: + throw new InvalidArgumentException( + sprintf("Don't know how to execute action `%s`", get_class($action)), + ); + } + } + + // Build ALTER TABLE template with algorithm and lock + $alterTemplate = sprintf('ALTER TABLE %s %%s', $this->quoteTableName($table->getName())); + + // Add algorithm and lock clauses + $algorithmLockClause = ''; + if ($algorithm !== null) { + $upperAlgorithm = strtoupper($algorithm); + $validAlgorithms = [ + self::ALGORITHM_DEFAULT, + self::ALGORITHM_INSTANT, + self::ALGORITHM_INPLACE, + self::ALGORITHM_COPY, + ]; + if (!in_array($upperAlgorithm, $validAlgorithms, true)) { + throw new InvalidArgumentException(sprintf( + 'Invalid algorithm "%s". Valid options: %s', + $algorithm, + implode(', ', $validAlgorithms), + )); + } + $algorithmLockClause .= ', ALGORITHM=' . $upperAlgorithm; + } + + if ($lock !== null) { + $upperLock = strtoupper($lock); + $validLocks = [ + self::LOCK_DEFAULT, + self::LOCK_NONE, + self::LOCK_SHARED, + self::LOCK_EXCLUSIVE, + ]; + if (!in_array($upperLock, $validLocks, true)) { + throw new InvalidArgumentException(sprintf( + 'Invalid lock "%s". Valid options: %s', + $lock, + implode(', ', $validLocks), + )); + } + $algorithmLockClause .= ', LOCK=' . $upperLock; + } + + // Execute with custom template + if ($instructions->getAlterParts()) { + $alter = sprintf($alterTemplate, implode(', ', $instructions->getAlterParts()) . $algorithmLockClause); + $this->execute($alter); + } + + // Execute post-steps + $state = []; + foreach ($instructions->getPostSteps() as $instruction) { + if (is_callable($instruction)) { + $state = $instruction($state); + continue; + } + + $this->execute($instruction); + } + } } diff --git a/src/Db/Table/Column.php b/src/Db/Table/Column.php index 7d8733fe..73aaaf02 100644 --- a/src/Db/Table/Column.php +++ b/src/Db/Table/Column.php @@ -87,6 +87,16 @@ class Column extends DatabaseColumn */ protected ?array $values = null; + /** + * @var string|null + */ + protected ?string $algorithm = null; + + /** + * @var string|null + */ + protected ?string $lock = null; + /** * Column constructor * @@ -650,6 +660,52 @@ public function getEncoding(): ?string return $this->encoding; } + /** + * Sets the ALTER TABLE algorithm (MySQL-specific). + * + * @param string $algorithm Algorithm + * @return $this + */ + public function setAlgorithm(string $algorithm) + { + $this->algorithm = $algorithm; + + return $this; + } + + /** + * Gets the ALTER TABLE algorithm. + * + * @return string|null + */ + public function getAlgorithm(): ?string + { + return $this->algorithm; + } + + /** + * Sets the ALTER TABLE lock mode (MySQL-specific). + * + * @param string $lock Lock mode + * @return $this + */ + public function setLock(string $lock) + { + $this->lock = $lock; + + return $this; + } + + /** + * Gets the ALTER TABLE lock mode. + * + * @return string|null + */ + public function getLock(): ?string + { + return $this->lock; + } + /** * Gets all allowed options. Each option must have a corresponding `setFoo` method. * @@ -677,6 +733,8 @@ protected function getValidOptions(): array 'seed', 'increment', 'generated', + 'algorithm', + 'lock', ]; } diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 27f68869..2c15b4fb 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -2539,4 +2539,175 @@ public function testInsertOrSkipWithoutDuplicates() $rows = $this->adapter->fetchAll('SELECT * FROM categories'); $this->assertCount(2, $rows); } + + public function testAddColumnWithAlgorithmInstant() + { + $table = new Table('users', [], $this->adapter); + $table->addColumn('email', 'string') + ->create(); + + $table->addColumn('status', 'string', [ + 'null' => true, + 'algorithm' => MysqlAdapter::ALGORITHM_INSTANT, + ])->update(); + + $this->assertTrue($this->adapter->hasColumn('users', 'status')); + } + + public function testAddColumnWithAlgorithmAndLock() + { + $table = new Table('products', [], $this->adapter); + $table->addColumn('name', 'string') + ->create(); + + $table->addColumn('price', 'decimal', [ + 'precision' => 10, + 'scale' => 2, + 'null' => true, + 'algorithm' => MysqlAdapter::ALGORITHM_INSTANT, + 'lock' => MysqlAdapter::LOCK_NONE, + ])->update(); + + $this->assertTrue($this->adapter->hasColumn('products', 'price')); + } + + public function testChangeColumnWithAlgorithm() + { + $table = new Table('items', [], $this->adapter); + $table->addColumn('description', 'string', ['limit' => 100]) + ->create(); + + $table->changeColumn('description', 'string', [ + 'limit' => 255, + 'algorithm' => MysqlAdapter::ALGORITHM_INPLACE, + 'lock' => MysqlAdapter::LOCK_SHARED, + ])->update(); + + $columns = $this->adapter->getColumns('items'); + foreach ($columns as $column) { + if ($column->getName() === 'description') { + $this->assertEquals(255, $column->getLimit()); + } + } + } + + public function testBatchedOperationsWithSameAlgorithm() + { + $table = new Table('batch_test', [], $this->adapter); + $table->addColumn('col1', 'string') + ->create(); + + $table->addColumn('col2', 'string', [ + 'null' => true, + 'algorithm' => MysqlAdapter::ALGORITHM_INSTANT, + ]) + ->addColumn('col3', 'string', [ + 'null' => true, + 'algorithm' => MysqlAdapter::ALGORITHM_INSTANT, + ]) + ->update(); + + $this->assertTrue($this->adapter->hasColumn('batch_test', 'col2')); + $this->assertTrue($this->adapter->hasColumn('batch_test', 'col3')); + } + + public function testBatchedOperationsWithConflictingAlgorithmsThrowsException() + { + $table = new Table('conflict_test', [], $this->adapter); + $table->addColumn('col1', 'string') + ->create(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Conflicting algorithm specifications'); + + $table->addColumn('col2', 'string', [ + 'null' => true, + 'algorithm' => MysqlAdapter::ALGORITHM_INSTANT, + ]) + ->addColumn('col3', 'string', [ + 'null' => true, + 'algorithm' => MysqlAdapter::ALGORITHM_COPY, + ]) + ->update(); + } + + public function testBatchedOperationsWithConflictingLocksThrowsException() + { + $table = new Table('lock_conflict_test', [], $this->adapter); + $table->addColumn('col1', 'string') + ->create(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Conflicting lock specifications'); + + $table->addColumn('col2', 'string', [ + 'null' => true, + 'lock' => MysqlAdapter::LOCK_NONE, + ]) + ->addColumn('col3', 'string', [ + 'null' => true, + 'lock' => MysqlAdapter::LOCK_SHARED, + ]) + ->update(); + } + + public function testInvalidAlgorithmThrowsException() + { + $table = new Table('invalid_algo', [], $this->adapter); + $table->addColumn('col1', 'string') + ->create(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid algorithm'); + + $table->addColumn('col2', 'string', [ + 'algorithm' => 'INVALID', + ])->update(); + } + + public function testInvalidLockThrowsException() + { + $table = new Table('invalid_lock', [], $this->adapter); + $table->addColumn('col1', 'string') + ->create(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid lock'); + + $table->addColumn('col2', 'string', [ + 'lock' => 'INVALID', + ])->update(); + } + + public function testAlgorithmConstantsAreDefined() + { + $this->assertEquals('DEFAULT', MysqlAdapter::ALGORITHM_DEFAULT); + $this->assertEquals('INSTANT', MysqlAdapter::ALGORITHM_INSTANT); + $this->assertEquals('INPLACE', MysqlAdapter::ALGORITHM_INPLACE); + $this->assertEquals('COPY', MysqlAdapter::ALGORITHM_COPY); + } + + public function testLockConstantsAreDefined() + { + $this->assertEquals('DEFAULT', MysqlAdapter::LOCK_DEFAULT); + $this->assertEquals('NONE', MysqlAdapter::LOCK_NONE); + $this->assertEquals('SHARED', MysqlAdapter::LOCK_SHARED); + $this->assertEquals('EXCLUSIVE', MysqlAdapter::LOCK_EXCLUSIVE); + } + + public function testAlgorithmWithMixedCase() + { + $table = new Table('mixed_case', [], $this->adapter); + $table->addColumn('col1', 'string') + ->create(); + + // Should work with lowercase + $table->addColumn('col2', 'string', [ + 'null' => true, + 'algorithm' => 'instant', + 'lock' => 'none', + ])->update(); + + $this->assertTrue($this->adapter->hasColumn('mixed_case', 'col2')); + } } From b4163a4c0229d94896fbf043a019864948cac8cb Mon Sep 17 00:00:00 2001 From: mscherer Date: Mon, 10 Nov 2025 18:04:34 +0100 Subject: [PATCH 2/3] Fix ALGORITHM/LOCK implementation issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove duplicate algorithm/lock clauses from individual column instructions - Add MySQL restriction validation: ALGORITHM=INSTANT cannot be combined with explicit LOCK modes (LOCK=NONE, LOCK=SHARED, LOCK=EXCLUSIVE) - Update tests to use ALGORITHM=INPLACE with explicit LOCK values instead - Add test for MySQL restriction validation - Remove unused algorithmClause() method - Update documentation to clarify MySQL restrictions Fixes duplicate clause issue where algorithm/lock was being added twice: once in getAddColumnInstructions() and once in executeActionsWithAlgorithmAndLock(). The algorithm/lock clauses should only be applied at the ALTER TABLE statement level, not at individual column instruction level. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/Db/Adapter/MysqlAdapter.php | 84 ++++++------------- .../TestCase/Db/Adapter/MysqlAdapterTest.php | 25 +++++- 2 files changed, 47 insertions(+), 62 deletions(-) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 338828ab..8aa67034 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -132,17 +132,29 @@ class MysqlAdapter extends AbstractAdapter * ```php * use Migrations\Db\Adapter\MysqlAdapter; * + * // ALGORITHM=INSTANT alone (recommended) * $table->addColumn('status', 'string', [ + * 'null' => true, * 'algorithm' => MysqlAdapter::ALGORITHM_INSTANT, + * ]); + * + * // Or with ALGORITHM=INPLACE and explicit LOCK + * $table->addColumn('status', 'string', [ + * 'algorithm' => MysqlAdapter::ALGORITHM_INPLACE, * 'lock' => MysqlAdapter::LOCK_NONE, * ]); * ``` * + * Important: ALGORITHM=INSTANT cannot be combined with LOCK=NONE, LOCK=SHARED, + * or LOCK=EXCLUSIVE (MySQL restriction). Use ALGORITHM=INSTANT alone or with + * LOCK=DEFAULT only. + * * Note: ALGORITHM_INSTANT requires MySQL 8.0+ or MariaDB 10.3+ and only works for * compatible operations (adding nullable columns, dropping columns, etc.). * If the operation cannot be performed instantly, MySQL will return an error. * * @see https://dev.mysql.com/doc/refman/8.0/en/alter-table.html + * @see https://dev.mysql.com/doc/refman/8.0/en/innodb-online-ddl-operations.html * @see https://mariadb.com/kb/en/alter-table/#algorithm */ public const ALGORITHM_DEFAULT = 'DEFAULT'; @@ -647,7 +659,6 @@ protected function getAddColumnInstructions(TableMetadata $table, Column $column ); $alter .= $this->afterClause($column); - $alter .= $this->algorithmClause($column); return new AlterInstructions([$alter]); } @@ -672,62 +683,6 @@ protected function afterClause(Column $column): string return ' AFTER ' . $this->quoteColumnName($after); } - /** - * Generates the ALGORITHM and LOCK clauses for MySQL ALTER TABLE statements. - * - * @param \Migrations\Db\Table\Column $column The column being altered. - * @return string The appropriate SQL fragment. - * @throws \InvalidArgumentException If an invalid algorithm or lock value is provided. - */ - protected function algorithmClause(Column $column): string - { - $clause = ''; - $algorithm = $column->getAlgorithm(); - $lock = $column->getLock(); - - if ($algorithm !== null) { - $validAlgorithms = [ - self::ALGORITHM_DEFAULT, - self::ALGORITHM_INSTANT, - self::ALGORITHM_INPLACE, - self::ALGORITHM_COPY, - ]; - $upperAlgorithm = strtoupper($algorithm); - - if (!in_array($upperAlgorithm, $validAlgorithms, true)) { - throw new InvalidArgumentException(sprintf( - 'Invalid algorithm "%s". Valid options: %s', - $algorithm, - implode(', ', $validAlgorithms), - )); - } - - $clause .= ', ALGORITHM=' . $upperAlgorithm; - } - - if ($lock !== null) { - $validLocks = [ - self::LOCK_DEFAULT, - self::LOCK_NONE, - self::LOCK_SHARED, - self::LOCK_EXCLUSIVE, - ]; - $upperLock = strtoupper($lock); - - if (!in_array($upperLock, $validLocks, true)) { - throw new InvalidArgumentException(sprintf( - 'Invalid lock "%s". Valid options: %s', - $lock, - implode(', ', $validLocks), - )); - } - - $clause .= ', LOCK=' . $upperLock; - } - - return $clause; - } - /** * {@inheritDoc} * @@ -799,11 +754,10 @@ protected function getChangeColumnInstructions(string $tableName, string $column $dialect = $this->getSchemaDialect(); $alter = sprintf( - 'CHANGE %s %s%s%s', + 'CHANGE %s %s%s', $this->quoteColumnName($columnName), $this->columnDefinitionSql($dialect, $newColumn), $this->afterClause($newColumn), - $this->algorithmClause($newColumn), ); return new AlterInstructions([$alter]); @@ -1478,6 +1432,9 @@ protected function executeActionsWithAlgorithmAndLock( // Add algorithm and lock clauses $algorithmLockClause = ''; + $upperAlgorithm = null; + $upperLock = null; + if ($algorithm !== null) { $upperAlgorithm = strtoupper($algorithm); $validAlgorithms = [ @@ -1514,6 +1471,15 @@ protected function executeActionsWithAlgorithmAndLock( $algorithmLockClause .= ', LOCK=' . $upperLock; } + // MySQL restriction: ALGORITHM=INSTANT cannot be combined with explicit LOCK modes + // except LOCK=DEFAULT. See: https://dev.mysql.com/doc/refman/8.0/en/innodb-online-ddl-operations.html + if ($upperAlgorithm === self::ALGORITHM_INSTANT && $upperLock !== null && $upperLock !== self::LOCK_DEFAULT) { + throw new InvalidArgumentException( + 'ALGORITHM=INSTANT cannot be combined with LOCK=NONE, LOCK=SHARED, or LOCK=EXCLUSIVE. ' . + 'Either use ALGORITHM=INSTANT alone, or use ALGORITHM=INSTANT with LOCK=DEFAULT.', + ); + } + // Execute with custom template if ($instructions->getAlterParts()) { $alter = sprintf($alterTemplate, implode(', ', $instructions->getAlterParts()) . $algorithmLockClause); diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 2c15b4fb..8019ac8d 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -2560,11 +2560,12 @@ public function testAddColumnWithAlgorithmAndLock() $table->addColumn('name', 'string') ->create(); + // Use ALGORITHM=INPLACE with LOCK=NONE (INSTANT can't have explicit locks) $table->addColumn('price', 'decimal', [ 'precision' => 10, 'scale' => 2, 'null' => true, - 'algorithm' => MysqlAdapter::ALGORITHM_INSTANT, + 'algorithm' => MysqlAdapter::ALGORITHM_INPLACE, 'lock' => MysqlAdapter::LOCK_NONE, ])->update(); @@ -2642,10 +2643,12 @@ public function testBatchedOperationsWithConflictingLocksThrowsException() $table->addColumn('col2', 'string', [ 'null' => true, + 'algorithm' => MysqlAdapter::ALGORITHM_INPLACE, 'lock' => MysqlAdapter::LOCK_NONE, ]) ->addColumn('col3', 'string', [ 'null' => true, + 'algorithm' => MysqlAdapter::ALGORITHM_INPLACE, 'lock' => MysqlAdapter::LOCK_SHARED, ]) ->update(); @@ -2679,6 +2682,22 @@ public function testInvalidLockThrowsException() ])->update(); } + public function testAlgorithmInstantWithExplicitLockThrowsException() + { + $table = new Table('instant_lock_test', [], $this->adapter); + $table->addColumn('col1', 'string') + ->create(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('ALGORITHM=INSTANT cannot be combined with LOCK=NONE'); + + $table->addColumn('col2', 'string', [ + 'null' => true, + 'algorithm' => MysqlAdapter::ALGORITHM_INSTANT, + 'lock' => MysqlAdapter::LOCK_NONE, + ])->update(); + } + public function testAlgorithmConstantsAreDefined() { $this->assertEquals('DEFAULT', MysqlAdapter::ALGORITHM_DEFAULT); @@ -2701,10 +2720,10 @@ public function testAlgorithmWithMixedCase() $table->addColumn('col1', 'string') ->create(); - // Should work with lowercase + // Should work with lowercase (use INPLACE with LOCK, not INSTANT) $table->addColumn('col2', 'string', [ 'null' => true, - 'algorithm' => 'instant', + 'algorithm' => 'inplace', 'lock' => 'none', ])->update(); From b027d17c6190ff95c701d9a25cb753d45047c3a9 Mon Sep 17 00:00:00 2001 From: mscherer Date: Wed, 19 Nov 2025 22:46:46 +0100 Subject: [PATCH 3/3] Adjust as per feedback. --- src/Db/Adapter/MysqlAdapter.php | 216 +++++--------------------------- src/Db/AlterInstructions.php | 78 ++++++++++++ 2 files changed, 107 insertions(+), 187 deletions(-) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 8aa67034..3e147677 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -14,18 +14,6 @@ use Cake\Database\Schema\SchemaDialect; use Cake\Database\Schema\TableSchema; use InvalidArgumentException; -use Migrations\Db\Action\AddColumn; -use Migrations\Db\Action\AddForeignKey; -use Migrations\Db\Action\AddIndex; -use Migrations\Db\Action\ChangeColumn; -use Migrations\Db\Action\ChangeComment; -use Migrations\Db\Action\ChangePrimaryKey; -use Migrations\Db\Action\DropForeignKey; -use Migrations\Db\Action\DropIndex; -use Migrations\Db\Action\DropTable; -use Migrations\Db\Action\RemoveColumn; -use Migrations\Db\Action\RenameColumn; -use Migrations\Db\Action\RenameTable; use Migrations\Db\AlterInstructions; use Migrations\Db\Table\CheckConstraint; use Migrations\Db\Table\Column; @@ -660,7 +648,16 @@ protected function getAddColumnInstructions(TableMetadata $table, Column $column $alter .= $this->afterClause($column); - return new AlterInstructions([$alter]); + $instructions = new AlterInstructions([$alter]); + + if ($column->getAlgorithm() !== null) { + $instructions->setAlgorithm($column->getAlgorithm()); + } + if ($column->getLock() !== null) { + $instructions->setLock($column->getLock()); + } + + return $instructions; } /** @@ -760,7 +757,16 @@ protected function getChangeColumnInstructions(string $tableName, string $column $this->afterClause($newColumn), ); - return new AlterInstructions([$alter]); + $instructions = new AlterInstructions([$alter]); + + if ($newColumn->getAlgorithm() !== null) { + $instructions->setAlgorithm($newColumn->getAlgorithm()); + } + if ($newColumn->getLock() !== null) { + $instructions->setLock($newColumn->getLock()); + } + + return $instructions; } /** @@ -1251,186 +1257,24 @@ protected function isMariaDb(): bool /** * {@inheritDoc} * - * Overridden to support ALGORITHM and LOCK clauses for MySQL ALTER TABLE operations. + * Overridden to support ALGORITHM and LOCK clauses from AlterInstructions. * + * @param string $tableName The table name + * @param \Migrations\Db\AlterInstructions $instructions The alter instructions * @throws \InvalidArgumentException * @return void */ - public function executeActions(TableMetadata $table, array $actions): void + protected function executeAlterSteps(string $tableName, AlterInstructions $instructions): void { - // Extract algorithm and lock specifications from all actions - $algorithm = null; - $lock = null; - - foreach ($actions as $action) { - if (!method_exists($action, 'getColumn')) { - continue; - } - - $column = $action->getColumn(); - if (!($column instanceof Column)) { - continue; - } - - $colAlgorithm = $column->getAlgorithm(); - $colLock = $column->getLock(); - - if ($colAlgorithm !== null) { - if ($algorithm !== null && $algorithm !== $colAlgorithm) { - throw new InvalidArgumentException(sprintf( - 'Conflicting algorithm specifications in batched operations: "%s" and "%s". ' . - 'All operations in a batch must use the same algorithm, or specify it on only one operation.', - $algorithm, - $colAlgorithm, - )); - } - $algorithm = $colAlgorithm; - } - - if ($colLock !== null) { - if ($lock !== null && $lock !== $colLock) { - throw new InvalidArgumentException(sprintf( - 'Conflicting lock specifications in batched operations: "%s" and "%s". ' . - 'All operations in a batch must use the same lock mode, or specify it on only one operation.', - $lock, - $colLock, - )); - } - $lock = $colLock; - } - } + $algorithm = $instructions->getAlgorithm(); + $lock = $instructions->getLock(); - // If no algorithm/lock specified, use parent implementation if ($algorithm === null && $lock === null) { - parent::executeActions($table, $actions); + parent::executeAlterSteps($tableName, $instructions); return; } - // Otherwise, execute with custom algorithm/lock support - $this->executeActionsWithAlgorithmAndLock($table, $actions, $algorithm, $lock); - } - - /** - * Executes actions with ALGORITHM and LOCK clauses. - * - * @param \Migrations\Db\Table\TableMetadata $table The table metadata - * @param array $actions The actions to execute - * @param string|null $algorithm The algorithm to use - * @param string|null $lock The lock mode to use - * @throws \InvalidArgumentException - * @return void - */ - protected function executeActionsWithAlgorithmAndLock( - TableMetadata $table, - array $actions, - ?string $algorithm, - ?string $lock, - ): void { - $instructions = new AlterInstructions(); - - // Build instructions (copied from AbstractAdapter::executeActions) - foreach ($actions as $action) { - switch (true) { - case $action instanceof AddColumn: - $instructions->merge($this->getAddColumnInstructions($table, $action->getColumn())); - break; - - case $action instanceof AddIndex: - $instructions->merge($this->getAddIndexInstructions($table, $action->getIndex())); - break; - - case $action instanceof AddForeignKey: - $instructions->merge($this->getAddForeignKeyInstructions($table, $action->getForeignKey())); - break; - - case $action instanceof ChangeColumn: - $instructions->merge($this->getChangeColumnInstructions( - $table->getName(), - $action->getColumnName(), - $action->getColumn(), - )); - break; - - case $action instanceof DropForeignKey && !$action->getForeignKey()->getName(): - $instructions->merge($this->getDropForeignKeyByColumnsInstructions( - $table->getName(), - $action->getForeignKey()->getColumns(), - )); - break; - - case $action instanceof DropForeignKey && $action->getForeignKey()->getName(): - $instructions->merge($this->getDropForeignKeyInstructions( - $table->getName(), - (string)$action->getForeignKey()->getName(), - )); - break; - - case $action instanceof DropIndex && $action->getIndex()->getName(): - $instructions->merge($this->getDropIndexByNameInstructions( - $table->getName(), - (string)$action->getIndex()->getName(), - )); - break; - - case $action instanceof DropIndex && !$action->getIndex()->getName(): - $instructions->merge($this->getDropIndexByColumnsInstructions( - $table->getName(), - (array)$action->getIndex()->getColumns(), - )); - break; - - case $action instanceof DropTable: - $instructions->merge($this->getDropTableInstructions($table->getName())); - break; - - case $action instanceof RemoveColumn: - $instructions->merge($this->getDropColumnInstructions( - $table->getName(), - (string)$action->getColumn()->getName(), - )); - break; - - case $action instanceof RenameColumn: - $instructions->merge($this->getRenameColumnInstructions( - $table->getName(), - (string)$action->getColumn()->getName(), - $action->getNewName(), - )); - break; - - case $action instanceof RenameTable: - $instructions->merge($this->getRenameTableInstructions( - $table->getName(), - $action->getNewName(), - )); - break; - - case $action instanceof ChangePrimaryKey: - $instructions->merge($this->getChangePrimaryKeyInstructions( - $table, - $action->getNewColumns(), - )); - break; - - case $action instanceof ChangeComment: - $instructions->merge($this->getChangeCommentInstructions( - $table, - $action->getNewComment(), - )); - break; - - default: - throw new InvalidArgumentException( - sprintf("Don't know how to execute action `%s`", get_class($action)), - ); - } - } - - // Build ALTER TABLE template with algorithm and lock - $alterTemplate = sprintf('ALTER TABLE %s %%s', $this->quoteTableName($table->getName())); - - // Add algorithm and lock clauses $algorithmLockClause = ''; $upperAlgorithm = null; $upperLock = null; @@ -1471,8 +1315,6 @@ protected function executeActionsWithAlgorithmAndLock( $algorithmLockClause .= ', LOCK=' . $upperLock; } - // MySQL restriction: ALGORITHM=INSTANT cannot be combined with explicit LOCK modes - // except LOCK=DEFAULT. See: https://dev.mysql.com/doc/refman/8.0/en/innodb-online-ddl-operations.html if ($upperAlgorithm === self::ALGORITHM_INSTANT && $upperLock !== null && $upperLock !== self::LOCK_DEFAULT) { throw new InvalidArgumentException( 'ALGORITHM=INSTANT cannot be combined with LOCK=NONE, LOCK=SHARED, or LOCK=EXCLUSIVE. ' . @@ -1480,13 +1322,13 @@ protected function executeActionsWithAlgorithmAndLock( ); } - // Execute with custom template + $alterTemplate = sprintf('ALTER TABLE %s %%s', $this->quoteTableName($tableName)); + if ($instructions->getAlterParts()) { $alter = sprintf($alterTemplate, implode(', ', $instructions->getAlterParts()) . $algorithmLockClause); $this->execute($alter); } - // Execute post-steps $state = []; foreach ($instructions->getPostSteps() as $instruction) { if (is_callable($instruction)) { diff --git a/src/Db/AlterInstructions.php b/src/Db/AlterInstructions.php index 9202ad7b..39a20d1f 100644 --- a/src/Db/AlterInstructions.php +++ b/src/Db/AlterInstructions.php @@ -8,6 +8,8 @@ namespace Migrations\Db; +use InvalidArgumentException; + /** * Contains all the information for running an ALTER command for a table, * and any post-steps required after the fact. @@ -24,6 +26,16 @@ class AlterInstructions */ protected array $postSteps = []; + /** + * @var string|null MySQL-specific: ALGORITHM clause + */ + protected ?string $algorithm = null; + + /** + * @var string|null MySQL-specific: LOCK clause + */ + protected ?string $lock = null; + /** * Constructor * @@ -87,12 +99,78 @@ public function getPostSteps(): array * Merges another AlterInstructions object to this one * * @param \Migrations\Db\AlterInstructions $other The other collection of instructions to merge in + * @throws \InvalidArgumentException When algorithm or lock specifications conflict * @return void */ public function merge(AlterInstructions $other): void { $this->alterParts = array_merge($this->alterParts, $other->getAlterParts()); $this->postSteps = array_merge($this->postSteps, $other->getPostSteps()); + + if ($other->getAlgorithm() !== null) { + if ($this->algorithm !== null && $this->algorithm !== $other->getAlgorithm()) { + throw new InvalidArgumentException(sprintf( + 'Conflicting algorithm specifications in batched operations: "%s" and "%s". ' . + 'All operations in a batch must use the same algorithm, or specify it on only one operation.', + $this->algorithm, + $other->getAlgorithm(), + )); + } + $this->algorithm = $other->getAlgorithm(); + } + if ($other->getLock() !== null) { + if ($this->lock !== null && $this->lock !== $other->getLock()) { + throw new InvalidArgumentException(sprintf( + 'Conflicting lock specifications in batched operations: "%s" and "%s". ' . + 'All operations in a batch must use the same lock mode, or specify it on only one operation.', + $this->lock, + $other->getLock(), + )); + } + $this->lock = $other->getLock(); + } + } + + /** + * Sets the ALGORITHM clause (MySQL-specific) + * + * @param string|null $algorithm The algorithm to use + * @return void + */ + public function setAlgorithm(?string $algorithm): void + { + $this->algorithm = $algorithm; + } + + /** + * Gets the ALGORITHM clause (MySQL-specific) + * + * @return string|null + */ + public function getAlgorithm(): ?string + { + return $this->algorithm; + } + + /** + * Sets the LOCK clause (MySQL-specific) + * + * @param string|null $lock The lock mode to use + * @return void + */ + public function setLock(?string $lock): void + { + $this->lock = $lock; + } + + /** + * Gets the LOCK clause (MySQL-specific) + * + * @return string|null + */ + public function getLock(): ?string + { + return $this->lock; } /**