From 3a403090fd2d4381321c0748dc702a28740e6e92 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Wed, 3 Feb 2021 19:05:17 +0300 Subject: [PATCH] Disallow adding local table with identity column to metadata (#4633) pg_get_tableschemadef_string doesn't know how to deparse identity columns so we cannot reflect those columns when creating shell relation. For this reason, we don't allow adding local tables -having identity cols- to metadata. --- .../citus_add_local_table_to_metadata.c | 26 +++++++++++++++++++ .../commands/create_distributed_table.c | 3 +-- src/include/distributed/metadata_utility.h | 1 + .../regress/expected/citus_local_tables.out | 12 +++++++++ src/test/regress/sql/citus_local_tables.sql | 12 +++++++++ 5 files changed, 52 insertions(+), 2 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 39b7d483f..d89c44630 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 @@ -49,6 +49,7 @@ static void citus_add_local_table_to_metadata_internal(Oid relationId, bool cascadeViaForeignKeys); static void ErrorIfUnsupportedCreateCitusLocalTable(Relation relation); static void ErrorIfUnsupportedCitusLocalTableKind(Oid relationId); +static void ErrorIfUnsupportedCitusLocalColumnDefinition(Relation relation); static List * GetShellTableDDLEventsForCitusLocalTable(Oid relationId); static uint64 ConvertLocalTableToShard(Oid relationId); static void RenameRelationToShardRelation(Oid shellRelationId, uint64 shardId); @@ -338,6 +339,7 @@ ErrorIfUnsupportedCreateCitusLocalTable(Relation relation) ErrorIfCoordinatorNotAddedAsWorkerNode(); ErrorIfUnsupportedCitusLocalTableKind(relationId); EnsureTableNotDistributed(relationId); + ErrorIfUnsupportedCitusLocalColumnDefinition(relation); /* * When creating other citus table types, we don't need to check that case as @@ -405,6 +407,30 @@ ErrorIfUnsupportedCitusLocalTableKind(Oid relationId) } +/* + * ErrorIfUnsupportedCitusLocalColumnDefinition errors out if given relation + * has unsupported column definition for citus local table creation. + */ +static void +ErrorIfUnsupportedCitusLocalColumnDefinition(Relation relation) +{ + TupleDesc relationDesc = RelationGetDescr(relation); + if (RelationUsesIdentityColumns(relationDesc)) + { + /* + * pg_get_tableschemadef_string doesn't know how to deparse identity + * columns so we cannot reflect those columns when creating shell + * relation. For this reason, error out here. + */ + Oid relationId = relation->rd_id; + ereport(ERROR, (errmsg("cannot add %s to citus metadata since table " + "has identity column", + generate_qualified_relation_name(relationId)), + errhint("Drop the identity columns and re-try the command"))); + } +} + + /* * GetShellTableDDLEventsForCitusLocalTable returns a list of DDL commands * to create the shell table from scratch. diff --git a/src/backend/distributed/commands/create_distributed_table.c b/src/backend/distributed/commands/create_distributed_table.c index e852aa8e3..744f08196 100644 --- a/src/backend/distributed/commands/create_distributed_table.c +++ b/src/backend/distributed/commands/create_distributed_table.c @@ -122,7 +122,6 @@ static void DropFKeysRelationInvolvedWithTableType(Oid relationId, int tableType static bool LocalTableEmpty(Oid tableId); static void CopyLocalDataIntoShards(Oid relationId); static List * TupleDescColumnNameList(TupleDesc tupleDescriptor); -static bool RelationUsesIdentityColumns(TupleDesc relationDesc); static bool DistributionColumnUsesGeneratedStoredColumn(TupleDesc relationDesc, Var *distributionColumn); static bool CanUseExclusiveConnections(Oid relationId, bool localTableEmpty); @@ -1636,7 +1635,7 @@ TupleDescColumnNameList(TupleDesc tupleDescriptor) * RelationUsesIdentityColumns returns whether a given relation uses * GENERATED ... AS IDENTITY */ -static bool +bool RelationUsesIdentityColumns(TupleDesc relationDesc) { for (int attributeIndex = 0; attributeIndex < relationDesc->natts; attributeIndex++) diff --git a/src/include/distributed/metadata_utility.h b/src/include/distributed/metadata_utility.h index 96db653d0..dfb5aeb0f 100644 --- a/src/include/distributed/metadata_utility.h +++ b/src/include/distributed/metadata_utility.h @@ -249,6 +249,7 @@ extern void EnsureTableNotDistributed(Oid relationId); extern void EnsureReplicationSettings(Oid relationId, char replicationModel); extern void EnsureRelationExists(Oid relationId); extern bool RegularTable(Oid relationId); +extern bool RelationUsesIdentityColumns(TupleDesc relationDesc); extern char * ConstructQualifiedShardName(ShardInterval *shardInterval); extern uint64 GetFirstShardId(Oid relationId); extern Datum StringToDatum(char *inputString, Oid dataType); diff --git a/src/test/regress/expected/citus_local_tables.out b/src/test/regress/expected/citus_local_tables.out index b1d2a1f22..5baca6098 100644 --- a/src/test/regress/expected/citus_local_tables.out +++ b/src/test/regress/expected/citus_local_tables.out @@ -56,6 +56,18 @@ BEGIN; SELECT citus_add_local_table_to_metadata('temp_table'); ERROR: constraints on temporary tables may reference only temporary tables ROLLBACK; +-- below two errors out since we don't support adding local tables +-- having any identity columns to metadata +BEGIN; + CREATE TABLE identity_cols_test (a int generated by default as identity (start with 42)); + SELECT citus_add_local_table_to_metadata('identity_cols_test'); +ERROR: cannot add citus_local_tables_test_schema.identity_cols_test to citus metadata since table has identity column +ROLLBACK; +BEGIN; + CREATE TABLE identity_cols_test (a int generated always as identity (increment by 42)); + SELECT citus_add_local_table_to_metadata('identity_cols_test'); +ERROR: cannot add citus_local_tables_test_schema.identity_cols_test to citus metadata since table has identity column +ROLLBACK; -- creating citus local table having no data initially would work SELECT citus_add_local_table_to_metadata('citus_local_table_1'); citus_add_local_table_to_metadata diff --git a/src/test/regress/sql/citus_local_tables.sql b/src/test/regress/sql/citus_local_tables.sql index 1cfa75d27..148244e51 100644 --- a/src/test/regress/sql/citus_local_tables.sql +++ b/src/test/regress/sql/citus_local_tables.sql @@ -46,6 +46,18 @@ BEGIN; SELECT citus_add_local_table_to_metadata('temp_table'); ROLLBACK; +-- below two errors out since we don't support adding local tables +-- having any identity columns to metadata +BEGIN; + CREATE TABLE identity_cols_test (a int generated by default as identity (start with 42)); + SELECT citus_add_local_table_to_metadata('identity_cols_test'); +ROLLBACK; + +BEGIN; + CREATE TABLE identity_cols_test (a int generated always as identity (increment by 42)); + SELECT citus_add_local_table_to_metadata('identity_cols_test'); +ROLLBACK; + -- creating citus local table having no data initially would work SELECT citus_add_local_table_to_metadata('citus_local_table_1');