Skip to content

Commit e6da2fe

Browse files
authored
Remove null collate option and convert collate to collation (#975)
When baking migration diffs, the collate option was included in column options even when null. This caused "collate is not a valid column option" errors when running migrations on PostgreSQL. Additionally, the collate to collation conversion was only done for MySQL and SQL Server. This caused the same error for PostgreSQL and SQLite columns with non-default collation. Fixes #974
1 parent ea48012 commit e6da2fe

File tree

2 files changed

+53
-5
lines changed

2 files changed

+53
-5
lines changed

src/View/Helper/MigrationHelper.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
use Cake\Core\Configure;
1818
use Cake\Database\Connection;
1919
use Cake\Database\Driver\Mysql;
20-
use Cake\Database\Driver\Sqlserver;
2120
use Cake\Database\Schema\CollectionInterface;
2221
use Cake\Database\Schema\TableSchemaInterface;
2322
use Cake\Utility\Hash;
@@ -401,18 +400,19 @@ public function getColumnOption(array $options): array
401400
if (empty($columnOptions['autoIncrement'])) {
402401
unset($columnOptions['autoIncrement']);
403402
}
403+
if (empty($columnOptions['collate'])) {
404+
unset($columnOptions['collate']);
405+
}
404406

405407
// currently only MySQL supports the signed option
406408
$driver = $connection->getDriver();
407409
$isMysql = $driver instanceof Mysql;
408-
$isSqlserver = $driver instanceof Sqlserver;
409-
410410
if (!$isMysql) {
411411
unset($columnOptions['signed']);
412412
}
413413

414-
if (($isMysql || $isSqlserver) && !empty($columnOptions['collate'])) {
415-
// due to Phinx using different naming for the collation
414+
if (!empty($columnOptions['collate'])) {
415+
// Phinx uses 'collation' not 'collate'
416416
$columnOptions['collation'] = $columnOptions['collate'];
417417
unset($columnOptions['collate']);
418418
}

tests/TestCase/View/Helper/MigrationHelperTest.php

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,4 +397,52 @@ public function testStringifyList()
397397
],
398398
]));
399399
}
400+
401+
/**
402+
* Test that getColumnOption removes null collate for all databases
403+
*
404+
* @see https://github.com/cakephp/migrations/issues/974
405+
*/
406+
public function testGetColumnOptionRemovesNullCollate(): void
407+
{
408+
$options = [
409+
'length' => 255,
410+
'null' => true,
411+
'default' => null,
412+
'collate' => null,
413+
];
414+
415+
$result = $this->helper->getColumnOption($options);
416+
417+
// collate => null should NOT be in the output for any database
418+
// because it causes "collate is not a valid column option" error
419+
$this->assertArrayNotHasKey('collate', $result, 'collate => null should be removed');
420+
$this->assertArrayNotHasKey('collation', $result, 'collation should not be set when collate is null');
421+
}
422+
423+
/**
424+
* Test that getColumnOption converts collate to collation for all databases
425+
*
426+
* Phinx uses 'collation' not 'collate', so this must be converted for any database
427+
* that supports per-column collation (MySQL, SQL Server, PostgreSQL, SQLite).
428+
*
429+
* @see https://github.com/cakephp/migrations/issues/974
430+
*/
431+
public function testGetColumnOptionConvertsCollateToCollation(): void
432+
{
433+
$options = [
434+
'length' => 255,
435+
'null' => true,
436+
'default' => null,
437+
'collate' => 'en_US.UTF-8',
438+
];
439+
440+
$result = $this->helper->getColumnOption($options);
441+
442+
// collate should be converted to collation for Phinx compatibility
443+
// This is a bug: currently only MySQL/SQLServer convert this
444+
$this->assertArrayNotHasKey('collate', $result, 'collate should be converted to collation');
445+
$this->assertArrayHasKey('collation', $result, 'collation should be set from collate value');
446+
$this->assertSame('en_US.UTF-8', $result['collation']);
447+
}
400448
}

0 commit comments

Comments
 (0)