From b14bae6311bcf9b60286b09636e4cc0e36bbd71f Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Fri, 23 Sep 2022 13:34:02 +0300 Subject: [PATCH] Not allow ON DELETE/UPDATE SET DEFAULT actions on columns that default to sequences (#6340) Given that we drop DEFAULT nextval('sequence') expressions from shard relation columns, allowing `ON DELETE/UPDATE SET DEFAULT` on such columns might cause inserting NULL values as a result of a delete/update operation. For this reason, we disallow ON DELETE/UPDATE SET DEFAULT actions on columns that default to sequences. DESCRIPTION: Disallows having ON DELETE/UPDATE SET DEFAULT actions on columns that default to sequences Fixes #6339. (cherry picked from commit a868cc049ac8f127ffcc7897fbf87c0d1e6b6c85) Conflicts (dropped those changes since pg15 is not supported on 11.0): src/test/regress/expected/pg15.out src/test/regress/sql/pg15.sql --- .../distributed/commands/foreign_constraint.c | 119 ++++++++++++++++++ .../regress/expected/multi_foreign_key.out | 105 +++++++++++++++- .../expected/ref_citus_local_fkeys.out | 48 ++++++- src/test/regress/sql/multi_foreign_key.sql | 84 ++++++++++++- .../regress/sql/ref_citus_local_fkeys.sql | 39 ++++++ 5 files changed, 392 insertions(+), 3 deletions(-) diff --git a/src/backend/distributed/commands/foreign_constraint.c b/src/backend/distributed/commands/foreign_constraint.c index a84e8cce2..139706862 100644 --- a/src/backend/distributed/commands/foreign_constraint.c +++ b/src/backend/distributed/commands/foreign_constraint.c @@ -23,6 +23,7 @@ #include "catalog/pg_type.h" #include "distributed/colocation_utils.h" #include "distributed/commands.h" +#include "distributed/commands/sequence.h" #include "distributed/coordinator_protocol.h" #include "distributed/listutils.h" #include "distributed/coordinator_protocol.h" @@ -30,6 +31,7 @@ #include "distributed/namespace_utils.h" #include "distributed/reference_table_utils.h" #include "distributed/version_compat.h" +#include "distributed/worker_protocol.h" #include "utils/builtins.h" #include "utils/fmgroids.h" #include "utils/inval.h" @@ -56,6 +58,8 @@ typedef bool (*CheckRelationFunc)(Oid); /* Local functions forward declarations */ static void EnsureReferencingTableNotReplicated(Oid referencingTableId); static void EnsureSupportedFKeyOnDistKey(Form_pg_constraint constraintForm); +static bool ForeignKeySetsNextValColumnToDefault(HeapTuple pgConstraintTuple); +static List * ForeignKeyGetDefaultingAttrs(HeapTuple pgConstraintTuple); static void EnsureSupportedFKeyBetweenCitusLocalAndRefTable(Form_pg_constraint constraintForm, char @@ -196,6 +200,23 @@ ErrorIfUnsupportedForeignConstraintExists(Relation relation, char referencingDis referencedReplicationModel = referencingReplicationModel; } + /* + * Given that we drop DEFAULT nextval('sequence') expressions from + * shard relation columns, allowing ON DELETE/UPDATE SET DEFAULT + * on such columns causes inserting NULL values to referencing relation + * as a result of a delete/update operation on referenced relation. + * + * For this reason, we disallow ON DELETE/UPDATE SET DEFAULT actions + * on columns that default to sequences. + */ + if (ForeignKeySetsNextValColumnToDefault(heapTuple)) + { + ereport(ERROR, (errmsg("cannot create foreign key constraint " + "since Citus does not support ON DELETE " + "/ UPDATE SET DEFAULT actions on the " + "columns that default to sequences"))); + } + bool referencingIsCitusLocalOrRefTable = (referencingDistMethod == DISTRIBUTE_BY_NONE); bool referencedIsCitusLocalOrRefTable = @@ -298,6 +319,104 @@ ErrorIfUnsupportedForeignConstraintExists(Relation relation, char referencingDis } +/* + * ForeignKeySetsNextValColumnToDefault returns true if at least one of the + * columns specified in ON DELETE / UPDATE SET DEFAULT clauses default to + * nextval(). + */ +static bool +ForeignKeySetsNextValColumnToDefault(HeapTuple pgConstraintTuple) +{ + Form_pg_constraint pgConstraintForm = + (Form_pg_constraint) GETSTRUCT(pgConstraintTuple); + + List *setDefaultAttrs = ForeignKeyGetDefaultingAttrs(pgConstraintTuple); + AttrNumber setDefaultAttr = InvalidAttrNumber; + foreach_int(setDefaultAttr, setDefaultAttrs) + { + if (ColumnDefaultsToNextVal(pgConstraintForm->conrelid, setDefaultAttr)) + { + return true; + } + } + + return false; +} + + +/* + * ForeignKeyGetDefaultingAttrs returns a list of AttrNumbers + * might be set to default ON DELETE or ON UPDATE. + * + * For example; if the foreign key has SET DEFAULT clause for + * both actions, then returns a superset of the attributes that + * might be set to DEFAULT on either of those actions. + */ +static List * +ForeignKeyGetDefaultingAttrs(HeapTuple pgConstraintTuple) +{ + bool isNull = false; + Datum referencingColumnsDatum = SysCacheGetAttr(CONSTROID, pgConstraintTuple, + Anum_pg_constraint_conkey, &isNull); + if (isNull) + { + ereport(ERROR, (errmsg("got NULL conkey from catalog"))); + } + + List *referencingColumns = + IntegerArrayTypeToList(DatumGetArrayTypeP(referencingColumnsDatum)); + + Form_pg_constraint pgConstraintForm = + (Form_pg_constraint) GETSTRUCT(pgConstraintTuple); + if (pgConstraintForm->confupdtype == FKCONSTR_ACTION_SETDEFAULT) + { + /* + * Postgres doesn't allow specifying SET DEFAULT for a subset of + * the referencing columns for ON UPDATE action, so in that case + * we return all referencing columns regardless of what ON DELETE + * action says. + */ + return referencingColumns; + } + + if (pgConstraintForm->confdeltype != FKCONSTR_ACTION_SETDEFAULT) + { + return NIL; + } + + List *onDeleteSetDefColumnList = NIL; +#if PG_VERSION_NUM >= PG_VERSION_15 + Datum onDeleteSetDefColumnsDatum = SysCacheGetAttr(CONSTROID, pgConstraintTuple, + Anum_pg_constraint_confdelsetcols, + &isNull); + + /* + * confdelsetcols being NULL means that "ON DELETE SET DEFAULT" doesn't + * specify which subset of columns should be set to DEFAULT, so fetching + * NULL from the catalog is also possible. + */ + if (!isNull) + { + onDeleteSetDefColumnList = + IntegerArrayTypeToList(DatumGetArrayTypeP(onDeleteSetDefColumnsDatum)); + } +#endif + + if (list_length(onDeleteSetDefColumnList) == 0) + { + /* + * That means that all referencing columns need to be set to + * DEFAULT. + */ + return referencingColumns; + } + else + { + return onDeleteSetDefColumnList; + } +} + + /* * EnsureSupportedFKeyBetweenCitusLocalAndRefTable is a helper function that * takes a foreign key constraint form for a foreign key between two citus diff --git a/src/test/regress/expected/multi_foreign_key.out b/src/test/regress/expected/multi_foreign_key.out index e0da97847..545ba3eea 100644 --- a/src/test/regress/expected/multi_foreign_key.out +++ b/src/test/regress/expected/multi_foreign_key.out @@ -1172,5 +1172,108 @@ SELECT create_distributed_table ('dropfkeytest2', 'x', colocate_with:='none'); (1 row) +CREATE TABLE set_on_default_test_referenced( + col_1 int, col_2 int, col_3 int, col_4 int, + unique (col_1, col_3) +); +SELECT create_reference_table('set_on_default_test_referenced'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +CREATE TABLE set_on_default_test_referencing( + col_1 int, col_2 int, col_3 serial, col_4 int, + FOREIGN KEY(col_1, col_3) + REFERENCES set_on_default_test_referenced(col_1, col_3) + ON UPDATE SET DEFAULT +); +-- from distributed / reference to reference, fkey exists before calling the UDFs +SELECT create_distributed_table('set_on_default_test_referencing', 'col_1'); +ERROR: cannot create foreign key constraint since Citus does not support ON DELETE / UPDATE SET DEFAULT actions on the columns that default to sequences +SELECT create_reference_table('set_on_default_test_referencing'); +ERROR: cannot create foreign key constraint since Citus does not support ON DELETE / UPDATE SET DEFAULT actions on the columns that default to sequences +DROP TABLE set_on_default_test_referencing; +CREATE TABLE set_on_default_test_referencing( + col_1 serial, col_2 int, col_3 int, col_4 int +); +SELECT create_reference_table('set_on_default_test_referencing'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +-- from reference to reference, fkey doesn't exist before calling the UDFs +ALTER TABLE set_on_default_test_referencing ADD CONSTRAINT fkey +FOREIGN KEY(col_1, col_3) REFERENCES set_on_default_test_referenced(col_1, col_3) +ON DELETE SET DEFAULT; +ERROR: cannot create foreign key constraint since Citus does not support ON DELETE / UPDATE SET DEFAULT actions on the columns that default to sequences +DROP TABLE set_on_default_test_referencing; +CREATE TABLE set_on_default_test_referencing( + col_1 int, col_2 serial, col_3 int, col_4 bigserial +); +SELECT create_reference_table('set_on_default_test_referencing'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +-- ok since referencing columns are not based on sequences +ALTER TABLE set_on_default_test_referencing ADD CONSTRAINT fkey +FOREIGN KEY(col_1, col_3) REFERENCES set_on_default_test_referenced(col_1, col_3) +ON DELETE SET DEFAULT; +DROP TABLE set_on_default_test_referencing; +CREATE SEQUENCE test_sequence; +CREATE TABLE set_on_default_test_referencing( + col_1 int, col_2 int, col_3 int DEFAULT nextval('test_sequence'), col_4 int +); +SELECT create_distributed_table('set_on_default_test_referencing', 'col_1'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- from distributed to reference, fkey doesn't exist before calling the UDFs +ALTER TABLE set_on_default_test_referencing ADD CONSTRAINT fkey +FOREIGN KEY(col_1, col_3) REFERENCES set_on_default_test_referenced(col_1, col_3) +ON DELETE SET DEFAULT ON UPDATE SET DEFAULT; +ERROR: cannot create foreign key constraint since Citus does not support ON DELETE / UPDATE SET DEFAULT actions on the columns that default to sequences +DROP TABLE set_on_default_test_referenced; +CREATE TABLE set_on_default_test_referenced( + col_1 int, col_2 int, col_3 int, col_4 int, + unique (col_1, col_3) +); +SELECT create_distributed_table('set_on_default_test_referenced', 'col_1'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +DROP TABLE set_on_default_test_referencing; +CREATE TABLE set_on_default_test_referencing( + col_1 bigserial, col_2 int, col_3 int DEFAULT nextval('test_sequence'), col_4 int, + FOREIGN KEY(col_1, col_3) + REFERENCES set_on_default_test_referenced(col_1, col_3) + ON DELETE SET DEFAULT +); +-- from distributed to distributed, fkey exists before calling the UDFs +SELECT create_distributed_table('set_on_default_test_referencing', 'col_1'); +ERROR: cannot create foreign key constraint since Citus does not support ON DELETE / UPDATE SET DEFAULT actions on the columns that default to sequences +DROP TABLE set_on_default_test_referencing; +CREATE TABLE set_on_default_test_referencing( + col_1 int DEFAULT nextval('test_sequence'), col_2 int, col_3 int, col_4 int +); +SELECT create_distributed_table('set_on_default_test_referencing', 'col_1'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- from distributed to distributed, fkey doesn't exist before calling the UDFs +ALTER TABLE set_on_default_test_referencing ADD CONSTRAINT fkey +FOREIGN KEY(col_1, col_3) REFERENCES set_on_default_test_referenced(col_1, col_3) +ON DELETE SET DEFAULT; +ERROR: cannot create foreign key constraint since Citus does not support ON DELETE / UPDATE SET DEFAULT actions on the columns that default to sequences -- we no longer need those tables -DROP TABLE referenced_by_reference_table, references_to_reference_table, reference_table, reference_table_second, referenced_local_table, self_referencing_reference_table, dropfkeytest2; +DROP TABLE referenced_by_reference_table, references_to_reference_table, reference_table, reference_table_second, referenced_local_table, self_referencing_reference_table, dropfkeytest2, + set_on_default_test_referenced, set_on_default_test_referencing; diff --git a/src/test/regress/expected/ref_citus_local_fkeys.out b/src/test/regress/expected/ref_citus_local_fkeys.out index daf4aeca6..483f23120 100644 --- a/src/test/regress/expected/ref_citus_local_fkeys.out +++ b/src/test/regress/expected/ref_citus_local_fkeys.out @@ -250,6 +250,52 @@ BEGIN; CREATE TABLE referencing_table(id int, ref_id int, FOREIGN KEY(ref_id) REFERENCES referenced_table(id) ON DELETE SET DEFAULT); ERROR: cannot switch local execution status from local execution disabled to local execution enabled since it can cause visibility problems in the current transaction ROLLBACK; +CREATE TABLE set_on_default_test_referenced( + col_1 int, col_2 int, col_3 int, col_4 int, + unique (col_1, col_3) +); +SELECT create_reference_table('set_on_default_test_referenced'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +-- from citus local to reference - 1 +CREATE TABLE set_on_default_test_referencing( + col_1 int, col_2 int, col_3 serial, col_4 int, + FOREIGN KEY(col_1, col_3) + REFERENCES set_on_default_test_referenced(col_1, col_3) + ON UPDATE SET DEFAULT +); +ERROR: cannot create foreign key constraint since Citus does not support ON DELETE / UPDATE SET DEFAULT actions on the columns that default to sequences +CREATE TABLE set_on_default_test_referencing( + col_1 serial, col_2 int, col_3 int, col_4 int +); +-- from citus local to reference - 2 +ALTER TABLE set_on_default_test_referencing ADD CONSTRAINT fkey +FOREIGN KEY(col_1, col_3) REFERENCES set_on_default_test_referenced(col_1, col_3) +ON DELETE SET DEFAULT; +ERROR: cannot create foreign key constraint since Citus does not support ON DELETE / UPDATE SET DEFAULT actions on the columns that default to sequences +DROP TABLE set_on_default_test_referencing, set_on_default_test_referenced; +NOTICE: executing the command locally: DROP TABLE IF EXISTS ref_citus_local_fkeys.set_on_default_test_referenced_xxxxx CASCADE +CREATE TABLE set_on_default_test_referenced( + col_1 int, col_2 int, col_3 int, col_4 int, + unique (col_1, col_3) +); +SELECT citus_add_local_table_to_metadata('set_on_default_test_referenced'); + citus_add_local_table_to_metadata +--------------------------------------------------------------------- + +(1 row) + +-- from citus local to citus local +CREATE TABLE set_on_default_test_referencing( + col_1 int, col_2 int, col_3 serial, col_4 int, + FOREIGN KEY(col_1, col_3) + REFERENCES set_on_default_test_referenced(col_1, col_3) + ON DELETE SET DEFAULT +); +ERROR: cannot create foreign key constraint since Citus does not support ON DELETE / UPDATE SET DEFAULT actions on the columns that default to sequences -- cleanup at exit DROP SCHEMA ref_citus_local_fkeys CASCADE; -NOTICE: drop cascades to 6 other objects +NOTICE: drop cascades to 8 other objects diff --git a/src/test/regress/sql/multi_foreign_key.sql b/src/test/regress/sql/multi_foreign_key.sql index 7830b3bb7..f39f0ace4 100644 --- a/src/test/regress/sql/multi_foreign_key.sql +++ b/src/test/regress/sql/multi_foreign_key.sql @@ -696,5 +696,87 @@ DROP TABLE dropfkeytest1 CASCADE; -- this should work SELECT create_distributed_table ('dropfkeytest2', 'x', colocate_with:='none'); +CREATE TABLE set_on_default_test_referenced( + col_1 int, col_2 int, col_3 int, col_4 int, + unique (col_1, col_3) +); +SELECT create_reference_table('set_on_default_test_referenced'); + +CREATE TABLE set_on_default_test_referencing( + col_1 int, col_2 int, col_3 serial, col_4 int, + FOREIGN KEY(col_1, col_3) + REFERENCES set_on_default_test_referenced(col_1, col_3) + ON UPDATE SET DEFAULT +); + +-- from distributed / reference to reference, fkey exists before calling the UDFs +SELECT create_distributed_table('set_on_default_test_referencing', 'col_1'); +SELECT create_reference_table('set_on_default_test_referencing'); + +DROP TABLE set_on_default_test_referencing; +CREATE TABLE set_on_default_test_referencing( + col_1 serial, col_2 int, col_3 int, col_4 int +); +SELECT create_reference_table('set_on_default_test_referencing'); + +-- from reference to reference, fkey doesn't exist before calling the UDFs +ALTER TABLE set_on_default_test_referencing ADD CONSTRAINT fkey +FOREIGN KEY(col_1, col_3) REFERENCES set_on_default_test_referenced(col_1, col_3) +ON DELETE SET DEFAULT; + +DROP TABLE set_on_default_test_referencing; +CREATE TABLE set_on_default_test_referencing( + col_1 int, col_2 serial, col_3 int, col_4 bigserial +); +SELECT create_reference_table('set_on_default_test_referencing'); + +-- ok since referencing columns are not based on sequences +ALTER TABLE set_on_default_test_referencing ADD CONSTRAINT fkey +FOREIGN KEY(col_1, col_3) REFERENCES set_on_default_test_referenced(col_1, col_3) +ON DELETE SET DEFAULT; + +DROP TABLE set_on_default_test_referencing; + +CREATE SEQUENCE test_sequence; +CREATE TABLE set_on_default_test_referencing( + col_1 int, col_2 int, col_3 int DEFAULT nextval('test_sequence'), col_4 int +); +SELECT create_distributed_table('set_on_default_test_referencing', 'col_1'); + +-- from distributed to reference, fkey doesn't exist before calling the UDFs +ALTER TABLE set_on_default_test_referencing ADD CONSTRAINT fkey +FOREIGN KEY(col_1, col_3) REFERENCES set_on_default_test_referenced(col_1, col_3) +ON DELETE SET DEFAULT ON UPDATE SET DEFAULT; + +DROP TABLE set_on_default_test_referenced; +CREATE TABLE set_on_default_test_referenced( + col_1 int, col_2 int, col_3 int, col_4 int, + unique (col_1, col_3) +); +SELECT create_distributed_table('set_on_default_test_referenced', 'col_1'); + +DROP TABLE set_on_default_test_referencing; +CREATE TABLE set_on_default_test_referencing( + col_1 bigserial, col_2 int, col_3 int DEFAULT nextval('test_sequence'), col_4 int, + FOREIGN KEY(col_1, col_3) + REFERENCES set_on_default_test_referenced(col_1, col_3) + ON DELETE SET DEFAULT +); + +-- from distributed to distributed, fkey exists before calling the UDFs +SELECT create_distributed_table('set_on_default_test_referencing', 'col_1'); + +DROP TABLE set_on_default_test_referencing; +CREATE TABLE set_on_default_test_referencing( + col_1 int DEFAULT nextval('test_sequence'), col_2 int, col_3 int, col_4 int +); +SELECT create_distributed_table('set_on_default_test_referencing', 'col_1'); + +-- from distributed to distributed, fkey doesn't exist before calling the UDFs +ALTER TABLE set_on_default_test_referencing ADD CONSTRAINT fkey +FOREIGN KEY(col_1, col_3) REFERENCES set_on_default_test_referenced(col_1, col_3) +ON DELETE SET DEFAULT; + -- we no longer need those tables -DROP TABLE referenced_by_reference_table, references_to_reference_table, reference_table, reference_table_second, referenced_local_table, self_referencing_reference_table, dropfkeytest2; +DROP TABLE referenced_by_reference_table, references_to_reference_table, reference_table, reference_table_second, referenced_local_table, self_referencing_reference_table, dropfkeytest2, + set_on_default_test_referenced, set_on_default_test_referencing; diff --git a/src/test/regress/sql/ref_citus_local_fkeys.sql b/src/test/regress/sql/ref_citus_local_fkeys.sql index 76028bc1a..70daae0fa 100644 --- a/src/test/regress/sql/ref_citus_local_fkeys.sql +++ b/src/test/regress/sql/ref_citus_local_fkeys.sql @@ -159,5 +159,44 @@ BEGIN; CREATE TABLE referencing_table(id int, ref_id int, FOREIGN KEY(ref_id) REFERENCES referenced_table(id) ON DELETE SET DEFAULT); ROLLBACK; +CREATE TABLE set_on_default_test_referenced( + col_1 int, col_2 int, col_3 int, col_4 int, + unique (col_1, col_3) +); +SELECT create_reference_table('set_on_default_test_referenced'); + +-- from citus local to reference - 1 +CREATE TABLE set_on_default_test_referencing( + col_1 int, col_2 int, col_3 serial, col_4 int, + FOREIGN KEY(col_1, col_3) + REFERENCES set_on_default_test_referenced(col_1, col_3) + ON UPDATE SET DEFAULT +); + +CREATE TABLE set_on_default_test_referencing( + col_1 serial, col_2 int, col_3 int, col_4 int +); + +-- from citus local to reference - 2 +ALTER TABLE set_on_default_test_referencing ADD CONSTRAINT fkey +FOREIGN KEY(col_1, col_3) REFERENCES set_on_default_test_referenced(col_1, col_3) +ON DELETE SET DEFAULT; + +DROP TABLE set_on_default_test_referencing, set_on_default_test_referenced; + +CREATE TABLE set_on_default_test_referenced( + col_1 int, col_2 int, col_3 int, col_4 int, + unique (col_1, col_3) +); +SELECT citus_add_local_table_to_metadata('set_on_default_test_referenced'); + +-- from citus local to citus local +CREATE TABLE set_on_default_test_referencing( + col_1 int, col_2 int, col_3 serial, col_4 int, + FOREIGN KEY(col_1, col_3) + REFERENCES set_on_default_test_referenced(col_1, col_3) + ON DELETE SET DEFAULT +); + -- cleanup at exit DROP SCHEMA ref_citus_local_fkeys CASCADE;