Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added nativeuuid type for databases with a native uuid type #786

Merged
merged 21 commits into from
Dec 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
fa33f6d
Added nativeuuid type for databases with a native uuid type
nicosp Dec 10, 2024
f7dca12
Added tests to postgres to ensure that nativeuuid behaves exactly lik…
nicosp Dec 10, 2024
8f9634f
Add the native uuid type to specificColumnTypes to allow usage as pri…
nicosp Dec 10, 2024
a52c423
Remove incorrect test assertion
nicosp Dec 10, 2024
5506ee0
Properly handle converting from text to binary and native uuid columns
nicosp Dec 10, 2024
264332f
Enable the nativeuuid type conditionally.
nicosp Dec 10, 2024
dd4288b
Fix test assertion
nicosp Dec 10, 2024
781387c
Check for uuid support in getSqlType as well
nicosp Dec 10, 2024
8303d1a
Coding style fix
nicosp Dec 10, 2024
b042de9
Map nativeuuid phinx type when the database type is uuid
nicosp Dec 10, 2024
aebd91d
Added MariaDB to ci
nicosp Dec 11, 2024
45b3983
Split the creation of extra mariadb databases in another step
nicosp Dec 11, 2024
2577870
use the mysql command which actually exists
nicosp Dec 11, 2024
0c70293
Test fixes for mariadb
nicosp Dec 11, 2024
57133d7
Unbreak mysql tests and fix one more issue with CURRENT_TIMESTAMP vs …
nicosp Dec 11, 2024
8402885
More test fixes
nicosp Dec 11, 2024
7e58ee4
Skip GIS defaults on MariaDB. They do not work and fixing them is out…
nicosp Dec 11, 2024
567cece
phpcs fixes
nicosp Dec 11, 2024
dd30608
Use the utf8mb4_unicode_520_ci which is present in both mysql and mar…
nicosp Dec 11, 2024
f914892
Fix on update current timestamp for mariadb
nicosp Dec 11, 2024
117af69
Move the default collation to a function. Fixes remaining failures wi…
nicosp Dec 11, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -50,6 +50,19 @@ jobs:
steps:
- uses: actions/checkout@v4

- name: Setup MariaDB
uses: getong/[email protected]
if: matrix.db-type == 'mariadb'
with:
mariadb version: '10.11.10'
mysql database: 'cakephp_test'
mysql root password: 'root'
- name: Setup MariaDB (part 2)
if: matrix.db-type == 'mariadb'
run: |
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'
run: |
Expand Down Expand Up @@ -106,6 +119,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:[email protected]/cakephp_test'
export DB_URL_COMPARE='mysql://root:[email protected]/cakephp_comparisons'
export DB_URL_SNAPSHOT='mysql://root:[email protected]/cakephp_snapshot'
fi
if [[ ${{ matrix.db-type }} == 'mysql' ]]; then
export DB='mysql'
export DB_URL='mysql://root:[email protected]/cakephp_test'
Expand Down
1 change: 1 addition & 0 deletions src/Db/Adapter/AdapterInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hoping to remove all of these constants in the future and use the abstract types defined in CakePHP. We'll have to make sure that the same abstract type names are used so that we don't break compatibility.

public const PHINX_TYPE_FILESTREAM = 'filestream';

// Geospatial database types
Expand Down
39 changes: 36 additions & 3 deletions src/Db/Adapter/MysqlAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -424,8 +424,9 @@

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');
}

Expand Down Expand Up @@ -1061,6 +1062,14 @@
return ['name' => 'tinyint', 'limit' => 1];
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'];

Check warning on line 1072 in src/Db/Adapter/MysqlAdapter.php

View check run for this annotation

Codecov / codecov/patch

src/Db/Adapter/MysqlAdapter.php#L1072

Added line #L1072 was not covered by tests
case static::PHINX_TYPE_YEAR:
if (!$limit || in_array($limit, [2, 4])) {
$limit = 4;
Expand Down Expand Up @@ -1179,6 +1188,10 @@
$type = static::PHINX_TYPE_BINARYUUID;
}
break;
case 'uuid':
$type = static::PHINX_TYPE_NATIVEUUID;
$limit = null;
break;

Check warning on line 1194 in src/Db/Adapter/MysqlAdapter.php

View check run for this annotation

Codecov / codecov/patch

src/Db/Adapter/MysqlAdapter.php#L1192-L1194

Added lines #L1192 - L1194 were not covered by tests
}

