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 785501dc3..55d8f7479 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 @@ -52,8 +52,7 @@ static void ErrorIfAddingPartitionTableToMetadata(Oid relationId); static void ErrorIfUnsupportedCreateCitusLocalTable(Relation relation); static void ErrorIfUnsupportedCitusLocalTableKind(Oid relationId); static void ErrorIfUnsupportedCitusLocalColumnDefinition(Relation relation); -static void NoticeIfAutoConvertingLocalTables(bool autoConverted); -static void NoticeRelationIsAlreadyAddedToMetadata(Oid relationId); +static void NoticeIfAutoConvertingLocalTables(bool autoConverted, Oid relationId); static CascadeOperationType GetCascadeTypeForCitusLocalTables(bool autoConverted); static List * GetShellTableDDLEventsForCitusLocalTable(Oid relationId); static uint64 ConvertLocalTableToShard(Oid relationId); @@ -213,8 +212,6 @@ CreateCitusLocalTable(Oid relationId, bool cascadeViaForeignKeys, bool autoConve UpdateAutoConvertedForConnectedRelations(list_make1_oid(relationId), autoConverted); - NoticeRelationIsAlreadyAddedToMetadata(relationId); - return; } @@ -242,7 +239,7 @@ CreateCitusLocalTable(Oid relationId, bool cascadeViaForeignKeys, bool autoConve */ relation_close(relation, NoLock); - NoticeIfAutoConvertingLocalTables(autoConverted); + NoticeIfAutoConvertingLocalTables(autoConverted, relationId); if (TableHasExternalForeignKeys(relationId)) { @@ -565,44 +562,29 @@ ErrorIfUnsupportedCitusLocalColumnDefinition(Relation relation) * will be undistributed automatically, if it gets disconnected from reference table(s). */ static void -NoticeIfAutoConvertingLocalTables(bool autoConverted) +NoticeIfAutoConvertingLocalTables(bool autoConverted, Oid relationId) { if (autoConverted && ShouldEnableLocalReferenceForeignKeys()) { + char *qualifiedRelationName = generate_qualified_relation_name(relationId); + /* * When foreign keys between reference tables and postgres tables are * enabled, we automatically undistribute citus local tables that are * not chained with any reference tables back to postgres tables. * So give a warning to user for that. */ - ereport(NOTICE, (errmsg("local tables that are added to metadata but not " - "chained with reference tables via foreign keys might " - "be automatically converted back to postgres tables"), - errhint("Consider setting " - "citus.enable_local_reference_table_foreign_keys " - "to 'off' to disable this behavior"))); + ereport(NOTICE, (errmsg("local tables that are added to metadata automatically " + "by citus, but not chained with reference tables via " + "foreign keys might be automatically converted back to " + "postgres tables"), + errhint("Executing citus_add_local_table_to_metadata($$%s$$) " + "prevents this for the given relation, and all of the " + "connected relations", qualifiedRelationName))); } } -/* - * NoticeRelationIsAlreadyAddedToMetadata logs a NOTICE message that informs the user - * that the given relation is already added to metadata. - * We set the field autoConverted for these cases to false. - * This function tells the user that the given table will not be removed from metadata, - * as it was a user request. - */ -static void -NoticeRelationIsAlreadyAddedToMetadata(Oid relationId) -{ - char *relname = get_rel_name(relationId); - ereport(NOTICE, (errmsg("relation \"%s\" is already added to metadata", relname), - errdetail("This relation will not be removed from metadata even if " - "it is not connected to a reference table via foreign " - "key(s), since it is added to metadata by the user"))); -} - - /* * GetCascadeTypeForCitusLocalTables returns CASCADE_AUTO_ADD_LOCAL_TABLE_TO_METADATA * if autoConverted is true. Returns CASCADE_USER_ADD_LOCAL_TABLE_TO_METADATA otherwise. diff --git a/src/backend/distributed/commands/table.c b/src/backend/distributed/commands/table.c index 9d43d3710..7a08ccda3 100644 --- a/src/backend/distributed/commands/table.c +++ b/src/backend/distributed/commands/table.c @@ -77,8 +77,9 @@ static bool RelationIdListContainsCitusTableType(List *relationIdList, static bool RelationIdListContainsPostgresTable(List *relationIdList); static void ConvertPostgresLocalTablesToCitusLocalTables( AlterTableStmt *alterTableStatement); -static bool RangeVarListHasRelationConvertedByUser(List *relationRangeVarList, - AlterTableStmt *alterTableStatement); +static bool RangeVarListHasLocalRelationConvertedByUser(List *relationRangeVarList, + AlterTableStmt * + alterTableStatement); static int CompareRangeVarsByOid(const void *leftElement, const void *rightElement); static List * GetAlterTableAddFKeyRightRelationIdList( AlterTableStmt *alterTableStatement); @@ -1269,13 +1270,9 @@ ConvertPostgresLocalTablesToCitusLocalTables(AlterTableStmt *alterTableStatement * table to a citus local table. */ relationRangeVarList = SortList(relationRangeVarList, CompareRangeVarsByOid); - - bool autoConverted = true; - - if (RangeVarListHasRelationConvertedByUser(relationRangeVarList, alterTableStatement)) - { - autoConverted = false; - } + bool containsAnyUserConvertedLocalRelation = + RangeVarListHasLocalRelationConvertedByUser(relationRangeVarList, + alterTableStatement); /* * Here we should operate on RangeVar objects since relations oid's would @@ -1299,20 +1296,22 @@ ConvertPostgresLocalTablesToCitusLocalTables(AlterTableStmt *alterTableStatement else if (IsCitusTableType(relationId, CITUS_LOCAL_TABLE)) { CitusTableCacheEntry *entry = GetCitusTableCacheEntry(relationId); - if (!entry->autoConverted || autoConverted) + if (!entry->autoConverted) { /* - * relationRangeVarList has also reference and citus local tables - * involved in this ADD FOREIGN KEY command. Moreover, even if - * relationId was belonging to a postgres local table initially, - * we might had already converted it to a citus local table by cascading. - * Note that we cannot skip here, if the relation is marked as - * auto-converted, but we are marking the relations in the command - * auto-converted = false. Because in that case, it means that this - * relation is marked as auto-converted because of a connection - * with a reference table, but now it is connected to a Citus - * Local Table. In that case, we need to mark this relation as - * auto-converted = false, so cannot do a "continue" here. + * This citus local table is already added to the metadata + * by the user, so no further operation needed. + */ + continue; + } + else if (!containsAnyUserConvertedLocalRelation) + { + /* + * We are safe to skip this relation because none of the citus local + * tables involved are manually added to the metadata by the user. + * This implies that all the Citus local tables involved are marked + * as autoConverted = true and there is no chance to update + * autoConverted = false. */ continue; } @@ -1353,6 +1352,24 @@ ConvertPostgresLocalTablesToCitusLocalTables(AlterTableStmt *alterTableStatement } else { + /* + * There might be two scenarios: + * + * a) A user created foreign key from a reference table + * to Postgres local table(s) or Citus local table(s) + * where all of the citus local tables involved are auto + * converted. In that case, we mark the new table as auto + * converted as well. + * + * b) A user created foreign key from a reference table + * to Postgres local table(s) or Citus local table(s) + * where at least one of the citus local tables + * involved is not auto converted. In that case, we mark + * this new Citus local table as autoConverted = false + * as well. Because our logic is to keep all the connected + * Citus local tables to have the same autoConverted value. + */ + bool autoConverted = containsAnyUserConvertedLocalRelation ? false : true; CreateCitusLocalTable(relationId, cascade, autoConverted); } } @@ -1380,14 +1397,14 @@ ConvertPostgresLocalTablesToCitusLocalTables(AlterTableStmt *alterTableStatement /* - * RangeVarListHasRelationConvertedByUser takes a list of relations and returns true + * RangeVarListHasLocalRelationConvertedByUser takes a list of relations and returns true * if any of these relations is marked as auto-converted = false. Returns true otherwise. * This function also takes the current alterTableStatement command, to obtain the * necessary locks. */ static bool -RangeVarListHasRelationConvertedByUser(List *relationRangeVarList, - AlterTableStmt *alterTableStatement) +RangeVarListHasLocalRelationConvertedByUser(List *relationRangeVarList, + AlterTableStmt *alterTableStatement) { RangeVar *relationRangeVar; foreach_ptr(relationRangeVar, relationRangeVarList) diff --git a/src/backend/distributed/sql/citus--10.2-3--11.0-1.sql b/src/backend/distributed/sql/citus--10.2-3--11.0-1.sql index 437c5f3eb..887643ccd 100644 --- a/src/backend/distributed/sql/citus--10.2-3--11.0-1.sql +++ b/src/backend/distributed/sql/citus--10.2-3--11.0-1.sql @@ -8,7 +8,11 @@ DROP FUNCTION IF EXISTS pg_catalog.master_apply_delete_command(text); DROP FUNCTION pg_catalog.master_get_table_metadata(text); + +-- all existing citus local tables are auto converted +-- none of the other tables can have auto-converted as true ALTER TABLE pg_catalog.pg_dist_partition ADD COLUMN autoconverted boolean DEFAULT false; +UPDATE pg_catalog.pg_dist_partition SET autoconverted = TRUE WHERE partmethod = 'n' AND repmodel = 's'; REVOKE ALL ON FUNCTION start_metadata_sync_to_node(text, integer) FROM PUBLIC; REVOKE ALL ON FUNCTION stop_metadata_sync_to_node(text, integer,bool) FROM PUBLIC; diff --git a/src/test/regress/bin/normalize.sed b/src/test/regress/bin/normalize.sed index 65ab17438..f9184da14 100644 --- a/src/test/regress/bin/normalize.sed +++ b/src/test/regress/bin/normalize.sed @@ -199,9 +199,9 @@ s/citus_local_table_4_idx_[0-9]+/citus_local_table_4_idx_xxxxxx/g s/citus_local_table_4_[0-9]+/citus_local_table_4_xxxxxx/g s/ERROR: cannot append to shardId [0-9]+/ERROR: cannot append to shardId xxxxxx/g -# hide warning/hint message that we get when executing create_citus_local_table -/local tables that are added to metadata but not chained with reference tables via foreign keys might be automatically converted back to postgres tables$/d -/Consider setting citus.enable_local_reference_table_foreign_keys to 'off' to disable this behavior$/d +# hide notice/hint message that we get when converting local tables automatically +/local tables that are added to metadata automatically by citus, but not chained with reference tables via foreign keys might be automatically converted back to postgres tables$/d +/Executing citus_add_local_table_to_metadata(.*) prevents this for the given relation, and all of the connected relations$/d # normalize partitioned table shard constraint name errors for upgrade_partition_constraints_(before|after) s/^(ERROR: child table is missing constraint "\w+)_([0-9])+"/\1_xxxxxx"/g