From 1acb8f490264e52992455ea57466d781cc4327a4 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sun, 5 Jul 2026 12:06:24 +0200 Subject: [PATCH] Auto-generate check constraint name when name is empty `AddCheckConstraint::build()` defaults a missing name to an empty string, but the adapters only auto-generated a name when it was `null`. Since `Cake\Database\Schema\Constraint` types the name as a non-nullable string, `getName()` never returns `null`, so the auto-generation branch was dead code and an unnamed check constraint produced invalid SQL, e.g. `ADD CONSTRAINT CHECK (...)` which PostgreSQL rejects with `SQLSTATE[42601]: Syntax error at or near "CHECK"`. Treat an empty string the same as `null` in the Postgres, MySQL and SQLite adapters so a name is generated. Add regression tests for the empty-name path and tighten the MySQL/MariaDB expectation now that migrations supplies the generated name explicitly instead of relying on the database fallback. --- src/Db/Adapter/MysqlAdapter.php | 2 +- src/Db/Adapter/PostgresAdapter.php | 2 +- src/Db/Adapter/SqliteAdapter.php | 2 +- tests/TestCase/Db/Adapter/MysqlAdapterTest.php | 6 ++++-- tests/TestCase/Db/Adapter/PostgresAdapterTest.php | 15 +++++++++++++++ tests/TestCase/Db/Adapter/SqliteAdapterTest.php | 15 +++++++++++++++ 6 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 3ebceb9e1..b3f0002bf 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -1121,7 +1121,7 @@ protected function getCheckConstraints(string $tableName): array protected function getAddCheckConstraintInstructions(TableMetadata $table, CheckConstraint $checkConstraint): AlterInstructions { $constraintName = $checkConstraint->getName(); - if ($constraintName === null) { + if ($constraintName === null || $constraintName === '') { // Auto-generate constraint name if not provided $constraintName = $table->getName() . '_chk_' . substr(md5($checkConstraint->getExpression()), 0, 8); } diff --git a/src/Db/Adapter/PostgresAdapter.php b/src/Db/Adapter/PostgresAdapter.php index 89fc010b1..84c3f1803 100644 --- a/src/Db/Adapter/PostgresAdapter.php +++ b/src/Db/Adapter/PostgresAdapter.php @@ -826,7 +826,7 @@ protected function getCheckConstraints(string $tableName): array protected function getAddCheckConstraintInstructions(TableMetadata $table, CheckConstraint $checkConstraint): AlterInstructions { $constraintName = $checkConstraint->getName(); - if ($constraintName === null) { + if ($constraintName === null || $constraintName === '') { // Auto-generate constraint name if not provided $parts = $this->getSchemaName($table->getName()); $constraintName = $parts['table'] . '_chk_' . substr(md5($checkConstraint->getExpression()), 0, 8); diff --git a/src/Db/Adapter/SqliteAdapter.php b/src/Db/Adapter/SqliteAdapter.php index 853c7ae46..9736f38f4 100644 --- a/src/Db/Adapter/SqliteAdapter.php +++ b/src/Db/Adapter/SqliteAdapter.php @@ -1546,7 +1546,7 @@ protected function getAddCheckConstraintInstructions(TableMetadata $table, Check $instructions->addPostStep(function (array $state) use ($checkConstraint): array { $constraintName = $checkConstraint->getName(); - if ($constraintName === null) { + if ($constraintName === null || $constraintName === '') { // Auto-generate constraint name if not provided $constraintName = 'chk_' . substr(md5($checkConstraint->getExpression()), 0, 8); } diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 7d26ca95b..df46e1ec0 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -2677,8 +2677,10 @@ public function testAddCheckConstraintWithAutoGeneratedName(): void $dialect = $driver->schemaDialect(); $constraints = $dialect->describeCheckConstraints('check_table2'); $this->assertCount(1, $constraints); - $expected = $driver->isMariaDb() ? 'CONSTRAINT_1' : 'check_table2_chk_'; - $this->assertStringContainsString($expected, $constraints[0]['name']); + // The name is generated by migrations (not left to the DB), so it is + // identical on MySQL and MariaDB instead of the DB fallback CONSTRAINT_1. + $expectedName = 'check_table2_chk_' . substr(md5('age >= 18'), 0, 8); + $this->assertSame($expectedName, $constraints[0]['name']); } public function testHasCheckConstraint(): void diff --git a/tests/TestCase/Db/Adapter/PostgresAdapterTest.php b/tests/TestCase/Db/Adapter/PostgresAdapterTest.php index e6a4e3eaa..831386949 100644 --- a/tests/TestCase/Db/Adapter/PostgresAdapterTest.php +++ b/tests/TestCase/Db/Adapter/PostgresAdapterTest.php @@ -2950,6 +2950,21 @@ public function testAddCheckConstraint(): void $this->assertTrue($this->adapter->hasCheckConstraint('check_table', 'price_positive')); } + public function testAddCheckConstraintAutoGeneratesName(): void + { + $table = new Table('check_table_auto', [], $this->adapter); + $table->addColumn('quantity', 'integer') + ->create(); + + $expression = 'quantity >= 0'; + // An empty name must trigger auto-generation, not emit invalid `ADD CONSTRAINT CHECK`. + $checkConstraint = new CheckConstraint('', $expression); + $this->adapter->addCheckConstraint($table->getTable(), $checkConstraint); + + $expectedName = 'check_table_auto_chk_' . substr(md5($expression), 0, 8); + $this->assertTrue($this->adapter->hasCheckConstraint('check_table_auto', $expectedName)); + } + public function testHasCheckConstraint(): void { $table = new Table('check_table3', [], $this->adapter); diff --git a/tests/TestCase/Db/Adapter/SqliteAdapterTest.php b/tests/TestCase/Db/Adapter/SqliteAdapterTest.php index 9eaa98db0..05438697b 100644 --- a/tests/TestCase/Db/Adapter/SqliteAdapterTest.php +++ b/tests/TestCase/Db/Adapter/SqliteAdapterTest.php @@ -3173,6 +3173,21 @@ public function testAddCheckConstraint(): void $this->assertTrue($this->adapter->hasCheckConstraint('check_table', 'price_positive')); } + public function testAddCheckConstraintAutoGeneratesName(): void + { + $table = new Table('check_table_auto', [], $this->adapter); + $table->addColumn('quantity', 'integer') + ->create(); + + $expression = 'quantity >= 0'; + // An empty name must trigger auto-generation, not produce an unnamed constraint. + $checkConstraint = new CheckConstraint('', $expression); + $this->adapter->addCheckConstraint($table->getTable(), $checkConstraint); + + $expectedName = 'chk_' . substr(md5($expression), 0, 8); + $this->assertTrue($this->adapter->hasCheckConstraint('check_table_auto', $expectedName)); + } + public function testHasCheckConstraint(): void { $table = new Table('check_table3', [], $this->adapter);