From fa33f6d112b795398087865f805b62a5f756fec1 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Tue, 10 Dec 2024 13:29:17 +0200 Subject: [PATCH 01/21] Added nativeuuid type for databases with a native uuid type --- src/Db/Adapter/AdapterInterface.php | 1 + src/Db/Adapter/MysqlAdapter.php | 2 ++ src/Db/Adapter/PostgresAdapter.php | 1 + src/Db/Adapter/SqlserverAdapter.php | 1 + src/Db/Table/Column.php | 1 + 5 files changed, 6 insertions(+) diff --git a/src/Db/Adapter/AdapterInterface.php b/src/Db/Adapter/AdapterInterface.php index fcc67cc7..de7025c0 100644 --- a/src/Db/Adapter/AdapterInterface.php +++ b/src/Db/Adapter/AdapterInterface.php @@ -51,6 +51,7 @@ interface AdapterInterface public const PHINX_TYPE_JSON = 'json'; public const PHINX_TYPE_JSONB = 'jsonb'; public const PHINX_TYPE_UUID = 'uuid'; + public const PHINX_TYPE_NATIVEUUID = 'nativeuuid'; public const PHINX_TYPE_FILESTREAM = 'filestream'; // Geospatial database types diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 4d260e20..09d3dc58 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -1061,6 +1061,8 @@ public function getSqlType(Literal|string $type, ?int $limit = null): array return ['name' => 'tinyint', 'limit' => 1]; case static::PHINX_TYPE_UUID: return ['name' => 'char', 'limit' => 36]; + case static::PHINX_TYPE_NATIVEUUID: + return ['name' => 'uuid']; case static::PHINX_TYPE_YEAR: if (!$limit || in_array($limit, [2, 4])) { $limit = 4; diff --git a/src/Db/Adapter/PostgresAdapter.php b/src/Db/Adapter/PostgresAdapter.php index a525c219..556d74a1 100644 --- a/src/Db/Adapter/PostgresAdapter.php +++ b/src/Db/Adapter/PostgresAdapter.php @@ -1023,6 +1023,7 @@ public function getSqlType(Literal|string $type, ?int $limit = null): array case static::PHINX_TYPE_DATETIME: return ['name' => 'timestamp']; case static::PHINX_TYPE_BINARYUUID: + case static::PHINX_TYPE_NATIVEUUID: return ['name' => 'uuid']; case static::PHINX_TYPE_BLOB: case static::PHINX_TYPE_BINARY: diff --git a/src/Db/Adapter/SqlserverAdapter.php b/src/Db/Adapter/SqlserverAdapter.php index aa844072..12bd5341 100644 --- a/src/Db/Adapter/SqlserverAdapter.php +++ b/src/Db/Adapter/SqlserverAdapter.php @@ -1025,6 +1025,7 @@ public function getSqlType(Literal|string $type, ?int $limit = null): array return ['name' => 'bit']; case static::PHINX_TYPE_BINARYUUID: case static::PHINX_TYPE_UUID: + case static::PHINX_TYPE_NATIVEUUID: return ['name' => 'uniqueidentifier']; case static::PHINX_TYPE_FILESTREAM: return ['name' => 'varbinary', 'limit' => 'max']; diff --git a/src/Db/Table/Column.php b/src/Db/Table/Column.php index 668d911f..64fb9a29 100644 --- a/src/Db/Table/Column.php +++ b/src/Db/Table/Column.php @@ -36,6 +36,7 @@ class Column public const TIMESTAMP = AdapterInterface::PHINX_TYPE_TIMESTAMP; public const UUID = AdapterInterface::PHINX_TYPE_UUID; public const BINARYUUID = AdapterInterface::PHINX_TYPE_BINARYUUID; + public const NATIVEUUID = AdapterInterface::PHINX_TYPE_NATIVEUUID; /** MySQL-only column type */ public const MEDIUMINTEGER = AdapterInterface::PHINX_TYPE_MEDIUM_INTEGER; /** MySQL-only column type */ From f7dca12c2e1aa0281304b223bec9bc0cbccd67d1 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Tue, 10 Dec 2024 15:02:01 +0200 Subject: [PATCH 02/21] Added tests to postgres to ensure that nativeuuid behaves exactly like the uuid type --- .../Db/Adapter/PostgresAdapterTest.php | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/TestCase/Db/Adapter/PostgresAdapterTest.php b/tests/TestCase/Db/Adapter/PostgresAdapterTest.php index 98a9acd8..4484745e 100644 --- a/tests/TestCase/Db/Adapter/PostgresAdapterTest.php +++ b/tests/TestCase/Db/Adapter/PostgresAdapterTest.php @@ -343,6 +343,23 @@ public function testCreateTableWithPrimaryKeyAsBinaryUuid() $this->assertTrue($this->adapter->hasColumn('ztable', 'user_id')); } + /** + * @return void + */ + public function testCreateTableWithPrimaryKeyAsNativeUuid() + { + $options = [ + 'id' => false, + 'primary_key' => 'id', + ]; + $table = new Table('ztable', $options, $this->adapter); + $table->addColumn('id', 'nativeuuid')->save(); + $table->addColumn('user_id', 'integer')->save(); + $this->assertTrue($this->adapter->hasColumn('ztable', 'id')); + $this->assertTrue($this->adapter->hasIndex('ztable', 'id')); + $this->assertTrue($this->adapter->hasColumn('ztable', 'user_id')); + } + public function testCreateTableWithMultipleIndexes() { $table = new Table('table1', [], $this->adapter); @@ -1066,6 +1083,24 @@ public function testChangeColumnCharToUuid() } } + public function testChangeColumnCharToNativeUuid() + { + $table = new Table('t', [], $this->adapter); + $table->addColumn('column1', 'char', ['default' => null, 'limit' => 36]) + ->save(); + $table->changeColumn('column1', 'nativeuuid', ['default' => null, 'null' => true]) + ->save(); + $columns = $this->adapter->getColumns('t'); + foreach ($columns as $column) { + if ($column->getName() === 'column1') { + $this->assertTrue($column->isNull()); + $this->assertNull($column->getDefault()); + $columnType = $table->getColumn('column1')->getType(); + $this->assertSame($columnType, 'uuid'); + } + } + } + public function testChangeColumnWithDefault() { $table = new Table('t', [], $this->adapter); @@ -1875,6 +1910,7 @@ public function testGetPhinxType() $this->assertEquals('datetime', $this->adapter->getPhinxType('timestamp without time zone')); $this->assertEquals('uuid', $this->adapter->getPhinxType('uuid')); + $this->assertEquals('uuid', $this->adapter->getPhinxType('nativeuuid')); $this->assertEquals('interval', $this->adapter->getPhinxType('interval')); } From 8f9634f95fe42d233779ccb2a386ecdbf95f5bb4 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Tue, 10 Dec 2024 15:10:12 +0200 Subject: [PATCH 03/21] Add the native uuid type to specificColumnTypes to allow usage as primary key --- src/Db/Adapter/MysqlAdapter.php | 1 + src/Db/Adapter/PostgresAdapter.php | 1 + src/Db/Adapter/SqlserverAdapter.php | 1 + 3 files changed, 3 insertions(+) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 09d3dc58..3244a18f 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -32,6 +32,7 @@ class MysqlAdapter extends PdoAdapter self::PHINX_TYPE_YEAR, self::PHINX_TYPE_JSON, self::PHINX_TYPE_BINARYUUID, + self::PHINX_TYPE_NATIVEUUID, self::PHINX_TYPE_TINYBLOB, self::PHINX_TYPE_MEDIUMBLOB, self::PHINX_TYPE_LONGBLOB, diff --git a/src/Db/Adapter/PostgresAdapter.php b/src/Db/Adapter/PostgresAdapter.php index 556d74a1..7c6facfd 100644 --- a/src/Db/Adapter/PostgresAdapter.php +++ b/src/Db/Adapter/PostgresAdapter.php @@ -39,6 +39,7 @@ class PostgresAdapter extends PdoAdapter self::PHINX_TYPE_MACADDR, self::PHINX_TYPE_INTERVAL, self::PHINX_TYPE_BINARYUUID, + self::PHINX_TYPE_NATIVEUUID, ]; private const GIN_INDEX_TYPE = 'gin'; diff --git a/src/Db/Adapter/SqlserverAdapter.php b/src/Db/Adapter/SqlserverAdapter.php index 12bd5341..2038260d 100644 --- a/src/Db/Adapter/SqlserverAdapter.php +++ b/src/Db/Adapter/SqlserverAdapter.php @@ -29,6 +29,7 @@ class SqlserverAdapter extends PdoAdapter protected static array $specificColumnTypes = [ self::PHINX_TYPE_FILESTREAM, self::PHINX_TYPE_BINARYUUID, + self::PHINX_TYPE_NATIVEUUID, ]; /** From a52c42353ac91fcb96384599a4c2409a44657a99 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Tue, 10 Dec 2024 15:11:46 +0200 Subject: [PATCH 04/21] Remove incorrect test assertion --- tests/TestCase/Db/Adapter/PostgresAdapterTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/TestCase/Db/Adapter/PostgresAdapterTest.php b/tests/TestCase/Db/Adapter/PostgresAdapterTest.php index 4484745e..eb713ca3 100644 --- a/tests/TestCase/Db/Adapter/PostgresAdapterTest.php +++ b/tests/TestCase/Db/Adapter/PostgresAdapterTest.php @@ -1910,7 +1910,6 @@ public function testGetPhinxType() $this->assertEquals('datetime', $this->adapter->getPhinxType('timestamp without time zone')); $this->assertEquals('uuid', $this->adapter->getPhinxType('uuid')); - $this->assertEquals('uuid', $this->adapter->getPhinxType('nativeuuid')); $this->assertEquals('interval', $this->adapter->getPhinxType('interval')); } From 5506ee07446e3f2c0264ca155e6f67f004452ae9 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Tue, 10 Dec 2024 15:40:56 +0200 Subject: [PATCH 05/21] Properly handle converting from text to binary and native uuid columns --- src/Db/Adapter/PostgresAdapter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Db/Adapter/PostgresAdapter.php b/src/Db/Adapter/PostgresAdapter.php index 7c6facfd..093d6d77 100644 --- a/src/Db/Adapter/PostgresAdapter.php +++ b/src/Db/Adapter/PostgresAdapter.php @@ -549,7 +549,7 @@ protected function getChangeColumnInstructions( $quotedColumnName ); } - if ($newColumn->getType() === 'uuid') { + if (in_array($newColumn->getType(), ['uuid', 'nativeuuid', 'binaryuuid'])) { $sql .= sprintf( ' USING (%s::uuid)', $quotedColumnName From 264332f9cd4b798501d70fdf3c72cce21df88add Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Tue, 10 Dec 2024 15:41:45 +0200 Subject: [PATCH 06/21] Enable the nativeuuid type conditionally. --- src/Db/Adapter/MysqlAdapter.php | 12 +++++- .../TestCase/Db/Adapter/MysqlAdapterTest.php | 37 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 3244a18f..5e1ba225 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -32,7 +32,6 @@ class MysqlAdapter extends PdoAdapter self::PHINX_TYPE_YEAR, self::PHINX_TYPE_JSON, self::PHINX_TYPE_BINARYUUID, - self::PHINX_TYPE_NATIVEUUID, self::PHINX_TYPE_TINYBLOB, self::PHINX_TYPE_MEDIUMBLOB, self::PHINX_TYPE_LONGBLOB, @@ -1494,6 +1493,15 @@ public function describeTable(string $tableName): array */ public function getColumnTypes(): array { - return array_merge(parent::getColumnTypes(), static::$specificColumnTypes); + $connection = $this->getConnection(); + $version = $connection->getDriver()->version(); + + $types = array_merge(parent::getColumnTypes(), static::$specificColumnTypes); + + if (version_compare($version, '10.7', '>=')) { + $types[] = self::PHINX_TYPE_NATIVEUUID; + } + + return $types; } } diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 824b0f4d..75d7a36d 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -12,6 +12,7 @@ use InvalidArgumentException; use Migrations\Db\Adapter\AdapterInterface; use Migrations\Db\Adapter\MysqlAdapter; +use Migrations\Db\Adapter\UnsupportedColumnTypeException; use Migrations\Db\Literal; use Migrations\Db\Table; use Migrations\Db\Table\Column; @@ -82,6 +83,13 @@ private function usingMysql8(): bool return version_compare($version, '8.0.0', '>='); } + private function usingMariaDbWithUuid(): bool + { + $version = $this->adapter->getConnection()->getDriver()->version(); + + return version_compare($version, '10.7.0', '>='); + } + public function testConnection() { $this->assertInstanceOf(Connection::class, $this->adapter->getConnection()); @@ -329,6 +337,27 @@ public function testCreateTableWithPrimaryKeyAsBinaryUuid() $this->assertTrue($this->adapter->hasColumn('ztable', 'user_id')); } + /** + * @return void + */ + public function testCreateTableWithPrimaryKeyAsNativeUuid() + { + if (!$this->usingMariaDbWithUuid()) { + $this->markTestSkipped('Database does not have a native uuid type'); + } + + $options = [ + 'id' => false, + 'primary_key' => 'id', + ]; + $table = new Table('ztable', $options, $this->adapter); + $table->addColumn('id', 'nativeuuid', ['null' => false])->save(); + $table->addColumn('user_id', 'integer')->save(); + $this->assertTrue($this->adapter->hasColumn('ztable', 'id')); + $this->assertTrue($this->adapter->hasIndex('ztable', 'id')); + $this->assertTrue($this->adapter->hasColumn('ztable', 'user_id')); + } + public function testCreateTableWithMultipleIndexes() { $table = new Table('table1', [], $this->adapter); @@ -2522,4 +2551,12 @@ public function testGetPhinxTypeFromSQLDefinition(string $sqlDefinition, array $ $this->assertSame($expectedResponse['name'], $result['name'], "Type mismatch - got '{$result['name']}' when expecting '{$expectedResponse['name']}'"); $this->assertSame($expectedResponse['limit'], $result['limit'], "Field upper boundary mismatch - got '{$result['limit']}' when expecting '{$expectedResponse['limit']}'"); } + + public function testGetPhinxType() + { + if (!$this->usingMariaDbWithUuid()) { + $this->expectException(UnsupportedColumnTypeException::class); + } + $this->assertSame('uuid', $this->adapter->getSqlType('nativeuuid')); + } } From dd4288b91ac531be26b9fc1fa061004a3bb9d907 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Tue, 10 Dec 2024 15:44:38 +0200 Subject: [PATCH 07/21] Fix test assertion --- tests/TestCase/Db/Adapter/MysqlAdapterTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 75d7a36d..60596289 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -2557,6 +2557,6 @@ public function testGetPhinxType() if (!$this->usingMariaDbWithUuid()) { $this->expectException(UnsupportedColumnTypeException::class); } - $this->assertSame('uuid', $this->adapter->getSqlType('nativeuuid')); + $this->assertSame(['name' => 'uuid'], $this->adapter->getSqlType('nativeuuid')); } } From 781387cb82a8230e90d9cc9f4e55323816e55ff2 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Tue, 10 Dec 2024 15:58:54 +0200 Subject: [PATCH 08/21] Check for uuid support in getSqlType as well --- src/Db/Adapter/MysqlAdapter.php | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 5e1ba225..7ac1165a 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -1062,6 +1062,11 @@ public function getSqlType(Literal|string $type, ?int $limit = null): array case static::PHINX_TYPE_UUID: return ['name' => 'char', 'limit' => 36]; case static::PHINX_TYPE_NATIVEUUID: + if (!$this->hasNativeUuid()) { + throw new UnsupportedColumnTypeException( + 'Column type "' . $type . '" is not supported by this version of MySQL.' + ); + } return ['name' => 'uuid']; case static::PHINX_TYPE_YEAR: if (!$limit || in_array($limit, [2, 4])) { @@ -1493,15 +1498,25 @@ public function describeTable(string $tableName): array */ public function getColumnTypes(): array { - $connection = $this->getConnection(); - $version = $connection->getDriver()->version(); - $types = array_merge(parent::getColumnTypes(), static::$specificColumnTypes); - if (version_compare($version, '10.7', '>=')) { + if ($this->hasNativeUuid()) { $types[] = self::PHINX_TYPE_NATIVEUUID; } return $types; } + + /** + * Whether the server has a native uuid type. + * (MariaDB 10.7.0+) + * @return bool + */ + protected function hasNativeUuid(): bool + { + $connection = $this->getConnection(); + $version = $connection->getDriver()->version(); + + return version_compare($version, '10.7', '>='); + } } From 8303d1a535633d87d12f7dab66535ddd3e8c9544 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Tue, 10 Dec 2024 16:01:44 +0200 Subject: [PATCH 09/21] Coding style fix --- src/Db/Adapter/MysqlAdapter.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 7ac1165a..a84230fe 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -1067,6 +1067,7 @@ public function getSqlType(Literal|string $type, ?int $limit = null): array 'Column type "' . $type . '" is not supported by this version of MySQL.' ); } + return ['name' => 'uuid']; case static::PHINX_TYPE_YEAR: if (!$limit || in_array($limit, [2, 4])) { @@ -1510,6 +1511,7 @@ public function getColumnTypes(): array /** * Whether the server has a native uuid type. * (MariaDB 10.7.0+) + * * @return bool */ protected function hasNativeUuid(): bool From b042de97620897da9834672e843ddbf9483fdd8f Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Tue, 10 Dec 2024 16:42:57 +0200 Subject: [PATCH 10/21] Map nativeuuid phinx type when the database type is uuid --- src/Db/Adapter/MysqlAdapter.php | 4 ++++ tests/TestCase/Db/Adapter/MysqlAdapterTest.php | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index a84230fe..954ae353 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -1187,6 +1187,10 @@ public function getPhinxType(string $sqlTypeDef): array $type = static::PHINX_TYPE_BINARYUUID; } break; + case 'uuid': + $type = static::PHINX_TYPE_NATIVEUUID; + $limit = null; + break; } try { diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 60596289..7c8873de 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -2552,7 +2552,7 @@ public function testGetPhinxTypeFromSQLDefinition(string $sqlDefinition, array $ $this->assertSame($expectedResponse['limit'], $result['limit'], "Field upper boundary mismatch - got '{$result['limit']}' when expecting '{$expectedResponse['limit']}'"); } - public function testGetPhinxType() + public function testGetSqlType() { if (!$this->usingMariaDbWithUuid()) { $this->expectException(UnsupportedColumnTypeException::class); From aebd91d936de38f2550da4c2c3563a290e06c8a2 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Wed, 11 Dec 2024 09:43:10 +0200 Subject: [PATCH 11/21] Added MariaDB to ci --- .github/workflows/ci.yml | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fac01bed..e3979d93 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,7 +20,7 @@ jobs: fail-fast: false matrix: php-version: ['8.1', '8.2', '8.3', '8.4'] - db-type: [mysql, pgsql, sqlite] + db-type: [mariadb, mysql, pgsql, sqlite] prefer-lowest: [''] cake_version: [''] include: @@ -50,6 +50,17 @@ jobs: steps: - uses: actions/checkout@v4 + - name: Setup MariaDB + uses: getong/mariadb-action@v1.11 + if: matrix.db-type == 'mariadb' + with: + mariadb version: '10.11.10' + mysql database: 'cakephp_test' + mysql root password: 'root' + run: | + mariadb -h 127.0.0.1 -u root -proot -e 'CREATE DATABASE cakephp_comparisons;' + mariadb -h 127.0.0.1 -u root -proot -e 'CREATE DATABASE cakephp_snapshot;' + - name: Setup MySQL if: matrix.db-type == 'mysql' run: | @@ -106,6 +117,12 @@ jobs: if [[ ${{ matrix.db-type }} == 'sqlite' ]]; then export DB='sqlite' fi + if [[ ${{ matrix.db-type }} == 'mariadb' ]]; then + export DB='mysql' + export DB_URL='mysql://root:root@127.0.0.1/cakephp_test' + export DB_URL_COMPARE='mysql://root:root@127.0.0.1/cakephp_comparisons' + export DB_URL_SNAPSHOT='mysql://root:root@127.0.0.1/cakephp_snapshot' + fi if [[ ${{ matrix.db-type }} == 'mysql' ]]; then export DB='mysql' export DB_URL='mysql://root:root@127.0.0.1/cakephp_test' From 45b3983b0a000def8cb70b8cc50972a4cbac2e44 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Wed, 11 Dec 2024 09:51:15 +0200 Subject: [PATCH 12/21] Split the creation of extra mariadb databases in another step --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e3979d93..444e8442 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,6 +57,8 @@ jobs: mariadb version: '10.11.10' mysql database: 'cakephp_test' mysql root password: 'root' + - name: Setup MariaDB (part 2) + if: matrix.db-type == 'mariadb' run: | mariadb -h 127.0.0.1 -u root -proot -e 'CREATE DATABASE cakephp_comparisons;' mariadb -h 127.0.0.1 -u root -proot -e 'CREATE DATABASE cakephp_snapshot;' From 25778709ca475c4de116147645068e9a0926df1e Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Wed, 11 Dec 2024 09:53:41 +0200 Subject: [PATCH 13/21] use the mysql command which actually exists --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 444e8442..11e16488 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,8 +60,8 @@ jobs: - name: Setup MariaDB (part 2) if: matrix.db-type == 'mariadb' run: | - mariadb -h 127.0.0.1 -u root -proot -e 'CREATE DATABASE cakephp_comparisons;' - mariadb -h 127.0.0.1 -u root -proot -e 'CREATE DATABASE cakephp_snapshot;' + mysql -h 127.0.0.1 -u root -proot -e 'CREATE DATABASE cakephp_comparisons;' + mysql -h 127.0.0.1 -u root -proot -e 'CREATE DATABASE cakephp_snapshot;' - name: Setup MySQL if: matrix.db-type == 'mysql' From 0c702937cc50d2c4422913e741d2e5b7400fb080 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Wed, 11 Dec 2024 10:14:19 +0200 Subject: [PATCH 14/21] Test fixes for mariadb --- .../TestCase/Db/Adapter/MysqlAdapterTest.php | 22 ++++++++++++++----- tests/TestCase/MigrationsTest.php | 6 ++++- .../addRemove/the_diff_add_remove_mysql.php | 9 +++++++- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 7c8873de..77b2cc43 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -426,7 +426,7 @@ public function testCreateTableAndInheritDefaultCollation() ->save(); $this->assertTrue($adapter->hasTable('table_with_default_collation')); $row = $adapter->fetchRow(sprintf("SHOW TABLE STATUS WHERE Name = '%s'", 'table_with_default_collation')); - $this->assertEquals('utf8mb4_0900_ai_ci', $row['Collation']); + $this->assertContains($row['Collation'], ['utf8mb4_0900_ai_ci', 'utf8mb4_unicode_520_ci']); } public function testCreateTableWithLatin1Collate() @@ -2181,8 +2181,14 @@ public function testDumpCreateTable() ->addColumn('column3', 'string', ['default' => 'test', 'null' => false]) ->save(); - $expectedOutput = <<<'OUTPUT' -CREATE TABLE `table1` (`id` INT(11) unsigned NOT NULL AUTO_INCREMENT, `column1` VARCHAR(255) NOT NULL, `column2` INT(11) NULL, `column3` VARCHAR(255) NOT NULL DEFAULT 'test', PRIMARY KEY (`id`)) ENGINE = InnoDB CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci; + if ($this->usingMariaDbWithUuid()) { + $collation = 'utf8mb4_unicode_520_ci'; + } else { + $collation = 'utf8mb4_0900_ai_ci'; + } + + $expectedOutput = <<out->messages()); $this->assertStringContainsString($expectedOutput, $actualOutput, 'Passing the --dry-run option does not dump create table query to the output'); @@ -2286,8 +2292,14 @@ public function testDumpCreateTableAndThenInsert() 'column2' => 1, ])->save(); - $expectedOutput = <<<'OUTPUT' -CREATE TABLE `table1` (`column1` VARCHAR(255) NOT NULL, `column2` INT(11) NULL, PRIMARY KEY (`column1`)) ENGINE = InnoDB CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci; + if ($this->usingMariaDbWithUuid()) { + $collation = 'utf8mb4_unicode_520_ci'; + } else { + $collation = 'utf8mb4_0900_ai_ci'; + } + + $expectedOutput = <<out->messages()); diff --git a/tests/TestCase/MigrationsTest.php b/tests/TestCase/MigrationsTest.php index 9120d182..fc1f1e10 100644 --- a/tests/TestCase/MigrationsTest.php +++ b/tests/TestCase/MigrationsTest.php @@ -15,6 +15,7 @@ use Cake\Core\Configure; use Cake\Core\Plugin; +use Cake\Database\Driver\Mysql; use Cake\Database\Driver\Sqlserver; use Cake\Datasource\ConnectionManager; use Cake\TestSuite\TestCase; @@ -232,7 +233,10 @@ public function testMigrateAndRollback($backend) $this->assertEquals($expected, $columns); $createdColumn = $storesTable->getSchema()->getColumn('created'); $expected = 'CURRENT_TIMESTAMP'; - if ($this->Connection->getDriver() instanceof Sqlserver) { + $driver = $this->Connection->getDriver(); + if ($driver instanceof Mysql && $driver->isMariadb()) { + $expected = 'current_timestamp()'; + } elseif ($driver instanceof Sqlserver) { $expected = 'getdate()'; } $this->assertEquals($expected, $createdColumn['default']); diff --git a/tests/comparisons/Diff/addRemove/the_diff_add_remove_mysql.php b/tests/comparisons/Diff/addRemove/the_diff_add_remove_mysql.php index 2958758a..7c1543f7 100644 --- a/tests/comparisons/Diff/addRemove/the_diff_add_remove_mysql.php +++ b/tests/comparisons/Diff/addRemove/the_diff_add_remove_mysql.php @@ -5,6 +5,13 @@ class TheDiffAddRemoveMysql extends BaseMigration { + private function isMariaDB(): bool + { + $version = $this->adapter->getConnection()->getDriver()->version(); + + return version_compare($version, '10.0', '>='); + } + /** * Up Method. * @@ -29,7 +36,7 @@ public function up(): void $this->table('articles') ->addColumn('the_text', 'text', [ 'after' => 'title', - 'collation' => 'utf8mb4_0900_ai_ci', + 'collation' => $this->isMariaDB() ? 'utf8mb4_unicode_520_ci' : 'utf8mb4_0900_ai_ci', 'default' => null, 'length' => null, 'null' => false, From 57133d752b219a265139b0a7bfce4e2336186baa Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Wed, 11 Dec 2024 10:21:48 +0200 Subject: [PATCH 15/21] Unbreak mysql tests and fix one more issue with CURRENT_TIMESTAMP vs current_timestamp() --- tests/TestCase/Db/Adapter/MysqlAdapterTest.php | 4 ++-- .../Diff/addRemove/the_diff_add_remove_mysql.php | 9 +-------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 77b2cc43..1e1d07ac 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -531,11 +531,11 @@ public function testAddTimestampsFeatureFlag() $this->assertEquals('datetime', $columns[1]->getType()); $this->assertEquals('', $columns[1]->getUpdate()); $this->assertFalse($columns[1]->isNull()); - $this->assertEquals('CURRENT_TIMESTAMP', $columns[1]->getDefault()); + $this->assertContains($columns[1]->getDefault(), ['CURRENT_TIMESTAMP', 'current_timestamp()']); $this->assertEquals('updated', $columns[2]->getName()); $this->assertEquals('datetime', $columns[2]->getType()); - $this->assertEquals('CURRENT_TIMESTAMP', $columns[2]->getUpdate()); + $this->assertContains($columns[2]->getUpdate(), ['CURRENT_TIMESTAMP', 'current_timestamp()']); $this->assertTrue($columns[2]->isNull()); $this->assertNull($columns[2]->getDefault()); } diff --git a/tests/comparisons/Diff/addRemove/the_diff_add_remove_mysql.php b/tests/comparisons/Diff/addRemove/the_diff_add_remove_mysql.php index 7c1543f7..2958758a 100644 --- a/tests/comparisons/Diff/addRemove/the_diff_add_remove_mysql.php +++ b/tests/comparisons/Diff/addRemove/the_diff_add_remove_mysql.php @@ -5,13 +5,6 @@ class TheDiffAddRemoveMysql extends BaseMigration { - private function isMariaDB(): bool - { - $version = $this->adapter->getConnection()->getDriver()->version(); - - return version_compare($version, '10.0', '>='); - } - /** * Up Method. * @@ -36,7 +29,7 @@ public function up(): void $this->table('articles') ->addColumn('the_text', 'text', [ 'after' => 'title', - 'collation' => $this->isMariaDB() ? 'utf8mb4_unicode_520_ci' : 'utf8mb4_0900_ai_ci', + 'collation' => 'utf8mb4_0900_ai_ci', 'default' => null, 'length' => null, 'null' => false, From 84028854e2c2add640ace8eb809af6b9247c8081 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Wed, 11 Dec 2024 10:52:17 +0200 Subject: [PATCH 16/21] More test fixes --- tests/TestCase/Db/Adapter/MysqlAdapterTest.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 1e1d07ac..1fe5d9fb 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -80,7 +80,8 @@ private function usingMysql8(): bool { $version = $this->adapter->getConnection()->getDriver()->version(); - return version_compare($version, '8.0.0', '>='); + return version_compare($version, '8.0.0', '>=') + && version_compare($version, '10.0.0', '<'); } private function usingMariaDbWithUuid(): bool @@ -2495,7 +2496,7 @@ public function testDefaultsCastAsExpressionsForCertainTypes(string $type, strin $this->adapter->connect(); $table = new Table('table1', ['id' => false], $this->adapter); - if (!$this->usingMysql8()) { + if (!$this->usingMysql8() && !$this->usingMariaDbWithUuid()) { $this->expectException(PDOException::class); } $table @@ -2505,7 +2506,12 @@ public function testDefaultsCastAsExpressionsForCertainTypes(string $type, strin $columns = $this->adapter->getColumns('table1'); $this->assertCount(1, $columns); $this->assertSame('col_1', $columns[0]->getName()); - $this->assertSame($default, $columns[0]->getDefault()); + + if ($this->usingMariaDbWithUuid()) { + $this->assertSame("'{$default}'", $columns[0]->getDefault()); + } else { + $this->assertSame($default, $columns[0]->getDefault()); + } } public function testCreateTableWithPrecisionCurrentTimestamp() From 7e58ee4c7f73b144afe32cc2ca65a750ad960501 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Wed, 11 Dec 2024 12:00:00 +0200 Subject: [PATCH 17/21] Skip GIS defaults on MariaDB. They do not work and fixing them is out of scope for this PR. --- tests/TestCase/Db/Adapter/MysqlAdapterTest.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 1fe5d9fb..93be3bfc 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -2493,6 +2493,15 @@ public static function defaultsCastAsExpressions() #[DataProvider('defaultsCastAsExpressions')] public function testDefaultsCastAsExpressionsForCertainTypes(string $type, string $default): void { + if ($this->usingMariaDbWithUuid() && in_array($type, [ + MysqlAdapter::PHINX_TYPE_GEOMETRY, + MysqlAdapter::PHINX_TYPE_POINT, + MysqlAdapter::PHINX_TYPE_LINESTRING, + MysqlAdapter::PHINX_TYPE_POLYGON, + ])) { + $this->markTestSkipped('GIS is broken with MariaDB'); + } + $this->adapter->connect(); $table = new Table('table1', ['id' => false], $this->adapter); From 567cece917dd0e87cb7a687a437e655dc1e2981a Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Wed, 11 Dec 2024 12:04:35 +0200 Subject: [PATCH 18/21] phpcs fixes --- tests/TestCase/Db/Adapter/MysqlAdapterTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 93be3bfc..ac8ced35 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -2493,12 +2493,14 @@ public static function defaultsCastAsExpressions() #[DataProvider('defaultsCastAsExpressions')] public function testDefaultsCastAsExpressionsForCertainTypes(string $type, string $default): void { - if ($this->usingMariaDbWithUuid() && in_array($type, [ + if ( + $this->usingMariaDbWithUuid() && in_array($type, [ MysqlAdapter::PHINX_TYPE_GEOMETRY, MysqlAdapter::PHINX_TYPE_POINT, MysqlAdapter::PHINX_TYPE_LINESTRING, MysqlAdapter::PHINX_TYPE_POLYGON, - ])) { + ]) + ) { $this->markTestSkipped('GIS is broken with MariaDB'); } From dd306089fb54101a722601620c92f16c89564742 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Wed, 11 Dec 2024 12:19:37 +0200 Subject: [PATCH 19/21] Use the utf8mb4_unicode_520_ci which is present in both mysql and mariadb --- tests/comparisons/Diff/addRemove/the_diff_add_remove_mysql.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/comparisons/Diff/addRemove/the_diff_add_remove_mysql.php b/tests/comparisons/Diff/addRemove/the_diff_add_remove_mysql.php index 2958758a..f4d652a1 100644 --- a/tests/comparisons/Diff/addRemove/the_diff_add_remove_mysql.php +++ b/tests/comparisons/Diff/addRemove/the_diff_add_remove_mysql.php @@ -29,7 +29,7 @@ public function up(): void $this->table('articles') ->addColumn('the_text', 'text', [ 'after' => 'title', - 'collation' => 'utf8mb4_0900_ai_ci', + 'collation' => 'utf8mb4_unicode_520_ci', 'default' => null, 'length' => null, 'null' => false, From f9148929437f088fe852893ccbedf71aa6d6541e Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Wed, 11 Dec 2024 12:29:47 +0200 Subject: [PATCH 20/21] Fix on update current timestamp for mariadb --- src/Db/Adapter/MysqlAdapter.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 954ae353..4c3f6c53 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -424,8 +424,9 @@ public function getColumns(string $tableName): array if ($columnInfo['Extra'] === 'auto_increment') { $column->setIdentity(true); - } - if ($columnInfo['Extra'] === 'on update CURRENT_TIMESTAMP') { + } elseif ($columnInfo['Extra'] === 'on update CURRENT_TIMESTAMP') { + $column->setUpdate('CURRENT_TIMESTAMP'); + } elseif ($columnInfo['Extra'] === 'on update current_timestamp()') { $column->setUpdate('CURRENT_TIMESTAMP'); } From 117af694af6468bf9b34f66c8acc72334f92b5f2 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Wed, 11 Dec 2024 14:18:13 +0200 Subject: [PATCH 21/21] Move the default collation to a function. Fixes remaining failures with mariadb --- .../TestCase/Db/Adapter/MysqlAdapterTest.php | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index ac8ced35..bba0d28b 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -76,6 +76,13 @@ protected function tearDown(): void unset($this->adapter, $this->out, $this->io); } + private function getDefaultCollation(): string + { + return $this->usingMariaDbWithUuid() ? + 'utf8mb4_general_ci' : + 'utf8mb4_0900_ai_ci'; + } + private function usingMysql8(): bool { $version = $this->adapter->getConnection()->getDriver()->version(); @@ -427,7 +434,7 @@ public function testCreateTableAndInheritDefaultCollation() ->save(); $this->assertTrue($adapter->hasTable('table_with_default_collation')); $row = $adapter->fetchRow(sprintf("SHOW TABLE STATUS WHERE Name = '%s'", 'table_with_default_collation')); - $this->assertContains($row['Collation'], ['utf8mb4_0900_ai_ci', 'utf8mb4_unicode_520_ci']); + $this->assertEquals($row['Collation'], $this->getDefaultCollation()); } public function testCreateTableWithLatin1Collate() @@ -2182,11 +2189,7 @@ public function testDumpCreateTable() ->addColumn('column3', 'string', ['default' => 'test', 'null' => false]) ->save(); - if ($this->usingMariaDbWithUuid()) { - $collation = 'utf8mb4_unicode_520_ci'; - } else { - $collation = 'utf8mb4_0900_ai_ci'; - } + $collation = $this->getDefaultCollation(); $expectedOutput = << 1, ])->save(); - if ($this->usingMariaDbWithUuid()) { - $collation = 'utf8mb4_unicode_520_ci'; - } else { - $collation = 'utf8mb4_0900_ai_ci'; - } + $collation = $this->getDefaultCollation(); $expectedOutput = <<