Skip postgres tables for UndistributeTable(cascadeViaFKeys) (#4530)

The reason behind skipping postgres tables is that we support
foreign keys between postgres tables and reference tables
(without converting postgres tables to citus local tables)
when enable_local_reference_table_foreign_keys is false or
when coordinator is not added to metadata.

src/backend/distributed/commands/cascade_table_operation_for_connected_relations.c

@@ -353,19 +353,28 @@ ExecuteCascadeOperationForRelationIdList(List *relationIdList,
 	foreach_oid(relationId, relationIdList)
 	{
 		/*
-		 * Caller already passed the relations that we should operate on,
+		 * The reason behind skipping certain table types in below loop is
-		 * so we should not cascade here.
+		 * that we support some sort of foreign keys between postgres tables
+		 * and citus tables when enable_local_reference_table_foreign_keys is
+		 * false or when coordinator is not added to metadata.
+		 *
+		 * Also, as caller already passed the relations that we should operate
+		 * on, we don't cascade via foreign keys here.
 		 */
 		bool cascadeViaForeignKeys = false;
 		switch (cascadeOperationType)
 		{
 			case CASCADE_FKEY_UNDISTRIBUTE_TABLE:
+			{
+				if (IsCitusTable(relationId))
 				{
 					TableConversionParameters params = {
 						.relationId = relationId,
 						.cascadeViaForeignKeys = cascadeViaForeignKeys
 					};
 					UndistributeTable(&params);
+				}
+
 				break;
 			}
 
@@ -373,14 +382,6 @@ ExecuteCascadeOperationForRelationIdList(List *relationIdList,
 			{
 				if (!IsCitusTable(relationId))
 				{
-					/*
-					 * Normally, we wouldn't expect a postgres table connected
-					 * to a citus local table via a foreign keys graph. But now
-					 * this is possible as we allow foreign keys from postgres
-					 * tables to reference tables when coordinator is not added
-					 * to metadata. So instead of erroring out, we skip citus
-					 * tables here.
-					 */
 					CreateCitusLocalTable(relationId, cascadeViaForeignKeys);
 				}
 

src/test/regress/expected/undistribute_table_cascade.out

@@ -4,6 +4,38 @@ SET citus.shard_replication_factor TO 1;
 CREATE SCHEMA undistribute_table_cascade;
 SET search_path TO undistribute_table_cascade;
 SET client_min_messages to ERROR;
+-- remove coordinator if it is added to pg_dist_node
+SELECT COUNT(master_remove_node(nodename, nodeport)) < 2
+FROM pg_dist_node WHERE nodename='localhost' AND nodeport=:master_port;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+BEGIN;
+  CREATE TABLE reference_table(col_1 INT UNIQUE);
+  CREATE TABLE distributed_table(col_1 INT UNIQUE);
+  CREATE TABLE local_table (col_1 INT REFERENCES reference_table(col_1), FOREIGN KEY (col_1) REFERENCES distributed_table(col_1));
+  SELECT create_reference_table('reference_table');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+  SELECT create_distributed_table('distributed_table', 'col_1');
+ create_distributed_table
+---------------------------------------------------------------------
+
+(1 row)
+
+  -- show that we skip postgres tables when undistributing citus tables
+  SELECT undistribute_table('reference_table', cascade_via_foreign_keys=>true);
+ undistribute_table
+---------------------------------------------------------------------
+
+(1 row)
+
+ROLLBACK;
 -- ensure that coordinator is added to pg_dist_node
 SELECT 1 FROM master_add_node('localhost', :master_port, groupId => 0);
  ?column?

src/test/regress/sql/undistribute_table_cascade.sql

@@ -8,6 +8,22 @@ SET search_path TO undistribute_table_cascade;
 
 SET client_min_messages to ERROR;
 
+-- remove coordinator if it is added to pg_dist_node
+SELECT COUNT(master_remove_node(nodename, nodeport)) < 2
+FROM pg_dist_node WHERE nodename='localhost' AND nodeport=:master_port;
+
+BEGIN;
+  CREATE TABLE reference_table(col_1 INT UNIQUE);
+  CREATE TABLE distributed_table(col_1 INT UNIQUE);
+  CREATE TABLE local_table (col_1 INT REFERENCES reference_table(col_1), FOREIGN KEY (col_1) REFERENCES distributed_table(col_1));
+
+  SELECT create_reference_table('reference_table');
+  SELECT create_distributed_table('distributed_table', 'col_1');
+
+  -- show that we skip postgres tables when undistributing citus tables
+  SELECT undistribute_table('reference_table', cascade_via_foreign_keys=>true);
+ROLLBACK;
+
 -- ensure that coordinator is added to pg_dist_node
 SELECT 1 FROM master_add_node('localhost', :master_port, groupId => 0);