From 9fc53c00c160bd0deefee9496e60db804584d071 Mon Sep 17 00:00:00 2001 From: Adam Dyson Date: Fri, 27 Jun 2025 20:01:57 +1000 Subject: [PATCH 01/14] Define new interface methods isFirst, getAfter and explicitly define getComment rather than use annotations --- src/ColumnInterface.php | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/ColumnInterface.php b/src/ColumnInterface.php index 1fbe8284..0a375b06 100644 --- a/src/ColumnInterface.php +++ b/src/ColumnInterface.php @@ -13,9 +13,6 @@ /** * Represents table schema column abstraction. - * - * @method string getComment() Get column comment. - * An empty string will be returned if the feature is not supported by the driver. */ interface ColumnInterface { @@ -80,4 +77,22 @@ public function hasDefaultValue(): bool; * Get column default value, value must be automatically converted to appropriate internal type. */ public function getDefaultValue(): mixed; + + /** + * Whether the column should be positioned first. Always returning false + * if the driver does not support this feature. + */ + public function isFirst(): bool; + + /** + * Retrieve the column name to position after. Always returning an empty string + * if the driver does not support this feature. + */ + public function getAfter(): string; + + /** + * Retrieve the column comment. Always returning an empty string + * if the driver does not support this feature. + */ + public function getComment(): string; } From 4482943b43d86c40e7396186e4a189ecfd48499a Mon Sep 17 00:00:00 2001 From: Adam Dyson Date: Fri, 27 Jun 2025 20:07:49 +1000 Subject: [PATCH 02/14] Add AbstractColumn stubs for methods first, isFirst, after, getAfter --- src/Schema/AbstractColumn.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Schema/AbstractColumn.php b/src/Schema/AbstractColumn.php index f83ceb79..365af6d8 100644 --- a/src/Schema/AbstractColumn.php +++ b/src/Schema/AbstractColumn.php @@ -623,6 +623,26 @@ public function isReadonlySchema(): bool return $this->getAttributes()['readonlySchema'] ?? false; } + public function first(bool $first = true): self + { + return $this; + } + + public function isFirst(): bool + { + return false; + } + + public function after(string $column): self + { + return $this; + } + + public function getAfter(): string + { + return ''; + } + /** * Get column comment. * An empty string will be returned if the feature is not supported by the driver. From 46f48f3c778ec805f3efd82d16f069f6b9c50aac Mon Sep 17 00:00:00 2001 From: Adam Dyson Date: Fri, 27 Jun 2025 20:09:05 +1000 Subject: [PATCH 03/14] Implement MySQLColumn methods first, isFirst, after, getAfter. The only driver supporting column positioning --- src/Driver/MySQL/Schema/MySQLColumn.php | 36 ++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/src/Driver/MySQL/Schema/MySQLColumn.php b/src/Driver/MySQL/Schema/MySQLColumn.php index f9a3b084..8ab4263e 100644 --- a/src/Driver/MySQL/Schema/MySQLColumn.php +++ b/src/Driver/MySQL/Schema/MySQLColumn.php @@ -41,7 +41,7 @@ class MySQLColumn extends AbstractColumn */ public const DATETIME_NOW = 'CURRENT_TIMESTAMP'; - public const EXCLUDE_FROM_COMPARE = ['size', 'timezone', 'userType', 'attributes']; + public const EXCLUDE_FROM_COMPARE = ['size', 'timezone', 'userType', 'attributes', 'first', 'after']; protected const INTEGER_TYPES = ['tinyint', 'smallint', 'mediumint', 'int', 'bigint']; protected array $mapping = [ @@ -184,6 +184,16 @@ class MySQLColumn extends AbstractColumn #[ColumnAttribute] protected string $comment = ''; + /** + * Column name to position after. + */ + protected string $after = ''; + + /** + * Whether the column should be positioned first. + */ + protected bool $first = false; + /** * @psalm-param non-empty-string $table */ @@ -325,6 +335,30 @@ public function isZerofill(): bool return $this->zerofill; } + public function first(bool $first = true): self + { + $this->first = $first; + + return $this; + } + + public function isFirst(): bool + { + return $this->first; + } + + public function after(string $column): self + { + $this->after = $column; + + return $this; + } + + public function getAfter(): string + { + return $this->after; + } + public function set(string|array $values): self { $this->type('set'); From cc7498dc536e682242f670f1bdc7c8b809a76585 Mon Sep 17 00:00:00 2001 From: Adam Dyson Date: Fri, 27 Jun 2025 20:11:19 +1000 Subject: [PATCH 04/14] Implement column positioning within Handler, allowing added columns to define being positioned first or after another column --- src/Driver/Handler.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Driver/Handler.php b/src/Driver/Handler.php index 20fb751d..e35e2087 100644 --- a/src/Driver/Handler.php +++ b/src/Driver/Handler.php @@ -89,9 +89,15 @@ public function renameTable(string $table, string $name): void public function createColumn(AbstractTable $table, AbstractColumn $column): void { - $this->run( - "ALTER TABLE {$this->identify($table)} ADD COLUMN {$column->sqlStatement($this->driver)}", - ); + $statement = "ALTER TABLE {$this->identify($table)} ADD COLUMN {$column->sqlStatement($this->driver)}"; + + if ($column->isFirst()) { + $statement = "{$statement} FIRST"; + } elseif ($column->getAfter() !== '') { + $statement = "{$statement} AFTER {$this->driver->identifier($column->getAfter())}"; + } + + $this->run($statement); } public function dropColumn(AbstractTable $table, AbstractColumn $column): void From fee8ed79a13952d6e6aa0a38a1dbd21bd9805b05 Mon Sep 17 00:00:00 2001 From: Adam Dyson Date: Fri, 27 Jun 2025 20:37:39 +1000 Subject: [PATCH 05/14] When adding a column after another, ensure the column already exists before executing --- src/Driver/Handler.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Driver/Handler.php b/src/Driver/Handler.php index e35e2087..b319ee23 100644 --- a/src/Driver/Handler.php +++ b/src/Driver/Handler.php @@ -94,6 +94,12 @@ public function createColumn(AbstractTable $table, AbstractColumn $column): void if ($column->isFirst()) { $statement = "{$statement} FIRST"; } elseif ($column->getAfter() !== '') { + $columnNames = \array_map(static fn(AbstractColumn $column) => $column->getName(), $table->getColumns()); + + if (! \in_array($column->getAfter(), $columnNames)) { + throw new DBALException("Column `{$column->getAfter()}` does not exist for positioning after"); + } + $statement = "{$statement} AFTER {$this->driver->identifier($column->getAfter())}"; } From a727677cb403b9a559eb8ccea9ee5de197fb3391 Mon Sep 17 00:00:00 2001 From: Adam Dyson Date: Fri, 27 Jun 2025 20:38:17 +1000 Subject: [PATCH 06/14] Add base test class for testing column positioning --- .../Common/Schema/PositionColumnTest.php | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 tests/Database/Functional/Driver/Common/Schema/PositionColumnTest.php diff --git a/tests/Database/Functional/Driver/Common/Schema/PositionColumnTest.php b/tests/Database/Functional/Driver/Common/Schema/PositionColumnTest.php new file mode 100644 index 00000000..516390d6 --- /dev/null +++ b/tests/Database/Functional/Driver/Common/Schema/PositionColumnTest.php @@ -0,0 +1,108 @@ +schema($table); + + if (! $schema->exists()) { + $schema->primary('id'); + $schema->string('first_name')->nullable(false); + $schema->string('last_name')->nullable(false); + $schema->string('email', 64)->nullable(false); + $schema->enum('status', ['active', 'disabled'])->defaultValue('active'); + $schema->double('balance')->defaultValue(0); + $schema->datetime('created_at')->defaultValue(AbstractColumn::DATETIME_NOW); + $schema->datetime('updated_at')->nullable(true); + + $schema->save(Handler::DO_ALL); + } + + return $schema; + } + + public function testPositionFirst(): void + { + $schema = $this->sampleSchema('table'); + + $this->assertTrue($schema->exists()); + $this->assertSameAsInDB($schema); + + $schema->string('identifier')->nullable(false)->first(); + $schema->save(); + + $this->assertSameAsInDB($schema); + + $updatedSchema = $this->fetchSchema($schema); + $updatedColumnNames = \array_map(static fn(AbstractColumn $column) => $column->getName(), $updatedSchema->getColumns()); + + $expectedColumnNames = [ + 'identifier' => 'identifier', + 'id' => 'id', + 'first_name' => 'first_name', + 'last_name' => 'last_name', + 'email' => 'email', + 'status' => 'status', + 'balance' => 'balance', + 'created_at' => 'created_at', + 'updated_at' => 'updated_at', + ]; + + $this->assertSame($expectedColumnNames, $updatedColumnNames); + } + + public function testPositionAfter(): void + { + $schema = $this->sampleSchema('table'); + + $this->assertTrue($schema->exists()); + $this->assertSameAsInDB($schema); + + $schema->string('identifier')->nullable(false)->after('email'); + $schema->save(); + + $this->assertSameAsInDB($schema); + + $updatedSchema = $this->fetchSchema($schema); + $updatedColumnNames = \array_map(static fn(AbstractColumn $column) => $column->getName(), $updatedSchema->getColumns()); + + $expectedColumnNames = [ + 'id' => 'id', + 'first_name' => 'first_name', + 'last_name' => 'last_name', + 'email' => 'email', + 'identifier' => 'identifier', + 'status' => 'status', + 'balance' => 'balance', + 'created_at' => 'created_at', + 'updated_at' => 'updated_at', + ]; + + $this->assertSame($expectedColumnNames, $updatedColumnNames); + } + + public function testPositionAfterThrowsException(): void + { + $schema = $this->sampleSchema('table'); + + $this->assertTrue($schema->exists()); + $this->assertSameAsInDB($schema); + + $this->expectException(DBALException::class); + $this->expectExceptionMessage('Column `nonexistent` does not exist for positioning after'); + + $schema->string('identifier')->nullable(false)->after('nonexistent'); + $schema->save(); + } +} From e8774adef28b292699d487066828fc54f20712ce Mon Sep 17 00:00:00 2001 From: Adam Dyson Date: Fri, 27 Jun 2025 20:38:44 +1000 Subject: [PATCH 07/14] Add MySQL test class for testing column positioning --- .../Driver/MySQL/Schema/PositionColumnTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 tests/Database/Functional/Driver/MySQL/Schema/PositionColumnTest.php diff --git a/tests/Database/Functional/Driver/MySQL/Schema/PositionColumnTest.php b/tests/Database/Functional/Driver/MySQL/Schema/PositionColumnTest.php new file mode 100644 index 00000000..04d8f049 --- /dev/null +++ b/tests/Database/Functional/Driver/MySQL/Schema/PositionColumnTest.php @@ -0,0 +1,16 @@ + Date: Fri, 4 Jul 2025 15:40:43 +1000 Subject: [PATCH 08/14] Readded getComment annotation and removed isFirst and getAfter methods. Which are now only defined in MySQLColumn class. --- src/ColumnInterface.php | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/src/ColumnInterface.php b/src/ColumnInterface.php index 0a375b06..459eab83 100644 --- a/src/ColumnInterface.php +++ b/src/ColumnInterface.php @@ -13,6 +13,9 @@ /** * Represents table schema column abstraction. + * + * @method string getComment() Get column comment. + * An empty string will be returned if the feature is not supported by the driver. */ interface ColumnInterface { @@ -77,22 +80,4 @@ public function hasDefaultValue(): bool; * Get column default value, value must be automatically converted to appropriate internal type. */ public function getDefaultValue(): mixed; - - /** - * Whether the column should be positioned first. Always returning false - * if the driver does not support this feature. - */ - public function isFirst(): bool; - - /** - * Retrieve the column name to position after. Always returning an empty string - * if the driver does not support this feature. - */ - public function getAfter(): string; - - /** - * Retrieve the column comment. Always returning an empty string - * if the driver does not support this feature. - */ - public function getComment(): string; } From 336d23e2d3e01f88fefd1349a2ccbea8d7d89122 Mon Sep 17 00:00:00 2001 From: Adam Dyson Date: Fri, 4 Jul 2025 15:41:35 +1000 Subject: [PATCH 09/14] Removed first, isFirst, after, and getAfter methods. Which are now only defined in MySQLColumn class. --- src/Schema/AbstractColumn.php | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/Schema/AbstractColumn.php b/src/Schema/AbstractColumn.php index 365af6d8..f83ceb79 100644 --- a/src/Schema/AbstractColumn.php +++ b/src/Schema/AbstractColumn.php @@ -623,26 +623,6 @@ public function isReadonlySchema(): bool return $this->getAttributes()['readonlySchema'] ?? false; } - public function first(bool $first = true): self - { - return $this; - } - - public function isFirst(): bool - { - return false; - } - - public function after(string $column): self - { - return $this; - } - - public function getAfter(): string - { - return ''; - } - /** * Get column comment. * An empty string will be returned if the feature is not supported by the driver. From 7fe7a1d92c97c79e0a7adef85e5f11675998479c Mon Sep 17 00:00:00 2001 From: Adam Dyson Date: Sun, 6 Jul 2025 13:36:39 +1000 Subject: [PATCH 10/14] Removed white space, as no changes are needed for this file --- src/ColumnInterface.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ColumnInterface.php b/src/ColumnInterface.php index 459eab83..1fbe8284 100644 --- a/src/ColumnInterface.php +++ b/src/ColumnInterface.php @@ -15,7 +15,7 @@ * Represents table schema column abstraction. * * @method string getComment() Get column comment. - * An empty string will be returned if the feature is not supported by the driver. + * An empty string will be returned if the feature is not supported by the driver. */ interface ColumnInterface { From 430d792b71a64025e23c035df0096f300262573b Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Wed, 9 Jul 2025 11:27:18 +0400 Subject: [PATCH 11/14] refactor: simplify column creation logic --- src/Driver/Handler.php | 18 ++--------- src/Driver/MySQL/Schema/MySQLColumn.php | 42 ++++++++++++++++--------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/Driver/Handler.php b/src/Driver/Handler.php index b319ee23..20fb751d 100644 --- a/src/Driver/Handler.php +++ b/src/Driver/Handler.php @@ -89,21 +89,9 @@ public function renameTable(string $table, string $name): void public function createColumn(AbstractTable $table, AbstractColumn $column): void { - $statement = "ALTER TABLE {$this->identify($table)} ADD COLUMN {$column->sqlStatement($this->driver)}"; - - if ($column->isFirst()) { - $statement = "{$statement} FIRST"; - } elseif ($column->getAfter() !== '') { - $columnNames = \array_map(static fn(AbstractColumn $column) => $column->getName(), $table->getColumns()); - - if (! \in_array($column->getAfter(), $columnNames)) { - throw new DBALException("Column `{$column->getAfter()}` does not exist for positioning after"); - } - - $statement = "{$statement} AFTER {$this->driver->identifier($column->getAfter())}"; - } - - $this->run($statement); + $this->run( + "ALTER TABLE {$this->identify($table)} ADD COLUMN {$column->sqlStatement($this->driver)}", + ); } public function dropColumn(AbstractTable $table, AbstractColumn $column): void diff --git a/src/Driver/MySQL/Schema/MySQLColumn.php b/src/Driver/MySQL/Schema/MySQLColumn.php index 8ab4263e..395a827b 100644 --- a/src/Driver/MySQL/Schema/MySQLColumn.php +++ b/src/Driver/MySQL/Schema/MySQLColumn.php @@ -187,11 +187,13 @@ class MySQLColumn extends AbstractColumn /** * Column name to position after. */ + #[ColumnAttribute] protected string $after = ''; /** * Whether the column should be positioned first. */ + #[ColumnAttribute] protected bool $first = false; /** @@ -293,23 +295,30 @@ public static function createInstance(string $table, array $schema, ?\DateTimeZo public function sqlStatement(DriverInterface $driver): string { if (\in_array($this->type, self::INTEGER_TYPES, true)) { - return $this->sqlStatementInteger($driver); - } + $statement = $this->sqlStatementInteger($driver); + } else { + $defaultValue = $this->defaultValue; + + if (\in_array($this->type, $this->forbiddenDefaults, true)) { + // Flushing default value for forbidden types + $this->defaultValue = null; + } - $defaultValue = $this->defaultValue; + $statement = parent::sqlStatement($driver); - if (\in_array($this->type, $this->forbiddenDefaults, true)) { - //Flushing default value for forbidden types - $this->defaultValue = null; + $this->defaultValue = $defaultValue; } - $statement = parent::sqlStatement($driver); + $this->comment === '' or $statement .= " COMMENT {$driver->quote($this->comment)}"; - $this->defaultValue = $defaultValue; + $first = $this->first; + $after = $first ? '' : $this->after; - if ($this->comment !== '') { - return "{$statement} COMMENT {$driver->quote($this->comment)}"; - } + $statement .= match (true) { + $first => ' FIRST', + $after !== '' => " AFTER {$driver->identifier($after)}", + default => '', + }; return $statement; } @@ -335,9 +344,9 @@ public function isZerofill(): bool return $this->zerofill; } - public function first(bool $first = true): self + public function first(bool $value = true): self { - $this->first = $first; + $this->first = $value; return $this; } @@ -347,6 +356,10 @@ public function isFirst(): bool return $this->first; } + /** + * @param non-empty-string $column + * @return $this + */ public function after(string $column): self { $this->after = $column; @@ -429,12 +442,11 @@ protected function formatDatetime( private function sqlStatementInteger(DriverInterface $driver): string { return \sprintf( - '%s %s(%s)%s%s%s%s%s%s', + '%s %s(%s)%s%s%s%s%s', $driver->identifier($this->name), $this->type, $this->size, $this->unsigned ? ' UNSIGNED' : '', - $this->comment !== '' ? " COMMENT {$driver->quote($this->comment)}" : '', $this->zerofill ? ' ZEROFILL' : '', $this->nullable ? ' NULL' : ' NOT NULL', $this->defaultValue !== null ? " DEFAULT {$this->quoteDefault($driver)}" : '', From 0d6f4b0a7be4ce8cc150048aafee845264be66c4 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Wed, 9 Jul 2025 11:28:21 +0400 Subject: [PATCH 12/14] test(PositionColumn): add `DontGenerateAttribute` annotation --- .../Common/Schema/PositionColumnTest.php | 44 ++++++++++--------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/tests/Database/Functional/Driver/Common/Schema/PositionColumnTest.php b/tests/Database/Functional/Driver/Common/Schema/PositionColumnTest.php index 516390d6..20a80dab 100644 --- a/tests/Database/Functional/Driver/Common/Schema/PositionColumnTest.php +++ b/tests/Database/Functional/Driver/Common/Schema/PositionColumnTest.php @@ -9,29 +9,11 @@ use Cycle\Database\Schema\AbstractColumn; use Cycle\Database\Schema\AbstractTable; use Cycle\Database\Tests\Functional\Driver\Common\BaseTest; +use Cycle\Database\Tests\Utils\DontGenerateAttribute; +#[DontGenerateAttribute] abstract class PositionColumnTest extends BaseTest { - public function sampleSchema(string $table): AbstractTable - { - $schema = $this->schema($table); - - if (! $schema->exists()) { - $schema->primary('id'); - $schema->string('first_name')->nullable(false); - $schema->string('last_name')->nullable(false); - $schema->string('email', 64)->nullable(false); - $schema->enum('status', ['active', 'disabled'])->defaultValue('active'); - $schema->double('balance')->defaultValue(0); - $schema->datetime('created_at')->defaultValue(AbstractColumn::DATETIME_NOW); - $schema->datetime('updated_at')->nullable(true); - - $schema->save(Handler::DO_ALL); - } - - return $schema; - } - public function testPositionFirst(): void { $schema = $this->sampleSchema('table'); @@ -100,9 +82,29 @@ public function testPositionAfterThrowsException(): void $this->assertSameAsInDB($schema); $this->expectException(DBALException::class); - $this->expectExceptionMessage('Column `nonexistent` does not exist for positioning after'); + $this->expectExceptionMessage("Unknown column 'nonexistent'"); $schema->string('identifier')->nullable(false)->after('nonexistent'); $schema->save(); } + + private function sampleSchema(string $table): AbstractTable + { + $schema = $this->schema($table); + + if (! $schema->exists()) { + $schema->primary('id'); + $schema->string('first_name')->nullable(false); + $schema->string('last_name')->nullable(false); + $schema->string('email', 64)->nullable(false); + $schema->enum('status', ['active', 'disabled'])->defaultValue('active'); + $schema->double('balance')->defaultValue(0); + $schema->datetime('created_at')->defaultValue(AbstractColumn::DATETIME_NOW); + $schema->datetime('updated_at')->nullable(true); + + $schema->save(Handler::DO_ALL); + } + + return $schema; + } } From 9e0ac5ffb48c3b02a1d173d3d41d663a73968942 Mon Sep 17 00:00:00 2001 From: Adam Dyson Date: Wed, 9 Jul 2025 22:13:24 +1000 Subject: [PATCH 13/14] Removed methods isFirst and getAfter from MySQLColumn class, not needed as this functionality is only relevant for migrations --- src/Driver/MySQL/Schema/MySQLColumn.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/Driver/MySQL/Schema/MySQLColumn.php b/src/Driver/MySQL/Schema/MySQLColumn.php index 395a827b..e955d8e5 100644 --- a/src/Driver/MySQL/Schema/MySQLColumn.php +++ b/src/Driver/MySQL/Schema/MySQLColumn.php @@ -351,11 +351,6 @@ public function first(bool $value = true): self return $this; } - public function isFirst(): bool - { - return $this->first; - } - /** * @param non-empty-string $column * @return $this @@ -367,11 +362,6 @@ public function after(string $column): self return $this; } - public function getAfter(): string - { - return $this->after; - } - public function set(string|array $values): self { $this->type('set'); From a3af650d04e164cf4534ad5f4cae99541df4dadc Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Wed, 9 Jul 2025 17:57:59 +0400 Subject: [PATCH 14/14] feat(MySQLColumn): convert `after` method into annotation declaration --- src/Driver/MySQL/Schema/MySQLColumn.php | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/Driver/MySQL/Schema/MySQLColumn.php b/src/Driver/MySQL/Schema/MySQLColumn.php index e955d8e5..6e34add4 100644 --- a/src/Driver/MySQL/Schema/MySQLColumn.php +++ b/src/Driver/MySQL/Schema/MySQLColumn.php @@ -33,6 +33,7 @@ * @method $this|AbstractColumn unsigned(bool $value) * @method $this|AbstractColumn zerofill(bool $value) * @method $this|AbstractColumn comment(string $value) + * @method $this|AbstractColumn after(string $column) */ class MySQLColumn extends AbstractColumn { @@ -351,17 +352,6 @@ public function first(bool $value = true): self return $this; } - /** - * @param non-empty-string $column - * @return $this - */ - public function after(string $column): self - { - $this->after = $column; - - return $this; - } - public function set(string|array $values): self { $this->type('set');