From 41dca121e26d7012f83c455d2cab9dff4cfbce8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philip=20Dub=C3=A9?= Date: Fri, 9 Aug 2019 22:38:03 +0000 Subject: [PATCH 1/2] Support GENERATE ALWAYS AS STORED --- .../commands/create_distributed_table.c | 6 ++++- src/backend/distributed/commands/multi_copy.c | 27 +++++++++++++++---- .../distributed/utils/citus_ruleutils.c | 14 +++++++++- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/backend/distributed/commands/create_distributed_table.c b/src/backend/distributed/commands/create_distributed_table.c index 0e5c1edd4..432279b1d 100644 --- a/src/backend/distributed/commands/create_distributed_table.c +++ b/src/backend/distributed/commands/create_distributed_table.c @@ -1368,7 +1368,11 @@ TupleDescColumnNameList(TupleDesc tupleDescriptor) Form_pg_attribute currentColumn = TupleDescAttr(tupleDescriptor, columnIndex); char *columnName = NameStr(currentColumn->attname); - if (currentColumn->attisdropped) + if (currentColumn->attisdropped +#if PG_VERSION_NUM >= 120000 + || currentColumn->attgenerated == ATTRIBUTE_GENERATED_STORED +#endif + ) { continue; } diff --git a/src/backend/distributed/commands/multi_copy.c b/src/backend/distributed/commands/multi_copy.c index defc08756..c62a10af3 100644 --- a/src/backend/distributed/commands/multi_copy.c +++ b/src/backend/distributed/commands/multi_copy.c @@ -64,6 +64,7 @@ #include "access/sysattr.h" #include "access/xact.h" #include "catalog/namespace.h" +#include "catalog/pg_attribute.h" #include "catalog/pg_type.h" #include "commands/copy.h" #include "commands/defrem.h" @@ -481,7 +482,11 @@ CopyToExistingShards(CopyStmt *copyStatement, char *completionTag) Form_pg_attribute currentColumn = TupleDescAttr(tupleDescriptor, columnIndex); char *columnName = NameStr(currentColumn->attname); - if (currentColumn->attisdropped) + if (currentColumn->attisdropped +#if PG_VERSION_NUM >= 120000 + || currentColumn->attgenerated == ATTRIBUTE_GENERATED_STORED +#endif + ) { continue; } @@ -510,7 +515,7 @@ CopyToExistingShards(CopyStmt *copyStatement, char *completionTag) * of BeginCopyFrom. However, we obviously should not do this in relcache * and therefore make a copy of the Relation. */ - copiedDistributedRelation = (Relation) palloc0(sizeof(RelationData)); + copiedDistributedRelation = (Relation) palloc(sizeof(RelationData)); copiedDistributedRelationTuple = (Form_pg_class) palloc(CLASS_TUPLE_SIZE); /* @@ -1028,7 +1033,11 @@ CanUseBinaryCopyFormat(TupleDesc tupleDescription) Form_pg_attribute currentColumn = TupleDescAttr(tupleDescription, columnIndex); Oid typeId = InvalidOid; - if (currentColumn->attisdropped) + if (currentColumn->attisdropped +#if PG_VERSION_NUM >= 120000 + || currentColumn->attgenerated == ATTRIBUTE_GENERATED_STORED +#endif + ) { continue; } @@ -1667,7 +1676,11 @@ AppendCopyRowData(Datum *valueArray, bool *isNullArray, TupleDesc rowDescriptor, value = CoerceColumnValue(value, &columnCoercionPaths[columnIndex]); } - if (currentColumn->attisdropped) + if (currentColumn->attisdropped +#if PG_VERSION_NUM >= 120000 + || currentColumn->attgenerated == ATTRIBUTE_GENERATED_STORED +#endif + ) { continue; } @@ -1787,7 +1800,11 @@ AvailableColumnCount(TupleDesc tupleDescriptor) { Form_pg_attribute currentColumn = TupleDescAttr(tupleDescriptor, columnIndex); - if (!currentColumn->attisdropped) + if (!currentColumn->attisdropped +#if PG_VERSION_NUM >= 120000 + && currentColumn->attgenerated != ATTRIBUTE_GENERATED_STORED +#endif + ) { columnCount++; } diff --git a/src/backend/distributed/utils/citus_ruleutils.c b/src/backend/distributed/utils/citus_ruleutils.c index d4c3cdfe1..f54c6f057 100644 --- a/src/backend/distributed/utils/citus_ruleutils.c +++ b/src/backend/distributed/utils/citus_ruleutils.c @@ -312,7 +312,7 @@ pg_get_tableschemadef_string(Oid tableRelationId, bool includeSequenceDefaults) * reasoning behind this is that Citus implements declarative partitioning * by creating the partitions first and then sending * "ALTER TABLE parent_table ATTACH PARTITION .." command. This may not play - * well with regular inhereted tables, which isn't a big concern from Citus' + * well with regular inherited tables, which isn't a big concern from Citus' * perspective. */ if (!attributeForm->attisdropped) @@ -371,7 +371,19 @@ pg_get_tableschemadef_string(Oid tableRelationId, bool includeSequenceDefaults) defaultString = deparse_expression(defaultNode, defaultContext, false, false); +#if PG_VERSION_NUM >= 120000 + if (attributeForm->attgenerated == ATTRIBUTE_GENERATED_STORED) + { + appendStringInfo(&buffer, " GENERATED ALWAYS AS (%s) STORED", + defaultString); + } + else + { + appendStringInfo(&buffer, " DEFAULT %s", defaultString); + } +#else appendStringInfo(&buffer, " DEFAULT %s", defaultString); +#endif } } From bdd30bb181571430b3f771538aef1f659471e95d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philip=20Dub=C3=A9?= Date: Fri, 30 Aug 2019 15:46:24 +0000 Subject: [PATCH 2/2] Don't allow distributing by a generated column --- .../commands/create_distributed_table.c | 33 ++++++++------- src/test/regress/expected/pg12.out | 40 ++++++++++--------- src/test/regress/sql/pg12.sql | 8 ++-- 3 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/backend/distributed/commands/create_distributed_table.c b/src/backend/distributed/commands/create_distributed_table.c index 432279b1d..8b15f04ae 100644 --- a/src/backend/distributed/commands/create_distributed_table.c +++ b/src/backend/distributed/commands/create_distributed_table.c @@ -103,7 +103,8 @@ static bool LocalTableEmpty(Oid tableId); static void CopyLocalDataIntoShards(Oid relationId); static List * TupleDescColumnNameList(TupleDesc tupleDescriptor); static bool RelationUsesIdentityColumns(TupleDesc relationDesc); -static bool RelationUsesGeneratedStoredColumns(TupleDesc relationDesc); +static bool DistributionColumnUsesGeneratedStoredColumn(TupleDesc relationDesc, + Var *distributionColumn); static bool RelationUsesHeapAccessMethodOrNone(Relation relation); static bool CanUseExclusiveConnections(Oid relationId, bool localTableEmpty); @@ -684,12 +685,13 @@ EnsureRelationCanBeDistributed(Oid relationId, Var *distributionColumn, "... AS IDENTITY."))); } - /* verify target relation does not use generated columns */ - if (RelationUsesGeneratedStoredColumns(relationDesc)) + /* verify target relation is not distributed by a generated columns */ + if (distributionMethod != DISTRIBUTE_BY_NONE && + DistributionColumnUsesGeneratedStoredColumn(relationDesc, distributionColumn)) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot distribute relation: %s", relationName), - errdetail("Distributed relations must not use GENERATED ALWAYS " + errdetail("Distribution column must not use GENERATED ALWAYS " "AS (...) STORED."))); } @@ -1385,8 +1387,8 @@ TupleDescColumnNameList(TupleDesc tupleDescriptor) /* - * RelationUsesIdentityColumns returns whether a given relation uses the SQL - * GENERATED ... AS IDENTITY features introduced as of PostgreSQL 10. + * RelationUsesIdentityColumns returns whether a given relation uses + * GENERATED ... AS IDENTITY */ static bool RelationUsesIdentityColumns(TupleDesc relationDesc) @@ -1408,23 +1410,20 @@ RelationUsesIdentityColumns(TupleDesc relationDesc) /* - * RelationUsesIdentityColumns returns whether a given relation uses the SQL - * GENERATED ... AS IDENTITY features introduced as of PostgreSQL 10. + * DistributionColumnUsesGeneratedStoredColumn returns whether a given relation uses + * GENERATED ALWAYS AS (...) STORED on distribution column */ static bool -RelationUsesGeneratedStoredColumns(TupleDesc relationDesc) +DistributionColumnUsesGeneratedStoredColumn(TupleDesc relationDesc, + Var *distributionColumn) { #if PG_VERSION_NUM >= 120000 - int attributeIndex = 0; + Form_pg_attribute attributeForm = TupleDescAttr(relationDesc, + distributionColumn->varattno - 1); - for (attributeIndex = 0; attributeIndex < relationDesc->natts; attributeIndex++) + if (attributeForm->attgenerated == ATTRIBUTE_GENERATED_STORED) { - Form_pg_attribute attributeForm = TupleDescAttr(relationDesc, attributeIndex); - - if (attributeForm->attgenerated == ATTRIBUTE_GENERATED_STORED) - { - return true; - } + return true; } #endif diff --git a/src/test/regress/expected/pg12.out b/src/test/regress/expected/pg12.out index be554a521..b946f99c7 100644 --- a/src/test/regress/expected/pg12.out +++ b/src/test/regress/expected/pg12.out @@ -33,37 +33,41 @@ create table gen2 ( ); insert into gen1 (id, val1) values (1,4),(3,6),(5,2),(7,2); insert into gen2 (id, val1) values (1,4),(3,6),(5,2),(7,2); -select * from create_distributed_table('gen1', 'id'); -ERROR: cannot distribute relation: gen1 -DETAIL: Distributed relations must not use GENERATED ALWAYS AS (...) STORED. -select * from create_distributed_table('gen2', 'val2'); +select create_distributed_table('gen1', 'id'); +NOTICE: Copying data from local table... + create_distributed_table +-------------------------- + +(1 row) + +select create_distributed_table('gen2', 'val2'); ERROR: cannot distribute relation: gen2 -DETAIL: Distributed relations must not use GENERATED ALWAYS AS (...) STORED. +DETAIL: Distribution column must not use GENERATED ALWAYS AS (...) STORED. insert into gen1 (id, val1) values (2,4),(4,6),(6,2),(8,2); insert into gen2 (id, val1) values (2,4),(4,6),(6,2),(8,2); -select * from gen1; +select * from gen1 order by 1,2,3; id | val1 | val2 ----+------+------ 1 | 4 | 6 - 3 | 6 | 8 - 5 | 2 | 4 - 7 | 2 | 4 2 | 4 | 6 + 3 | 6 | 8 4 | 6 | 8 + 5 | 2 | 4 6 | 2 | 4 + 7 | 2 | 4 8 | 2 | 4 (8 rows) -select * from gen2; +select * from gen2 order by 1,2,3; id | val1 | val2 ----+------+------ 1 | 4 | 6 - 3 | 6 | 8 - 5 | 2 | 4 - 7 | 2 | 4 2 | 4 | 6 + 3 | 6 | 8 4 | 6 | 8 + 5 | 2 | 4 6 | 2 | 4 + 7 | 2 | 4 8 | 2 | 4 (8 rows) @@ -169,7 +173,7 @@ $Q$); coordinator_plan ------------------------------------------ Custom Scan (Citus Adaptive) - -> Distributed Subplan 5_1 + -> Distributed Subplan 7_1 -> Custom Scan (Citus Adaptive) Task Count: 4 (4 rows) @@ -214,7 +218,7 @@ $Q$); ------------------------------------------------ Aggregate -> Custom Scan (Citus Adaptive) - -> Distributed Subplan 8_1 + -> Distributed Subplan 10_1 -> Custom Scan (Citus Adaptive) Task Count: 4 (5 rows) @@ -235,7 +239,7 @@ $Q$); ------------------------------------------------ Aggregate -> Custom Scan (Citus Adaptive) - -> Distributed Subplan 10_1 + -> Distributed Subplan 12_1 -> Custom Scan (Citus Adaptive) Task Count: 4 (5 rows) @@ -280,8 +284,8 @@ NOTICE: Copying data from local table... -- should still fail because of fkey INSERT INTO collection_users VALUES (1, 1000, 1); -ERROR: insert or update on table "collection_users_60024" violates foreign key constraint "collection_users_fkey_60024" -DETAIL: Key (key, collection_id)=(1, 1000) is not present in table "collections_list_60012". +ERROR: insert or update on table "collection_users_60028" violates foreign key constraint "collection_users_fkey_60028" +DETAIL: Key (key, collection_id)=(1, 1000) is not present in table "collections_list_60016". CONTEXT: while executing command on localhost:57637 -- whereas new record with partition should go through INSERT INTO collections_list VALUES (2, 1, '1.2'); diff --git a/src/test/regress/sql/pg12.sql b/src/test/regress/sql/pg12.sql index 3054724e0..9b3848cca 100644 --- a/src/test/regress/sql/pg12.sql +++ b/src/test/regress/sql/pg12.sql @@ -39,14 +39,14 @@ create table gen2 ( insert into gen1 (id, val1) values (1,4),(3,6),(5,2),(7,2); insert into gen2 (id, val1) values (1,4),(3,6),(5,2),(7,2); -select * from create_distributed_table('gen1', 'id'); -select * from create_distributed_table('gen2', 'val2'); +select create_distributed_table('gen1', 'id'); +select create_distributed_table('gen2', 'val2'); insert into gen1 (id, val1) values (2,4),(4,6),(6,2),(8,2); insert into gen2 (id, val1) values (2,4),(4,6),(6,2),(8,2); -select * from gen1; -select * from gen2; +select * from gen1 order by 1,2,3; +select * from gen2 order by 1,2,3; -- Test new VACUUM/ANALYZE options analyze (skip_locked) gen1;