From 912d829757ad84511016635d40c8105c7e9f9beb Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Mon, 1 Feb 2021 18:07:01 +0300 Subject: [PATCH 1/3] Skip GENERATED AS ALWAYS STORED cols when processing cols owning sequences When finding columns owning sequences, we shouldn't rely on atthasdef since it might be true when column has GENERATED ALWAYS AS (...) STORED expression. --- src/backend/distributed/commands/sequence.c | 8 ++++ src/test/regress/expected/pg12.out | 46 +++++++++++++++++++++ src/test/regress/sql/pg12.sql | 24 +++++++++++ 3 files changed, 78 insertions(+) diff --git a/src/backend/distributed/commands/sequence.c b/src/backend/distributed/commands/sequence.c index d3fdcfd9f..8170ee0c2 100644 --- a/src/backend/distributed/commands/sequence.c +++ b/src/backend/distributed/commands/sequence.c @@ -177,6 +177,14 @@ ExtractColumnsOwningSequences(Oid relationId, List **columnNameList, continue; } +#if PG_VERSION_NUM >= PG_VERSION_12 + if (attributeForm->attgenerated == ATTRIBUTE_GENERATED_STORED) + { + /* skip columns with GENERATED AS ALWAYS expressions */ + continue; + } +#endif + char *columnName = NameStr(attributeForm->attname); *columnNameList = lappend(*columnNameList, columnName); diff --git a/src/test/regress/expected/pg12.out b/src/test/regress/expected/pg12.out index d2b07de95..6c7033b7b 100644 --- a/src/test/regress/expected/pg12.out +++ b/src/test/regress/expected/pg12.out @@ -411,6 +411,52 @@ where val = 'asdf'; 3 (1 row) +-- not replicate reference tables from other test files +SET citus.replicate_reference_tables_on_activate TO off; +SELECT 1 FROM citus_add_node('localhost', :master_port, groupId => 0); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +BEGIN; + CREATE TABLE generated_stored_col_test (x int, y int generated always as (x+1) stored); + SELECT citus_add_local_table_to_metadata('generated_stored_col_test'); + citus_add_local_table_to_metadata +--------------------------------------------------------------------- + +(1 row) + + -- simply check if GENERATED ALWAYS AS (...) STORED expression works fine + INSERT INTO generated_stored_col_test VALUES(1), (2); + SELECT * FROM generated_stored_col_test ORDER BY 1,2; + x | y +--------------------------------------------------------------------- + 1 | 2 + 2 | 3 +(2 rows) + + -- show that we keep such expressions on shell relation and shard relation + SELECT s.relname, a.attname, a.attgenerated + FROM pg_class s + JOIN pg_attribute a ON a.attrelid=s.oid + WHERE s.relname LIKE 'generated_stored_col_test%' AND + attname = 'y' + ORDER BY 1,2; + relname | attname | attgenerated +--------------------------------------------------------------------- + generated_stored_col_test | y | s + generated_stored_col_test_60040 | y | s +(2 rows) + +ROLLBACK; +RESET citus.replicate_reference_tables_on_activate; +SELECT citus_remove_node('localhost', :master_port); + citus_remove_node +--------------------------------------------------------------------- + +(1 row) + \set VERBOSITY terse drop schema test_pg12 cascade; NOTICE: drop cascades to 10 other objects diff --git a/src/test/regress/sql/pg12.sql b/src/test/regress/sql/pg12.sql index 7b3c0be6f..53a4f11f4 100644 --- a/src/test/regress/sql/pg12.sql +++ b/src/test/regress/sql/pg12.sql @@ -275,6 +275,30 @@ select count(*) from col_test where val = 'asdf'; +-- not replicate reference tables from other test files +SET citus.replicate_reference_tables_on_activate TO off; +SELECT 1 FROM citus_add_node('localhost', :master_port, groupId => 0); + +BEGIN; + CREATE TABLE generated_stored_col_test (x int, y int generated always as (x+1) stored); + SELECT citus_add_local_table_to_metadata('generated_stored_col_test'); + + -- simply check if GENERATED ALWAYS AS (...) STORED expression works fine + INSERT INTO generated_stored_col_test VALUES(1), (2); + SELECT * FROM generated_stored_col_test ORDER BY 1,2; + + -- show that we keep such expressions on shell relation and shard relation + SELECT s.relname, a.attname, a.attgenerated + FROM pg_class s + JOIN pg_attribute a ON a.attrelid=s.oid + WHERE s.relname LIKE 'generated_stored_col_test%' AND + attname = 'y' + ORDER BY 1,2; +ROLLBACK; + +RESET citus.replicate_reference_tables_on_activate; +SELECT citus_remove_node('localhost', :master_port); + \set VERBOSITY terse drop schema test_pg12 cascade; \set VERBOSITY default From 93c3f30024f22556deba0a042ef464af4ad61014 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Tue, 2 Feb 2021 16:02:19 +0300 Subject: [PATCH 2/3] Rename ExtractColumnsOwningSequences --- .../commands/citus_add_local_table_to_metadata.c | 4 ++-- src/backend/distributed/commands/sequence.c | 11 ++++++----- src/backend/distributed/metadata/metadata_sync.c | 4 ++-- src/include/distributed/commands/sequence.h | 5 +++-- 4 files changed, 13 insertions(+), 11 deletions(-) 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 b485df318..d3654a41a 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 @@ -893,8 +893,8 @@ DropAndMoveDefaultSequenceOwnerships(Oid sourceRelationId, Oid targetRelationId) { List *columnNameList = NIL; List *ownedSequenceIdList = NIL; - ExtractColumnsOwningSequences(sourceRelationId, &columnNameList, - &ownedSequenceIdList); + ExtractDefaultColumnsAndOwnedSequences(sourceRelationId, &columnNameList, + &ownedSequenceIdList); ListCell *columnNameCell = NULL; ListCell *ownedSequenceIdCell = NULL; diff --git a/src/backend/distributed/commands/sequence.c b/src/backend/distributed/commands/sequence.c index 8170ee0c2..64ae4e246 100644 --- a/src/backend/distributed/commands/sequence.c +++ b/src/backend/distributed/commands/sequence.c @@ -151,13 +151,14 @@ OptionsSpecifyOwnedBy(List *optionList, Oid *ownedByTableId) /* - * ExtractColumnsOwningSequences finds each column of relation with relationId - * defaulting to an owned sequence. Then, appends the column name and id of the - * owned sequence -that the column defaults- to the lists passed as NIL initially. + * ExtractDefaultColumnsAndOwnedSequences finds each column of relation with + * relationId that has a DEFAULT expression and each sequence owned by such + * columns (if any). Then, appends the column name and id of the owned sequence + * -that the column defaults- to the lists passed as NIL initially. */ void -ExtractColumnsOwningSequences(Oid relationId, List **columnNameList, - List **ownedSequenceIdList) +ExtractDefaultColumnsAndOwnedSequences(Oid relationId, List **columnNameList, + List **ownedSequenceIdList) { Assert(*columnNameList == NIL && *ownedSequenceIdList == NIL); diff --git a/src/backend/distributed/metadata/metadata_sync.c b/src/backend/distributed/metadata/metadata_sync.c index 8f507d84a..8aa8618e3 100644 --- a/src/backend/distributed/metadata/metadata_sync.c +++ b/src/backend/distributed/metadata/metadata_sync.c @@ -1131,7 +1131,7 @@ SequenceDependencyCommandList(Oid relationId) List *columnNameList = NIL; List *sequenceIdList = NIL; - ExtractColumnsOwningSequences(relationId, &columnNameList, &sequenceIdList); + ExtractDefaultColumnsAndOwnedSequences(relationId, &columnNameList, &sequenceIdList); ListCell *columnNameCell = NULL; ListCell *sequenceIdCell = NULL; @@ -1144,7 +1144,7 @@ SequenceDependencyCommandList(Oid relationId) if (!OidIsValid(sequenceId)) { /* - * ExtractColumnsOwningSequences returns entries for all columns, + * ExtractDefaultColumnsAndOwnedSequences returns entries for all columns, * but with 0 sequence ID unless there is default nextval(..). */ continue; diff --git a/src/include/distributed/commands/sequence.h b/src/include/distributed/commands/sequence.h index 1addbd1c1..5c39df1ac 100644 --- a/src/include/distributed/commands/sequence.h +++ b/src/include/distributed/commands/sequence.h @@ -13,8 +13,9 @@ #include "nodes/pg_list.h" -extern void ExtractColumnsOwningSequences(Oid relationId, List **columnNameList, - List **ownedSequenceIdList); +extern void ExtractDefaultColumnsAndOwnedSequences(Oid relationId, + List **columnNameList, + List **ownedSequenceIdList); #endif /* CITUS_SEQUENCE_H */ From 53b1888cac8520ee73a00e9cd5df81e2cf1d8c54 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Tue, 2 Feb 2021 16:07:29 +0300 Subject: [PATCH 3/3] Rename DropAndMoveDefaultSequenceOwnerships --- .../citus_add_local_table_to_metadata.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) 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 d3654a41a..39b7d483f 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 @@ -68,8 +68,8 @@ static char * GetRenameShardTriggerCommand(Oid shardRelationId, char *triggerNam static void DropRelationTruncateTriggers(Oid relationId); static char * GetDropTriggerCommand(Oid relationId, char *triggerName); static List * GetRenameStatsCommandList(List *statsOidList, uint64 shardId); -static void DropAndMoveDefaultSequenceOwnerships(Oid sourceRelationId, - Oid targetRelationId); +static void DropDefaultExpressionsAndMoveOwnedSequenceOwnerships(Oid sourceRelationId, + Oid targetRelationId); static void DropDefaultColumnDefinition(Oid relationId, char *columnName); static void TransferSequenceOwnership(Oid ownedSequenceId, Oid targetRelationId, char *columnName); @@ -309,7 +309,8 @@ CreateCitusLocalTable(Oid relationId, bool cascadeViaForeignKeys) * DEFAULT expressions from shard relation as we should evaluate such columns * in shell table when needed. */ - DropAndMoveDefaultSequenceOwnerships(shardRelationId, shellRelationId); + DropDefaultExpressionsAndMoveOwnedSequenceOwnerships(shardRelationId, + shellRelationId); InsertMetadataForCitusLocalTable(shellRelationId, shardId); @@ -883,13 +884,14 @@ GetRenameStatsCommandList(List *statsOidList, uint64 shardId) /* - * DropAndMoveDefaultSequenceOwnerships 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. + * 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. */ static void -DropAndMoveDefaultSequenceOwnerships(Oid sourceRelationId, Oid targetRelationId) +DropDefaultExpressionsAndMoveOwnedSequenceOwnerships(Oid sourceRelationId, + Oid targetRelationId) { List *columnNameList = NIL; List *ownedSequenceIdList = NIL;