Error out for views with circular dependencies (#6051)

Adds error check for views with circular dependencies

src/backend/distributed/commands/dependencies.c

@@ -202,12 +202,12 @@ ErrorIfCircularDependencyExists(const ObjectAddress *objectAddress)
 
 /*
  * DeferErrorIfCircularDependencyExists checks whether given object has
- * circular dependency with itself via existing objects of pg_dist_object.
+ * circular dependency with itself. If so, returns a deferred error.
  */
 DeferredErrorMessage *
 DeferErrorIfCircularDependencyExists(const ObjectAddress *objectAddress)
 {
-	List *dependencies = GetAllSupportedDependenciesForObject(objectAddress);
+	List *dependencies = GetAllDependenciesForObject(objectAddress);
 
 	ObjectAddress *dependency = NULL;
 	foreach_ptr(dependency, dependencies)

src/backend/distributed/metadata/dependency.c

@@ -1615,7 +1615,7 @@ ExpandCitusSupportedTypes(ObjectAddressCollector *collector, ObjectAddress targe
 			 * rule and that rule has dependencies to other objects.
 			 */
 			char relKind = get_rel_relkind(relationId);
-			if (relKind == RELKIND_VIEW)
+			if (relKind == RELKIND_VIEW || relKind == RELKIND_MATVIEW)
 			{
 				List *ruleRefDepList = GetViewRuleReferenceDependencyList(relationId);
 				result = list_concat(result, ruleRefDepList);
@@ -2109,6 +2109,23 @@ GetDependingViews(Oid relationId)
 		ViewDependencyNode *dependingNode = NULL;
 		foreach_ptr(dependingNode, node->dependingNodes)
 		{
+			ObjectAddress relationAddress = { 0 };
+			ObjectAddressSet(relationAddress, RelationRelationId, dependingNode->id);
+
+			/*
+			 * This function does not catch views with circular dependencies,
+			 * because of the remaining dependency count check below.
+			 * Here we check if the view has a circular dependency or not.
+			 * If yes, we error out with a message that tells the user that
+			 * Citus does not handle circular dependencies.
+			 */
+			DeferredErrorMessage *depError =
+				DeferErrorIfCircularDependencyExists(&relationAddress);
+			if (depError != NULL)
+			{
+				RaiseDeferredError(depError, ERROR);
+			}
+
 			dependingNode->remainingDependencyCount--;
 			if (dependingNode->remainingDependencyCount == 0)
 			{

src/test/regress/expected/citus_local_tables_mx.out

@@ -882,8 +882,13 @@ CREATE MATERIALIZED VIEW matview_101 AS SELECT * from loc_tb;
 CREATE VIEW v103 AS SELECT * from loc_tb;
 CREATE MATERIALIZED VIEW matview_102 AS SELECT * from loc_tb JOIN v103 USING (a);
 CREATE OR REPLACE VIEW v103 AS SELECT * from loc_tb JOIN matview_102 USING (a);
+-- fails to add local table to metadata, because of the circular dependency
+ALTER TABLE loc_tb ADD CONSTRAINT fkey FOREIGN KEY (a) references ref_tb(a);
+ERROR:  Citus can not handle circular dependencies between distributed objects
+-- drop the view&matview with circular dependency
+DROP VIEW v103 CASCADE;
 SET client_min_messages TO DEBUG1;
--- auto undistribute
+-- now it should successfully add to metadata and create the views on workers
 ALTER TABLE loc_tb ADD CONSTRAINT fkey FOREIGN KEY (a) references ref_tb(a);
 DEBUG:  executing "CREATE OR REPLACE VIEW citus_local_tables_mx.v100 (a) AS  SELECT loc_tb.a
    FROM citus_local_tables_mx.loc_tb; ALTER VIEW citus_local_tables_mx.v100 OWNER TO postgres"
@@ -907,6 +912,7 @@ select run_command_on_workers($$SELECT count(*) from citus_local_tables_mx.v100,
  (localhost,57638,t,0)
 (2 rows)
 
+-- auto undistribute
 ALTER TABLE loc_tb DROP CONSTRAINT fkey;
 -- fails because fkey is dropped and table is converted to local table
 select run_command_on_workers($$SELECT count(*) from citus_local_tables_mx.v100$$);
@@ -1075,6 +1081,29 @@ SELECT count(*) FROM citus_local_tables_mx.mv4;
      0
 (1 row)
 
+-- test circular dependency detection among views
+create table root_tbl (a int);
+create materialized view chain_v1 as select * from root_tbl;
+create view chain_v2 as select * from chain_v1;
+WARNING:  "view chain_v2" has dependency to "table root_tbl" that is not in Citus' metadata
+create materialized view chain_v3 as select * from chain_v2;
+create or replace view chain_v2 as select * from chain_v1 join chain_v3 using (a);
+WARNING:  "view chain_v2" has dependency on unsupported object "materialized view chain_v3"
+-- catch circular dependency and error out
+select citus_add_local_table_to_metadata('root_tbl');
+ERROR:  Citus can not handle circular dependencies between distributed objects
+-- same for create_distributed_table
+select create_distributed_table('root_tbl','a');
+ERROR:  Citus can not handle circular dependencies between distributed objects
+-- fix the circular dependency and add to metadata
+create or replace view chain_v2 as select * from chain_v1;
+WARNING:  "view chain_v2" has dependency to "table root_tbl" that is not in Citus' metadata
+select citus_add_local_table_to_metadata('root_tbl');
+ citus_add_local_table_to_metadata
+---------------------------------------------------------------------
+
+(1 row)
+
 -- todo: add more matview tests once 5968 and 6028 are fixed
 -- cleanup at exit
 set client_min_messages to error;

src/test/regress/expected/view_propagation.out

@@ -811,11 +811,8 @@ WARNING:  "view v_test_2" has dependency to "table employees" that is not in Cit
 DETAIL:  "view v_test_2" will be created only locally
 HINT:  Distribute "table employees" first to distribute "view v_test_2"
 SELECT create_distributed_table('employees','employee_id');
- create_distributed_table
----------------------------------------------------------------------
-
-(1 row)
-
+ERROR:  Citus can not handle circular dependencies between distributed objects
+DETAIL:  "view v_test_1" circularly depends itself, resolve circular dependency first
 -- verify not distributed
 SELECT run_command_on_workers($$SELECT count(*) FROM v_test_1$$);
                        run_command_on_workers

src/test/regress/sql/citus_local_tables_mx.sql

@@ -460,14 +460,20 @@ CREATE VIEW v103 AS SELECT * from loc_tb;
 CREATE MATERIALIZED VIEW matview_102 AS SELECT * from loc_tb JOIN v103 USING (a);
 CREATE OR REPLACE VIEW v103 AS SELECT * from loc_tb JOIN matview_102 USING (a);
 
+-- fails to add local table to metadata, because of the circular dependency
+ALTER TABLE loc_tb ADD CONSTRAINT fkey FOREIGN KEY (a) references ref_tb(a);
+-- drop the view&matview with circular dependency
+DROP VIEW v103 CASCADE;
+
 SET client_min_messages TO DEBUG1;
--- auto undistribute
+-- now it should successfully add to metadata and create the views on workers
 ALTER TABLE loc_tb ADD CONSTRAINT fkey FOREIGN KEY (a) references ref_tb(a);
 SET client_min_messages TO WARNING;
 
 -- works fine
 select run_command_on_workers($$SELECT count(*) from citus_local_tables_mx.v100, citus_local_tables_mx.v101, citus_local_tables_mx.v102$$);
 
+-- auto undistribute
 ALTER TABLE loc_tb DROP CONSTRAINT fkey;
 -- fails because fkey is dropped and table is converted to local table
 select run_command_on_workers($$SELECT count(*) from citus_local_tables_mx.v100$$);
@@ -537,6 +543,19 @@ SELECT count(*) FROM citus_local_tables_mx.mv2;
 SELECT count(*) FROM citus_local_tables_mx.mv3;
 SELECT count(*) FROM citus_local_tables_mx.mv4;
 
+-- test circular dependency detection among views
+create table root_tbl (a int);
+create materialized view chain_v1 as select * from root_tbl;
+create view chain_v2 as select * from chain_v1;
+create materialized view chain_v3 as select * from chain_v2;
+create or replace view chain_v2 as select * from chain_v1 join chain_v3 using (a);
+-- catch circular dependency and error out
+select citus_add_local_table_to_metadata('root_tbl');
+-- same for create_distributed_table
+select create_distributed_table('root_tbl','a');
+-- fix the circular dependency and add to metadata
+create or replace view chain_v2 as select * from chain_v1;
+select citus_add_local_table_to_metadata('root_tbl');
 -- todo: add more matview tests once 5968 and 6028 are fixed
 
 -- cleanup at exit