From 80681067a5129adfa834d30886b51b9b948eeaa0 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Wed, 2 Aug 2023 15:09:34 +0700 Subject: [PATCH 01/10] Fix foreign keys --- src/Schema.php | 59 ++++++++++++++++++++------------ tests/SchemaTest.php | 30 ++++++++++++++++ tests/Support/Fixture/sqlite.sql | 20 +++++++++++ 3 files changed, 87 insertions(+), 22 deletions(-) diff --git a/src/Schema.php b/src/Schema.php index eb1286f9..d1b2d9f3 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -20,6 +20,7 @@ use Yiisoft\Db\Schema\ColumnSchemaInterface; use Yiisoft\Db\Schema\TableSchemaInterface; +use function array_column; use function array_merge; use function count; use function explode; @@ -44,7 +45,7 @@ * seq:string, * table:string, * from:string, - * to:string, + * to:string|null, * on_update:string, * on_delete:string * } @@ -58,7 +59,7 @@ * seq:string, * table:string, * from:string, - * to:string, + * to:string|null, * on_update:string, * on_delete:string * } @@ -210,15 +211,31 @@ protected function loadTableForeignKeys(string $tableName): array DbArrayHelper::multisort($foreignKeysList, 'seq'); /** @psalm-var NormalizePragmaForeignKeyList $foreignKeysList */ - foreach ($foreignKeysList as $table => $foreignKey) { - $fk = (new ForeignKeyConstraint()) - ->columnNames(DbArrayHelper::getColumn($foreignKey, 'from')) - ->foreignTableName($table) - ->foreignColumnNames(DbArrayHelper::getColumn($foreignKey, 'to')) - ->onDelete($foreignKey[0]['on_delete'] ?? null) - ->onUpdate($foreignKey[0]['on_update'] ?? null); - - $result[] = $fk; + foreach ($foreignKeysList as $table => $foreignKeys) { + $foreignKeysById = DbArrayHelper::index($foreignKeys, null, ['id']); + + /** @psalm-var NormalizePragmaForeignKeyList $foreignKeysById */ + foreach ($foreignKeysById as $id => $foreignKey) { + if ($foreignKey[0]['to'] === null) { + $primaryKey = $this->getTablePrimaryKey($table); + + if ($primaryKey !== null) { + /** @psalm-var string $primaryKeyName */ + foreach ((array) $primaryKey->getColumnNames() as $i => $primaryKeyName) { + $foreignKey[$i]['to'] = $primaryKeyName; + } + } + } + + $fk = (new ForeignKeyConstraint()) + ->columnNames(array_column($foreignKey, 'from')) + ->foreignTableName($table) + ->foreignColumnNames(array_column($foreignKey, 'to')) + ->onDelete($foreignKey[0]['on_delete']) + ->onUpdate($foreignKey[0]['on_update']); + + $result[(int) $id] = $fk; + } } return $result; @@ -380,19 +397,17 @@ protected function findColumns(TableSchemaInterface $table): bool */ protected function findConstraints(TableSchemaInterface $table): void { - /** @psalm-var PragmaForeignKeyList $foreignKeysList */ - $foreignKeysList = $this->getPragmaForeignKeyList($table->getName()); + /** @psalm-var ForeignKeyConstraint[] $foreignKeysList */ + $foreignKeysList = $this->getTableForeignKeys($table->getName(), true); - foreach ($foreignKeysList as $foreignKey) { - $id = (int) $foreignKey['id']; - $fk = $table->getForeignKeys(); + foreach ($foreignKeysList as $id => $foreignKey) { + /** @var array $columnNames */ + $columnNames = (array) $foreignKey->getColumnNames(); + $columnNames = array_combine($columnNames, $foreignKey->getForeignColumnNames()); - if (!isset($fk[$id])) { - $table->foreignKey($id, [$foreignKey['table'], $foreignKey['from'] => $foreignKey['to']]); - } else { - /** composite FK */ - $table->compositeForeignKey($id, $foreignKey['from'], $foreignKey['to']); - } + $foreignReference = array_merge([$foreignKey->getForeignTableName()], $columnNames); + + $table->foreignKey($id, $foreignReference); } } diff --git a/tests/SchemaTest.php b/tests/SchemaTest.php index cf0cb310..d9e2653c 100644 --- a/tests/SchemaTest.php +++ b/tests/SchemaTest.php @@ -162,6 +162,33 @@ public function testForeingKey(): void $this->assertSame('NO ACTION', $foreingKeys[1]->getOnUpdate()); } + public function testMultiForeingKeys(): void + { + $db = $this->getConnection(true); + $schema = $db->getSchema(); + $tableSchema = $schema->getTableSchema('foreign_keys_child'); + + $this->assertNotNull($tableSchema); + + $foreignKeys = $tableSchema->getForeignKeys(); + + $this->assertSame( + [ + [ + 'foreign_keys_parent', + 'y' => 'b', + 'z' => 'c', + ], + [ + 'foreign_keys_parent', + 'x' => 'a', + 'y' => 'b', + ], + ], + $foreignKeys + ); + } + /** * @throws Exception * @throws InvalidConfigException @@ -242,6 +269,9 @@ public function testGetTableForeignKeys(): void $this->assertSame(['C_id_1', 'C_id_2'], $tableForeingKeys[0]->getForeignColumnNames()); $this->assertSame('CASCADE', $tableForeingKeys[0]->getOnDelete()); $this->assertSame('CASCADE', $tableForeingKeys[0]->getOnUpdate()); + + $tableTwoForeignKeys = $schema->getTableForeignKeys('foreign_keys_child'); + $this->assertSame(2, count($tableTwoForeignKeys)); } /** diff --git a/tests/Support/Fixture/sqlite.sql b/tests/Support/Fixture/sqlite.sql index f8d503a0..87fcfaaf 100644 --- a/tests/Support/Fixture/sqlite.sql +++ b/tests/Support/Fixture/sqlite.sql @@ -23,6 +23,8 @@ DROP TABLE IF EXISTS "T_constraints_1"; DROP TABLE IF EXISTS "T_upsert"; DROP TABLE IF EXISTS "T_upsert_1"; DROP TABLE IF EXISTS "T_constraints_check"; +DROP TABLE IF EXISTS "foreign_keys_parent"; +DROP TABLE IF EXISTS "foreign_keys_child"; CREATE TABLE "profile" ( id INTEGER NOT NULL, @@ -288,3 +290,21 @@ CREATE TABLE "T_constraints_check" "C_check_2" INT NOT NULL CHECK ("C_check_2" > 0), CONSTRAINT "CN_constraints_check" CHECK ("C_check_1" > "C_check_2") ); + +CREATE TABLE foreign_keys_parent +( + a INTEGER, + b INTEGER, + c INTEGER, + PRIMARY KEY(a, b), + UNIQUE (b, c) +); + +CREATE TABLE foreign_keys_child +( + x INTEGER, + y INTEGER, + z INTEGER, + FOREIGN KEY(x, y) REFERENCES foreign_keys_parent, + FOREIGN KEY(y, z) REFERENCES foreign_keys_parent(b, c) +); From ef73c6bbcba95771d2f27b4a0cdee0127d3e8bf5 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Wed, 2 Aug 2023 16:03:41 +0700 Subject: [PATCH 02/10] Apply StyleCI fixes --- tests/SchemaTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/SchemaTest.php b/tests/SchemaTest.php index d9e2653c..e15c3f1b 100644 --- a/tests/SchemaTest.php +++ b/tests/SchemaTest.php @@ -271,7 +271,7 @@ public function testGetTableForeignKeys(): void $this->assertSame('CASCADE', $tableForeingKeys[0]->getOnUpdate()); $tableTwoForeignKeys = $schema->getTableForeignKeys('foreign_keys_child'); - $this->assertSame(2, count($tableTwoForeignKeys)); + $this->assertCount(2, $tableTwoForeignKeys); } /** From b6d16fe6d49f4cd8f3188ef74e9671761e26edaf Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sun, 6 Aug 2023 20:12:56 +0700 Subject: [PATCH 03/10] Apply @vjik review --- src/Schema.php | 14 ++++++++------ src/TableSchema.php | 3 +++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/Schema.php b/src/Schema.php index d1b2d9f3..c293d056 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -220,21 +220,22 @@ protected function loadTableForeignKeys(string $tableName): array $primaryKey = $this->getTablePrimaryKey($table); if ($primaryKey !== null) { - /** @psalm-var string $primaryKeyName */ - foreach ((array) $primaryKey->getColumnNames() as $i => $primaryKeyName) { - $foreignKey[$i]['to'] = $primaryKeyName; + /** @psalm-var string $primaryKeyColumnName */ + foreach ((array) $primaryKey->getColumnNames() as $i => $primaryKeyColumnName) { + $foreignKey[$i]['to'] = $primaryKeyColumnName; } } } $fk = (new ForeignKeyConstraint()) + ->name($id) ->columnNames(array_column($foreignKey, 'from')) ->foreignTableName($table) ->foreignColumnNames(array_column($foreignKey, 'to')) ->onDelete($foreignKey[0]['on_delete']) ->onUpdate($foreignKey[0]['on_update']); - $result[(int) $id] = $fk; + $result[] = $fk; } } @@ -400,14 +401,15 @@ protected function findConstraints(TableSchemaInterface $table): void /** @psalm-var ForeignKeyConstraint[] $foreignKeysList */ $foreignKeysList = $this->getTableForeignKeys($table->getName(), true); - foreach ($foreignKeysList as $id => $foreignKey) { + foreach ($foreignKeysList as $foreignKey) { /** @var array $columnNames */ $columnNames = (array) $foreignKey->getColumnNames(); $columnNames = array_combine($columnNames, $foreignKey->getForeignColumnNames()); $foreignReference = array_merge([$foreignKey->getForeignTableName()], $columnNames); - $table->foreignKey($id, $foreignReference); + /** @psalm-suppress InvalidCast */ + $table->foreignKey((int) $foreignKey->getName(), $foreignReference); } } diff --git a/src/TableSchema.php b/src/TableSchema.php index 8251dc04..8d1a7b0b 100644 --- a/src/TableSchema.php +++ b/src/TableSchema.php @@ -11,6 +11,9 @@ */ final class TableSchema extends AbstractTableSchema { + /** + * @deprecated will be removed in version 2.0.0 + */ public function compositeForeignKey(int $id, string $from, string $to): void { $this->foreignKeys[$id][$from] = $to; From 2dc0fb3a82665a55b07440213c27c9120b4c384b Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sun, 6 Aug 2023 20:19:23 +0700 Subject: [PATCH 04/10] Fix test issues --- src/Schema.php | 2 +- tests/Provider/SchemaProvider.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Schema.php b/src/Schema.php index c293d056..646248f5 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -228,7 +228,7 @@ protected function loadTableForeignKeys(string $tableName): array } $fk = (new ForeignKeyConstraint()) - ->name($id) + ->name((string) $id) ->columnNames(array_column($foreignKey, 'from')) ->foreignTableName($table) ->foreignColumnNames(array_column($foreignKey, 'to')) diff --git a/tests/Provider/SchemaProvider.php b/tests/Provider/SchemaProvider.php index 81d20231..5d4cd4fc 100644 --- a/tests/Provider/SchemaProvider.php +++ b/tests/Provider/SchemaProvider.php @@ -392,7 +392,7 @@ public static function constraints(): array $constraints['2: unique'][2][0]->name(AnyValue::getInstance()); $constraints['2: index'][2][2]->name(AnyValue::getInstance()); - $constraints['3: foreign key'][2][0]->name(null); + $constraints['3: foreign key'][2][0]->name('0'); $constraints['3: index'][2] = []; $constraints['4: primary key'][2]->name(null); From 82d35c6a90d54e2aaa9334e314cac1c837a5c3ab Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sun, 6 Aug 2023 20:22:01 +0700 Subject: [PATCH 05/10] Add line to CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d714ca7..f4af16b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 1.0.2 under development -- no changes in this release. +- Bug #268: Fix foreign keys (@Tigrov) ## 1.0.1 July 24, 2023 From 15826422a572175972b751abf2a56093e2c8dede Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sun, 6 Aug 2023 20:30:36 +0700 Subject: [PATCH 06/10] Fix psalm issue --- src/Schema.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Schema.php b/src/Schema.php index 646248f5..45455ef9 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -214,7 +214,10 @@ protected function loadTableForeignKeys(string $tableName): array foreach ($foreignKeysList as $table => $foreignKeys) { $foreignKeysById = DbArrayHelper::index($foreignKeys, null, ['id']); - /** @psalm-var NormalizePragmaForeignKeyList $foreignKeysById */ + /** + * @psalm-var NormalizePragmaForeignKeyList $foreignKeysById + * @psalm-var int $id + */ foreach ($foreignKeysById as $id => $foreignKey) { if ($foreignKey[0]['to'] === null) { $primaryKey = $this->getTablePrimaryKey($table); From 00eb8e15a32630b2f01d02ca2d4d2560673be752 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sun, 6 Aug 2023 20:32:40 +0700 Subject: [PATCH 07/10] Apply StyleCI fixes --- src/Schema.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Schema.php b/src/Schema.php index 45455ef9..a95e4705 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -214,7 +214,7 @@ protected function loadTableForeignKeys(string $tableName): array foreach ($foreignKeysList as $table => $foreignKeys) { $foreignKeysById = DbArrayHelper::index($foreignKeys, null, ['id']); - /** + /** * @psalm-var NormalizePragmaForeignKeyList $foreignKeysById * @psalm-var int $id */ From c5b49bcf528c2f183c0eb7bb7a218d9ea5e5cb37 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Sun, 6 Aug 2023 22:34:46 +0300 Subject: [PATCH 08/10] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4af16b7..309bb65a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 1.0.2 under development -- Bug #268: Fix foreign keys (@Tigrov) +- Bug #268: Fix foreign keys: support multiple foreign keys referencing to one table and possible null columns for reference (@Tigrov) ## 1.0.1 July 24, 2023 From e9f4a47297516b125fe50ed68a1237b92ce83a16 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 8 Aug 2023 18:56:00 +0700 Subject: [PATCH 09/10] Remove cast $id to int, PHP casts to int array indexes. --- src/Schema.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Schema.php b/src/Schema.php index a95e4705..63a31599 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -412,7 +412,7 @@ protected function findConstraints(TableSchemaInterface $table): void $foreignReference = array_merge([$foreignKey->getForeignTableName()], $columnNames); /** @psalm-suppress InvalidCast */ - $table->foreignKey((int) $foreignKey->getName(), $foreignReference); + $table->foreignKey($foreignKey->getName(), $foreignReference); } } From 7b0fbd727f52d82877440a177f589370b301ca81 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 8 Aug 2023 18:59:35 +0700 Subject: [PATCH 10/10] `$foreignKey->getName()` returns string in `sqlite` packet --- src/Schema.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Schema.php b/src/Schema.php index 63a31599..5cc27296 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -412,7 +412,7 @@ protected function findConstraints(TableSchemaInterface $table): void $foreignReference = array_merge([$foreignKey->getForeignTableName()], $columnNames); /** @psalm-suppress InvalidCast */ - $table->foreignKey($foreignKey->getName(), $foreignReference); + $table->foreignKey((string) $foreignKey->getName(), $foreignReference); } }