try {
Expand Down Expand Up @@ -1491,6 +1504,26 @@
*/
public function getColumnTypes(): array
{
return array_merge(parent::getColumnTypes(), static::$specificColumnTypes);
$types = array_merge(parent::getColumnTypes(), static::$specificColumnTypes);

if ($this->hasNativeUuid()) {
$types[] = self::PHINX_TYPE_NATIVEUUID;

Check warning on line 1510 in src/Db/Adapter/MysqlAdapter.php

View check run for this annotation

Codecov / codecov/patch

src/Db/Adapter/MysqlAdapter.php#L1510

Added line #L1510 was not covered by tests
}

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', '>=');
}
}
4 changes: 3 additions & 1 deletion src/Db/Adapter/PostgresAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
self::PHINX_TYPE_MACADDR,
self::PHINX_TYPE_INTERVAL,
self::PHINX_TYPE_BINARYUUID,
self::PHINX_TYPE_NATIVEUUID,
];

private const GIN_INDEX_TYPE = 'gin';
Expand Down Expand Up @@ -548,7 +549,7 @@
$quotedColumnName
);
}
if ($newColumn->getType() === 'uuid') {
if (in_array($newColumn->getType(), ['uuid', 'nativeuuid', 'binaryuuid'])) {

Check warning on line 552 in src/Db/Adapter/PostgresAdapter.php

View check run for this annotation

Codecov / codecov/patch

src/Db/Adapter/PostgresAdapter.php#L552

Added line #L552 was not covered by tests
$sql .= sprintf(
' USING (%s::uuid)',
$quotedColumnName
Expand Down Expand Up @@ -1023,6 +1024,7 @@
case static::PHINX_TYPE_DATETIME:
return ['name' => 'timestamp'];
case static::PHINX_TYPE_BINARYUUID:
case static::PHINX_TYPE_NATIVEUUID:

Check warning on line 1027 in src/Db/Adapter/PostgresAdapter.php

View check run for this annotation

Codecov / codecov/patch

src/Db/Adapter/PostgresAdapter.php#L1027

Added line #L1027 was not covered by tests
return ['name' => 'uuid'];
case static::PHINX_TYPE_BLOB:
case static::PHINX_TYPE_BINARY:
Expand Down
2 changes: 2 additions & 0 deletions src/Db/Adapter/SqlserverAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class SqlserverAdapter extends PdoAdapter
protected static array $specificColumnTypes = [
self::PHINX_TYPE_FILESTREAM,
self::PHINX_TYPE_BINARYUUID,
self::PHINX_TYPE_NATIVEUUID,
];

/**
Expand Down Expand Up @@ -1025,6 +1026,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'];
Expand Down
1 change: 1 addition & 0 deletions src/Db/Table/Column.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
85 changes: 75 additions & 10 deletions tests/TestCase/Db/Adapter/MysqlAdapterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -75,11 +76,26 @@ 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();

return version_compare($version, '8.0.0', '>=');
return version_compare($version, '8.0.0', '>=')
&& version_compare($version, '10.0.0', '<');
}

private function usingMariaDbWithUuid(): bool
{
$version = $this->adapter->getConnection()->getDriver()->version();

return version_compare($version, '10.7.0', '>=');
}

public function testConnection()
Expand Down Expand Up @@ -329,6 +345,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);
Expand Down Expand Up @@ -397,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->assertEquals('utf8mb4_0900_ai_ci', $row['Collation']);
$this->assertEquals($row['Collation'], $this->getDefaultCollation());
}

public function testCreateTableWithLatin1Collate()
Expand Down Expand Up @@ -502,11 +539,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());
}
Expand Down Expand Up @@ -2152,8 +2189,10 @@ 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;
$collation = $this->getDefaultCollation();

$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 {$collation};
OUTPUT;
$actualOutput = join("\n", $this->out->messages());
$this->assertStringContainsString($expectedOutput, $actualOutput, 'Passing the --dry-run option does not dump create table query to the output');
Expand Down Expand Up @@ -2257,8 +2296,10 @@ 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;
$collation = $this->getDefaultCollation();

$expectedOutput = <<<OUTPUT
CREATE TABLE `table1` (`column1` VARCHAR(255) NOT NULL, `column2` INT(11) NULL, PRIMARY KEY (`column1`)) ENGINE = InnoDB CHARACTER SET utf8mb4 COLLATE {$collation};
INSERT INTO `table1` (`column1`, `column2`) VALUES ('id1', 1);
OUTPUT;
$actualOutput = join("\n", $this->out->messages());
Expand Down Expand Up @@ -2451,10 +2492,21 @@ 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);
if (!$this->usingMysql8()) {
if (!$this->usingMysql8() && !$this->usingMariaDbWithUuid()) {
$this->expectException(PDOException::class);
}
$table
Expand All @@ -2464,7 +2516,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()
Expand Down Expand Up @@ -2522,4 +2579,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 testGetSqlType()
{
if (!$this->usingMariaDbWithUuid()) {
$this->expectException(UnsupportedColumnTypeException::class);
}
$this->assertSame(['name' => 'uuid'], $this->adapter->getSqlType('nativeuuid'));
}
}
35 changes: 35 additions & 0 deletions tests/TestCase/Db/Adapter/PostgresAdapterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion tests/TestCase/MigrationsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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']);
Expand Down
Loading
Loading