diff --git a/src/backend/distributed/commands/citus_add_local_table_to_metadata.c b/src/backend/distributed/commands/citus_add_local_table_to_metadata.c index 83f542492..6353df0b0 100644 --- a/src/backend/distributed/commands/citus_add_local_table_to_metadata.c +++ b/src/backend/distributed/commands/citus_add_local_table_to_metadata.c @@ -87,8 +87,8 @@ static List * ReversedOidList(List *oidList); static void AppendExplicitIndexIdsToList(Form_pg_index indexForm, List **explicitIndexIdList, int flags); -static void DropDefaultExpressionsAndMoveOwnedSequenceOwnerships(Oid sourceRelationId, - Oid targetRelationId); +static void DropNextValExprsAndMoveOwnedSeqOwnerships(Oid sourceRelationId, + Oid targetRelationId); static void DropDefaultColumnDefinition(Oid relationId, char *columnName); static void TransferSequenceOwnership(Oid ownedSequenceId, Oid targetRelationId, char *columnName); @@ -366,11 +366,11 @@ CreateCitusLocalTable(Oid relationId, bool cascadeViaForeignKeys, bool autoConve /* * Move sequence ownerships from shard table to shell table and also drop - * DEFAULT expressions from shard relation as we should evaluate such columns - * in shell table when needed. + * DEFAULT expressions based on sequences from shard relation as we should + * evaluate such columns in shell table when needed. */ - DropDefaultExpressionsAndMoveOwnedSequenceOwnerships(shardRelationId, - shellRelationId); + DropNextValExprsAndMoveOwnedSeqOwnerships(shardRelationId, + shellRelationId); InsertMetadataForCitusLocalTable(shellRelationId, shardId, autoConverted); @@ -1161,14 +1161,15 @@ GetRenameStatsCommandList(List *statsOidList, uint64 shardId) /* - * DropDefaultExpressionsAndMoveOwnedSequenceOwnerships drops default column - * definitions for relation with sourceRelationId. Also, for each column that - * defaults to an owned sequence, it grants ownership to the same named column - * of the relation with targetRelationId. + * DropNextValExprsAndMoveOwnedSeqOwnerships drops default column definitions + * that are based on sequences for relation with sourceRelationId. + * + * Also, for each such column that owns a sequence, it grants ownership to the + * same named column of the relation with targetRelationId. */ static void -DropDefaultExpressionsAndMoveOwnedSequenceOwnerships(Oid sourceRelationId, - Oid targetRelationId) +DropNextValExprsAndMoveOwnedSeqOwnerships(Oid sourceRelationId, + Oid targetRelationId) { List *columnNameList = NIL; List *ownedSequenceIdList = NIL; @@ -1179,9 +1180,28 @@ DropDefaultExpressionsAndMoveOwnedSequenceOwnerships(Oid sourceRelationId, Oid ownedSequenceId = InvalidOid; forboth_ptr_oid(columnName, columnNameList, ownedSequenceId, ownedSequenceIdList) { - DropDefaultColumnDefinition(sourceRelationId, columnName); + /* + * We drop nextval() expressions because Citus currently evaluates + * nextval() on the shell table, not on the shards. Hence, there is + * no reason for keeping nextval(). Also, distributed/reference table + * shards do not have - so be consistent with those. + * + * Note that we keep other kind of DEFAULT expressions on shards + * because we still want to be able to evaluate DEFAULT expressions + * that are not based on sequences on shards, e.g., for foreign key + * - SET DEFAULT actions. + */ + AttrNumber columnAttrNumber = get_attnum(sourceRelationId, columnName); + if (ColumnDefaultsToNextVal(sourceRelationId, columnAttrNumber)) + { + DropDefaultColumnDefinition(sourceRelationId, columnName); + } - /* column might not own a sequence */ + /* + * Column might own a sequence without having a nextval() expr on it + * --e.g., due to ALTER SEQUENCE OWNED BY .. --, so check if that is + * the case even if the column doesn't have a DEFAULT. + */ if (OidIsValid(ownedSequenceId)) { TransferSequenceOwnership(ownedSequenceId, targetRelationId, columnName); diff --git a/src/backend/distributed/commands/sequence.c b/src/backend/distributed/commands/sequence.c index 20b7666ad..a37b0f267 100644 --- a/src/backend/distributed/commands/sequence.c +++ b/src/backend/distributed/commands/sequence.c @@ -27,6 +27,7 @@ #include "nodes/makefuncs.h" #include "distributed/worker_create_or_replace.h" #include "nodes/parsenodes.h" +#include "rewrite/rewriteHandler.h" #include "utils/builtins.h" #include "utils/lsyscache.h" @@ -213,6 +214,29 @@ ExtractDefaultColumnsAndOwnedSequences(Oid relationId, List **columnNameList, } +/* + * ColumnDefaultsToNextVal returns true if the column with attrNumber + * has a default expression that contains nextval(). + */ +bool +ColumnDefaultsToNextVal(Oid relationId, AttrNumber attrNumber) +{ + AssertArg(AttributeNumberIsValid(attrNumber)); + + Relation relation = RelationIdGetRelation(relationId); + Node *defExpr = build_column_default(relation, attrNumber); + RelationClose(relation); + + if (defExpr == NULL) + { + /* column doesn't have a DEFAULT expression */ + return false; + } + + return contain_nextval_expression_walker(defExpr, NULL); +} + + /* * PreprocessDropSequenceStmt gets called during the planning phase of a DROP SEQUENCE statement * and returns a list of DDLJob's that will drop any distributed sequences from the diff --git a/src/include/distributed/commands/sequence.h b/src/include/distributed/commands/sequence.h index 5c39df1ac..c6708a4e0 100644 --- a/src/include/distributed/commands/sequence.h +++ b/src/include/distributed/commands/sequence.h @@ -10,9 +10,11 @@ #ifndef CITUS_SEQUENCE_H #define CITUS_SEQUENCE_H +#include "access/attnum.h" #include "nodes/pg_list.h" +extern bool ColumnDefaultsToNextVal(Oid relationId, AttrNumber attrNumber); extern void ExtractDefaultColumnsAndOwnedSequences(Oid relationId, List **columnNameList, List **ownedSequenceIdList); diff --git a/src/test/regress/expected/pg15.out b/src/test/regress/expected/pg15.out index ed9a0622a..f2d03077f 100644 --- a/src/test/regress/expected/pg15.out +++ b/src/test/regress/expected/pg15.out @@ -349,6 +349,58 @@ NOTICE: renaming the new table to pg15.tbl2 (1 row) +-- Make sure that we allow foreign key columns on local tables added to +-- metadata to have SET NULL/DEFAULT on column basis. +CREATE TABLE PKTABLE_local (tid int, id int, PRIMARY KEY (tid, id)); +CREATE TABLE FKTABLE_local ( + tid int, id int, + fk_id_del_set_null int, + fk_id_del_set_default int DEFAULT 0, + FOREIGN KEY (tid, fk_id_del_set_null) REFERENCES PKTABLE_local ON DELETE SET NULL (fk_id_del_set_null), + FOREIGN KEY (tid, fk_id_del_set_default) REFERENCES PKTABLE_local ON DELETE SET DEFAULT (fk_id_del_set_default) +); +SELECT citus_add_local_table_to_metadata('FKTABLE_local', cascade_via_foreign_keys=>true); + citus_add_local_table_to_metadata +--------------------------------------------------------------------- + +(1 row) + +-- show that the definition is expected +SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'FKTABLE_local'::regclass::oid ORDER BY oid; + pg_get_constraintdef +--------------------------------------------------------------------- + FOREIGN KEY (tid, fk_id_del_set_default) REFERENCES pktable_local(tid, id) ON DELETE SET DEFAULT (fk_id_del_set_default) + FOREIGN KEY (tid, fk_id_del_set_null) REFERENCES pktable_local(tid, id) ON DELETE SET NULL (fk_id_del_set_null) +(2 rows) + +\c - - - :worker_1_port +SET search_path TO pg15; +-- show that the definition is expected on the worker as well +SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'FKTABLE_local'::regclass::oid ORDER BY oid; + pg_get_constraintdef +--------------------------------------------------------------------- + FOREIGN KEY (tid, fk_id_del_set_default) REFERENCES pktable_local(tid, id) ON DELETE SET DEFAULT (fk_id_del_set_default) + FOREIGN KEY (tid, fk_id_del_set_null) REFERENCES pktable_local(tid, id) ON DELETE SET NULL (fk_id_del_set_null) +(2 rows) + +-- also, make sure that it works as expected +INSERT INTO PKTABLE_local VALUES (1, 0), (1, 1), (1, 2); +INSERT INTO FKTABLE_local VALUES + (1, 1, 1, NULL), + (1, 2, NULL, 2); +DELETE FROM PKTABLE_local WHERE id = 1 OR id = 2; +SELECT * FROM FKTABLE_local ORDER BY id; + tid | id | fk_id_del_set_null | fk_id_del_set_default +--------------------------------------------------------------------- + 1 | 1 | | + 1 | 2 | | 0 +(2 rows) + +\c - - - :master_port +SET search_path TO pg15; +SET client_min_messages to ERROR; +DROP TABLE FKTABLE_local, PKTABLE_local; +RESET client_min_messages; SELECT 1 FROM citus_remove_node('localhost', :master_port); ?column? --------------------------------------------------------------------- diff --git a/src/test/regress/sql/pg15.sql b/src/test/regress/sql/pg15.sql index ce29e1d7d..14467a13b 100644 --- a/src/test/regress/sql/pg15.sql +++ b/src/test/regress/sql/pg15.sql @@ -213,6 +213,47 @@ WHEN MATCHED THEN DELETE; -- now, both distributed, not works SELECT undistribute_table('tbl1'); SELECT undistribute_table('tbl2'); + +-- Make sure that we allow foreign key columns on local tables added to +-- metadata to have SET NULL/DEFAULT on column basis. + +CREATE TABLE PKTABLE_local (tid int, id int, PRIMARY KEY (tid, id)); +CREATE TABLE FKTABLE_local ( + tid int, id int, + fk_id_del_set_null int, + fk_id_del_set_default int DEFAULT 0, + FOREIGN KEY (tid, fk_id_del_set_null) REFERENCES PKTABLE_local ON DELETE SET NULL (fk_id_del_set_null), + FOREIGN KEY (tid, fk_id_del_set_default) REFERENCES PKTABLE_local ON DELETE SET DEFAULT (fk_id_del_set_default) +); + +SELECT citus_add_local_table_to_metadata('FKTABLE_local', cascade_via_foreign_keys=>true); + +-- show that the definition is expected +SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'FKTABLE_local'::regclass::oid ORDER BY oid; + +\c - - - :worker_1_port + +SET search_path TO pg15; + +-- show that the definition is expected on the worker as well +SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'FKTABLE_local'::regclass::oid ORDER BY oid; + +-- also, make sure that it works as expected +INSERT INTO PKTABLE_local VALUES (1, 0), (1, 1), (1, 2); +INSERT INTO FKTABLE_local VALUES + (1, 1, 1, NULL), + (1, 2, NULL, 2); +DELETE FROM PKTABLE_local WHERE id = 1 OR id = 2; +SELECT * FROM FKTABLE_local ORDER BY id; + +\c - - - :master_port + +SET search_path TO pg15; + +SET client_min_messages to ERROR; +DROP TABLE FKTABLE_local, PKTABLE_local; +RESET client_min_messages; + SELECT 1 FROM citus_remove_node('localhost', :master_port); SELECT create_distributed_table('tbl1', 'x');