From 76608b130757e769c3e7b4fe77e9395ab243a482 Mon Sep 17 00:00:00 2001 From: Nils Dijk Date: Mon, 22 Mar 2021 15:30:29 +0100 Subject: [PATCH] Fix foreign key non colocated (#4840) DESCRIPTION: Fix a bug where one could create a foreign key between non-colocated tables Due to the scoping and only setting the variable to true when matched, not false when not matched, it was possible for a foreign key to be created on non-colocated tables. This patch changes the scope and uses an always assign the test result to the variable we prevent users from creating foreign keys between non-colocated tables. --- .../distributed/commands/foreign_constraint.c | 14 +++---- .../foreign_key_to_reference_table.out | 38 ++++++++++++++++++- .../sql/foreign_key_to_reference_table.sql | 19 ++++++++++ 3 files changed, 62 insertions(+), 9 deletions(-) diff --git a/src/backend/distributed/commands/foreign_constraint.c b/src/backend/distributed/commands/foreign_constraint.c index f9bdc99c4..91c5a8ca9 100644 --- a/src/backend/distributed/commands/foreign_constraint.c +++ b/src/backend/distributed/commands/foreign_constraint.c @@ -134,10 +134,6 @@ ErrorIfUnsupportedForeignConstraint(Relation relation, char distributionMethod, int referencedColumnCount = 0; bool isNull = false; int attrIdx = 0; - bool foreignConstraintOnPartitionColumn = false; - bool selfReferencingTable = false; - bool referencedTableIsAReferenceTable = false; - bool referencingColumnsIncludeDistKey = false; pgConstraint = heap_open(ConstraintRelationId, AccessShareLock); ScanKeyInit(&scanKey[0], Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, @@ -148,6 +144,10 @@ ErrorIfUnsupportedForeignConstraint(Relation relation, char distributionMethod, heapTuple = systable_getnext(scanDescriptor); while (HeapTupleIsValid(heapTuple)) { + bool foreignConstraintOnPartitionColumn = false; + bool selfReferencingTable = false; + bool referencedTableIsAReferenceTable = false; + bool referencingColumnsIncludeDistKey = false; Form_pg_constraint constraintForm = (Form_pg_constraint) GETSTRUCT(heapTuple); bool singleReplicatedTable = true; @@ -196,10 +196,8 @@ ErrorIfUnsupportedForeignConstraint(Relation relation, char distributionMethod, * tables. This is why we make this check under !selfReferencingTable * and after !IsDistributedTable(referencedTableId). */ - if (PartitionMethod(referencedTableId) == DISTRIBUTE_BY_NONE) - { - referencedTableIsAReferenceTable = true; - } + referencedTableIsAReferenceTable = + (PartitionMethod(referencedTableId) == DISTRIBUTE_BY_NONE); /* * To enforce foreign constraints, tables must be co-located unless a diff --git a/src/test/regress/expected/foreign_key_to_reference_table.out b/src/test/regress/expected/foreign_key_to_reference_table.out index 8b73fa63d..1f6aadca3 100644 --- a/src/test/regress/expected/foreign_key_to_reference_table.out +++ b/src/test/regress/expected/foreign_key_to_reference_table.out @@ -1886,9 +1886,45 @@ ROLLBACK; DROP TABLE referenced_table CASCADE; NOTICE: drop cascades to constraint fkey_to_ref on table referencing_table_4 DROP TABLE referencing_table; +-- tests specific to an edgecase in citus 8.x where it was possible to create foreign keys +-- in between colocation groups due to a bug of foreign key to reference tables +CREATE TABLE t1 (a int PRIMARY KEY, b text); +CREATE TABLE t2 (a bigint PRIMARY KEY, b text); +CREATE TABLE r1 (a int PRIMARY KEY, b text); +SELECT create_distributed_table('t1', 'a'); + create_distributed_table +-------------------------- + +(1 row) + +SELECT create_distributed_table('t2', 'a'); + create_distributed_table +-------------------------- + +(1 row) + +SELECT create_reference_table('r1'); + create_reference_table +------------------------ + +(1 row) + +-- this always fails as it should be +ALTER TABLE t1 ADD CONSTRAINT c1 FOREIGN KEY (a) REFERENCES t2(a); +ERROR: cannot create foreign key constraint since relations are not colocated or not referencing a reference table +DETAIL: A distributed table can only have foreign keys if it is referencing another colocated hash distributed table or a reference table +-- after we create a foreign key to the reference table, that has a lower order by name, +-- we would have been able to create a FK to non-colocated tables +ALTER TABLE t1 ADD CONSTRAINT c2 FOREIGN KEY (a) REFERENCES r1(a); +ALTER TABLE t1 ADD CONSTRAINT c3 FOREIGN KEY (a) REFERENCES t2(a); +ERROR: cannot create foreign key constraint since relations are not colocated or not referencing a reference table +DETAIL: A distributed table can only have foreign keys if it is referencing another colocated hash distributed table or a reference table DROP SCHEMA fkey_reference_table CASCADE; -NOTICE: drop cascades to 3 other objects +NOTICE: drop cascades to 6 other objects DETAIL: drop cascades to type foreign_details drop cascades to view table_fkeys_in_workers drop cascades to type composite +drop cascades to table t1 +drop cascades to table t2 +drop cascades to table r1 SET search_path TO DEFAULT; diff --git a/src/test/regress/sql/foreign_key_to_reference_table.sql b/src/test/regress/sql/foreign_key_to_reference_table.sql index 07924bdb5..38e84d9e8 100644 --- a/src/test/regress/sql/foreign_key_to_reference_table.sql +++ b/src/test/regress/sql/foreign_key_to_reference_table.sql @@ -951,5 +951,24 @@ ROLLBACK; DROP TABLE referenced_table CASCADE; DROP TABLE referencing_table; +-- tests specific to an edgecase in citus 8.x where it was possible to create foreign keys +-- in between colocation groups due to a bug of foreign key to reference tables +CREATE TABLE t1 (a int PRIMARY KEY, b text); +CREATE TABLE t2 (a bigint PRIMARY KEY, b text); +CREATE TABLE r1 (a int PRIMARY KEY, b text); + +SELECT create_distributed_table('t1', 'a'); +SELECT create_distributed_table('t2', 'a'); +SELECT create_reference_table('r1'); + +-- this always fails as it should be +ALTER TABLE t1 ADD CONSTRAINT c1 FOREIGN KEY (a) REFERENCES t2(a); + +-- after we create a foreign key to the reference table, that has a lower order by name, +-- we would have been able to create a FK to non-colocated tables +ALTER TABLE t1 ADD CONSTRAINT c2 FOREIGN KEY (a) REFERENCES r1(a); +ALTER TABLE t1 ADD CONSTRAINT c3 FOREIGN KEY (a) REFERENCES t2(a); + + DROP SCHEMA fkey_reference_table CASCADE; SET search_path TO DEFAULT;