Add support for ALTER TABLE commands defining foreign keys

src/backend/distributed/commands/table.c

@@ -45,10 +45,23 @@
 /* Local functions forward declarations for unsupported command checks */
 static void PostprocessCreateTableStmtPartitionOf(CreateStmt *createStatement,
 												  const char *queryString);
-static void ErrorIfAlterTableDefinesFKeyFromPostgresToCitusLocalTable(
+static bool AlterTableDefinesFKeyBetweenPostgresAndNonDistTable(
 	AlterTableStmt *alterTableStatement);
-static List * GetAlterTableStmtFKeyConstraintList(AlterTableStmt *alterTableStatement);
+static bool RelationIdListContainsCitusTableType(List *relationIdList,
+												 CitusTableType citusTableType);
+static bool RelationIdListContainsPostgresTable(List *relationIdList);
+static void ConvertPostgresLocalTablesToCitusLocalTables(
+	AlterTableStmt *alterTableStatement);
+static int CompareRangeVarsByOid(const void *leftElement, const void *rightElement);
+static List * GetAlterTableAddFKeyRightRelationIdList(
+	AlterTableStmt *alterTableStatement);
+static List * GetAlterTableAddFKeyRightRelationRangeVarList(
+	AlterTableStmt *alterTableStatement);
+static List * GetAlterTableAddFKeyConstraintList(AlterTableStmt *alterTableStatement);
 static List * GetAlterTableCommandFKeyConstraintList(AlterTableCmd *command);
+static List * GetRangeVarListFromFKeyConstraintList(List *fKeyConstraintList);
+static List * GetRelationIdListFromRangeVarList(List *rangeVarList, LOCKMODE lockmode,
+												bool missingOk);
 static bool AlterTableCommandTypeIsTrigger(AlterTableType alterTableType);
 static void ErrorIfUnsupportedAlterTableStmt(AlterTableStmt *alterTableStatement);
 static void ErrorIfCitusLocalTablePartitionCommand(AlterTableCmd *alterTableCmd,
@@ -154,7 +167,7 @@ PreprocessDropTableStmt(Node *node, const char *queryString,
 /*
  * PostprocessCreateTableStmt takes CreateStmt object as a parameter and errors
  * out if it creates a table with a foreign key that references to a citus local
- * table if pg version is older than 13 (see comment in function).
+ * table.
  *
  * This function also processes CREATE TABLE ... PARTITION OF statements via
  * PostprocessCreateTableStmtPartitionOf function.
@@ -162,17 +175,6 @@ PreprocessDropTableStmt(Node *node, const char *queryString,
 void
 PostprocessCreateTableStmt(CreateStmt *createStatement, const char *queryString)
 {
-#if PG_VERSION_NUM < PG_VERSION_13
-
-	/*
-	 * Postgres processes foreign key constraints implied by CREATE TABLE
-	 * commands by internally executing ALTER TABLE commands via standard
-	 * process utility starting from PG13. Hence, we will already perform
-	 * unsupported foreign key checks via PreprocessAlterTableStmt function
-	 * in PG13. But for the older version, we need to do unsupported foreign
-	 * key checks here.
-	 */
-
 	/*
 	 * Relation must exist and it is already locked as standard process utility
 	 * is already executed.
@@ -183,7 +185,6 @@ PostprocessCreateTableStmt(CreateStmt *createStatement, const char *queryString)
 	{
 		ErrorOutForFKeyBetweenPostgresAndCitusLocalTable(relationId);
 	}
-#endif
 
 	if (createStatement->inhRelations != NIL && createStatement->partbound != NULL)
 	{
@@ -378,14 +379,38 @@ PreprocessAlterTableStmt(Node *node, const char *alterTableCommand,
 		leftRelationId = IndexGetRelation(leftRelationId, missingOk);
 	}
 
+	if (processUtilityContext != PROCESS_UTILITY_SUBCOMMAND &&
+		AlterTableDefinesFKeyBetweenPostgresAndNonDistTable(alterTableStatement) &&
+		CoordinatorAddedAsWorkerNode())
+	{
 		/*
-	 * Normally, we would do this check in ErrorIfUnsupportedForeignConstraintExists
+		 * We don't process subcommands generated by postgres.
-	 * in post process step. However, we skip doing error checks in post process if
+		 * This is mainly because postgres started to issue ALTER TABLE commands
-	 * this pre process returns NIL -and this method returns NIL if the left relation
+		 * for some set of objects that are defined via CREATE TABLE commands as
-	 * is a postgres table. So, we need to error out for foreign keys from postgres
+		 * of pg13. However, citus already has a separate logic for CREATE TABLE
-	 * tables to citus local tables here.
+		 * commands.
+		 *
+		 * To support foreign keys from/to postgres local tables to/from reference
+		 * or citus local tables, we convert given postgres local table -and the
+		 * other postgres tables that it is connected via a fkey graph- to a citus
+		 * local table.
+		 *
+		 * Note that we don't convert postgres tables to citus local tables if
+		 * coordinator is not added to metadata as CreateCitusLocalTable requires
+		 * this. In this case, we assume user is about to create reference or
+		 * distributed table from local table and we don't want to break user
+		 * experience by asking to add coordinator to metadata.
 		 */
-	ErrorIfAlterTableDefinesFKeyFromPostgresToCitusLocalTable(alterTableStatement);
+		ConvertPostgresLocalTablesToCitusLocalTables(alterTableStatement);
+
+		/*
+		 * CreateCitusLocalTable converts relation to a shard relation and creates
+		 * shell table from scratch.
+		 * For this reason we should re-enter to PreprocessAlterTableStmt to operate
+		 * on shell table relation id.
+		 */
+		return PreprocessAlterTableStmt(node, alterTableCommand, processUtilityContext);
+	}
 
 	bool referencingIsLocalTable = !IsCitusTable(leftRelationId);
 	if (referencingIsLocalTable)
@@ -595,59 +620,249 @@ PreprocessAlterTableStmt(Node *node, const char *alterTableCommand,
 
 
 /*
- * ErrorIfAlterTableDefinesFKeyFromPostgresToCitusLocalTable errors out if
+ * AlterTableDefinesFKeyBetweenPostgresAndNonDistTable returns true if given
- * given ALTER TABLE statement defines foreign key from a postgres local table
+ * alter table command defines foreign key between a postgres table and a
- * to a citus local table.
+ * reference or citus local table.
+ */
+static bool
+AlterTableDefinesFKeyBetweenPostgresAndNonDistTable(AlterTableStmt *alterTableStatement)
+{
+	List *foreignKeyConstraintList =
+		GetAlterTableAddFKeyConstraintList(alterTableStatement);
+	if (list_length(foreignKeyConstraintList) == 0)
+	{
+		/* we are not defining any foreign keys */
+		return false;
+	}
+
+	List *rightRelationIdList =
+		GetAlterTableAddFKeyRightRelationIdList(alterTableStatement);
+
+	LOCKMODE lockmode = AlterTableGetLockLevel(alterTableStatement->cmds);
+	Oid leftRelationId = AlterTableLookupRelation(alterTableStatement, lockmode);
+	if (!IsCitusTable(leftRelationId))
+	{
+		return RelationIdListContainsCitusTableType(rightRelationIdList,
+													CITUS_TABLE_WITH_NO_DIST_KEY);
+	}
+	else if (IsCitusTableType(leftRelationId, CITUS_TABLE_WITH_NO_DIST_KEY))
+	{
+		return RelationIdListContainsPostgresTable(rightRelationIdList);
+	}
+
+	return false;
+}
+
+
+/*
+ * RelationIdListContainsCitusTableType returns true if given relationIdList
+ * contains a citus table with given type.
+ */
+static bool
+RelationIdListContainsCitusTableType(List *relationIdList, CitusTableType citusTableType)
+{
+	Oid relationId = InvalidOid;
+	foreach_oid(relationId, relationIdList)
+	{
+		if (IsCitusTableType(relationId, citusTableType))
+		{
+			return true;
+		}
+	}
+
+	return false;
+}
+
+
+/*
+ * RelationIdListContainsPostgresTable returns true if given relationIdList
+ * contains a postgres table.
+ */
+static bool
+RelationIdListContainsPostgresTable(List *relationIdList)
+{
+	Oid relationId = InvalidOid;
+	foreach_oid(relationId, relationIdList)
+	{
+		if (OidIsValid(relationId) && !IsCitusTable(relationId))
+		{
+			return true;
+		}
+	}
+
+	return false;
+}
+
+
+/*
+ * ConvertPostgresLocalTablesToCitusLocalTables converts each postgres table
+ * involved in foreign keys to be defined by given alter table command and the
+ * other tables connected to them via a foreign key graph to citus local tables.
  */
 static void
-ErrorIfAlterTableDefinesFKeyFromPostgresToCitusLocalTable(
+ConvertPostgresLocalTablesToCitusLocalTables(AlterTableStmt *alterTableStatement)
-	AlterTableStmt *alterTableStatement)
+{
+	List *rightRelationRangeVarList =
+		GetAlterTableAddFKeyRightRelationRangeVarList(alterTableStatement);
+	RangeVar *leftRelationRangeVar = alterTableStatement->relation;
+	List *relationRangeVarList = lappend(rightRelationRangeVarList, leftRelationRangeVar);
+
+	/*
+	 * To prevent deadlocks, sort the list before converting each postgres local
+	 * table to a citus local table.
+	 */
+	relationRangeVarList = SortList(relationRangeVarList, CompareRangeVarsByOid);
+
+	/*
+	 * Here we should operate on RangeVar objects since relations oid's would
+	 * change in below loop due to CreateCitusLocalTable.
+	 */
+	RangeVar *relationRangeVar;
+	foreach_ptr(relationRangeVar, relationRangeVarList)
 	{
 		List *commandList = alterTableStatement->cmds;
-
+		LOCKMODE lockMode = AlterTableGetLockLevel(commandList);
-	LOCKMODE lockmode = AlterTableGetLockLevel(commandList);
+		bool missingOk = alterTableStatement->missing_ok;
-	Oid leftRelationId = AlterTableLookupRelation(alterTableStatement, lockmode);
+		Oid relationId = RangeVarGetRelid(relationRangeVar, lockMode, missingOk);
-
+		if (!OidIsValid(relationId))
-	if (IsCitusTable(leftRelationId))
 		{
-		/* left relation is not a postgres local table, */
+			/*
-		return;
+			 * As we are in preprocess, missingOk might be true and relation
+			 * might not exist.
+			 */
+			continue;
+		}
+		else if (IsCitusTable(relationId))
+		{
+			/*
+			 * 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.
+			 */
+			continue;
 		}
 
-	List *alterTableFKeyConstraints =
+		/*
-		GetAlterTableStmtFKeyConstraintList(alterTableStatement);
+		 * The only reason behind using a try/catch block here is giving a proper
-	Constraint *constraint = NULL;
+		 * error message. For example, when creating a citus local table we might
-	foreach_ptr(constraint, alterTableFKeyConstraints)
+		 * give an error telling that partitioned tables are not supported for
+		 * citus local table creation. But as a user it wouldn't make much sense
+		 * to see such an error. So here we extend error message to tell that we
+		 * actually ended up with this error when trying to define the foreign key.
+		 *
+		 * Also, as CopyErrorData() requires (CurrentMemoryContext != ErrorContext),
+		 * so we store CurrentMemoryContext here.
+		 */
+		MemoryContext savedMemoryContext = CurrentMemoryContext;
+		PG_TRY();
 		{
-		Oid rightRelationId = RangeVarGetRelid(constraint->pktable, lockmode,
+			bool cascade = true;
-											   alterTableStatement->missing_ok);
+			CreateCitusLocalTable(relationId, cascade);
-		if (IsCitusTableType(rightRelationId, CITUS_LOCAL_TABLE))
-		{
-			ErrorOutForFKeyBetweenPostgresAndCitusLocalTable(leftRelationId);
 		}
+		PG_CATCH();
+		{
+			MemoryContextSwitchTo(savedMemoryContext);
+
+			ErrorData *errorData = CopyErrorData();
+			FlushErrorState();
+
+			if (errorData->elevel != ERROR)
+			{
+				PG_RE_THROW();
+			}
+
+			/* override error detail */
+			errorData->detail = "When adding a foreign key from a local table to "
+								"a reference table, Citus applies a conversion to "
+								"all the local tables in the foreign key graph";
+			ThrowErrorData(errorData);
+		}
+		PG_END_TRY();
 	}
 }
 
 
 /*
- * GetAlterTableStmtFKeyConstraintList returns a list of Constraint objects for
+ * CompareRangeVarsByOid is a comparison function to sort RangeVar object list.
- * the foreign keys that given ALTER TABLE statement defines.
+ */
+static int
+CompareRangeVarsByOid(const void *leftElement, const void *rightElement)
+{
+	RangeVar *leftRangeVar = *((RangeVar **) leftElement);
+	RangeVar *rightRangeVar = *((RangeVar **) rightElement);
+
+	/*
+	 * Any way we will check their existence, so it's okay to map non-existing
+	 * relations to InvalidOid when sorting.
+	 */
+	bool missingOk = true;
+
+	/*
+	 * As this is an object comparator function, there is no way to understand
+	 * proper lock mode. So assume caller already locked relations.
+	 */
+	LOCKMODE lockMode = NoLock;
+
+	Oid leftRelationId = RangeVarGetRelid(leftRangeVar, lockMode, missingOk);
+	Oid rightRelationId = RangeVarGetRelid(rightRangeVar, lockMode, missingOk);
+	return CompareOids(&leftRelationId, &rightRelationId);
+}
+
+
+/*
+ * GetAlterTableAddFKeyRightRelationIdList returns a list of oid's for right
+ * relations involved in foreign keys to be defined by given ALTER TABLE command.
  */
 static List *
-GetAlterTableStmtFKeyConstraintList(AlterTableStmt *alterTableStatement)
+GetAlterTableAddFKeyRightRelationIdList(AlterTableStmt *alterTableStatement)
 {
-	List *alterTableFKeyConstraintList = NIL;
+	List *rightRelationRangeVarList =
+		GetAlterTableAddFKeyRightRelationRangeVarList(alterTableStatement);
+	List *commandList = alterTableStatement->cmds;
+	LOCKMODE lockMode = AlterTableGetLockLevel(commandList);
+	bool missingOk = alterTableStatement->missing_ok;
+	List *rightRelationIdList =
+		GetRelationIdListFromRangeVarList(rightRelationRangeVarList, lockMode, missingOk);
+	return rightRelationIdList;
+}
+
+
+/*
+ * GetAlterTableAddFKeyRightRelationRangeVarList returns a list of RangeVar
+ * objects for right relations involved in foreign keys to be defined by
+ * given ALTER TABLE command.
+ */
+static List *
+GetAlterTableAddFKeyRightRelationRangeVarList(AlterTableStmt *alterTableStatement)
+{
+	List *fKeyConstraintList = GetAlterTableAddFKeyConstraintList(alterTableStatement);
+	List *rightRelationRangeVarList =
+		GetRangeVarListFromFKeyConstraintList(fKeyConstraintList);
+	return rightRelationRangeVarList;
+}
+
+
+/*
+ * GetAlterTableAddFKeyConstraintList returns a list of Constraint objects for
+ * foreign keys that given ALTER TABLE to be defined by given ALTER TABLE command.
+ */
+static List *
+GetAlterTableAddFKeyConstraintList(AlterTableStmt *alterTableStatement)
+{
+	List *foreignKeyConstraintList = NIL;
 
 	List *commandList = alterTableStatement->cmds;
 	AlterTableCmd *command = NULL;
 	foreach_ptr(command, commandList)
 	{
-		List *commandFKeyConstraintList = GetAlterTableCommandFKeyConstraintList(command);
+		List *commandForeignKeyConstraintList =
-		alterTableFKeyConstraintList = list_concat(alterTableFKeyConstraintList,
+			GetAlterTableCommandFKeyConstraintList(command);
-												   commandFKeyConstraintList);
+		foreignKeyConstraintList = list_concat(foreignKeyConstraintList,
+											   commandForeignKeyConstraintList);
 	}
 
-	return alterTableFKeyConstraintList;
+	return foreignKeyConstraintList;
 }
 
 
@@ -692,6 +907,47 @@ GetAlterTableCommandFKeyConstraintList(AlterTableCmd *command)
 }
 
 
+/*
+ * GetRangeVarListFromFKeyConstraintList returns a list of RangeVar objects for
+ * right relations in fKeyConstraintList.
+ */
+static List *
+GetRangeVarListFromFKeyConstraintList(List *fKeyConstraintList)
+{
+	List *rightRelationRangeVarList = NIL;
+
+	Constraint *fKeyConstraint = NULL;
+	foreach_ptr(fKeyConstraint, fKeyConstraintList)
+	{
+		RangeVar *rightRelationRangeVar = fKeyConstraint->pktable;
+		rightRelationRangeVarList = lappend(rightRelationRangeVarList,
+											rightRelationRangeVar);
+	}
+
+	return rightRelationRangeVarList;
+}
+
+
+/*
+ * GetRelationIdListFromRangeVarList returns relation id list for relations
+ * identified by RangeVar objects in given list.
+ */
+static List *
+GetRelationIdListFromRangeVarList(List *rangeVarList, LOCKMODE lockMode, bool missingOk)
+{
+	List *relationIdList = NIL;
+
+	RangeVar *rangeVar = NULL;
+	foreach_ptr(rangeVar, rangeVarList)
+	{
+		Oid rightRelationId = RangeVarGetRelid(rangeVar, lockMode, missingOk);
+		relationIdList = lappend_oid(relationIdList, rightRelationId);
+	}
+
+	return relationIdList;
+}
+
+
 /*
  * AlterTableCommandTypeIsTrigger returns true if given alter table command type
  * is identifies an ALTER TABLE .. TRIGGER .. command.

src/include/distributed/commands.h

@@ -484,4 +484,8 @@ extern void CreateCitusLocalTable(Oid relationId, bool cascadeViaForeignKeys);
 extern bool ShouldPropagateSetCommand(VariableSetStmt *setStmt);
 extern void PostprocessVariableSetStmt(VariableSetStmt *setStmt, const char *setCommand);
 
+/* create_citus_local_table.c */
+
+extern void CreateCitusLocalTable(Oid relationId, bool cascade);
+
 #endif /*CITUS_COMMANDS_H */

src/test/regress/expected/citus_local_tables.out

@@ -413,8 +413,6 @@ NOTICE:  executing the command locally: CREATE UNIQUE INDEX   uniqueindex2_15040
 ---- utility command execution ----
 ---------------------------------------------------------------------
 SET search_path TO citus_local_tables_test_schema;
--- any foreign key between citus local tables and other tables except reference tables cannot be set
--- more tests at ref_citus_local_fkeys.sql
 -- between citus local tables and distributed tables
 ALTER TABLE citus_local_table_1 ADD CONSTRAINT fkey_c_to_dist FOREIGN KEY(a) references distributed_table(a);
 ERROR:  cannot create foreign key constraint since foreign keys from reference tables and citus local tables to distributed tables are not supported
@@ -422,15 +420,15 @@ ALTER TABLE distributed_table ADD CONSTRAINT fkey_dist_to_c FOREIGN KEY(a) refer
 ERROR:  cannot create foreign key constraint since relations are not colocated or not referencing a reference table
 -- between citus local tables and local tables
 ALTER TABLE citus_local_table_1 ADD CONSTRAINT fkey_c_to_local FOREIGN KEY(a) references local_table(a);
-ERROR:  cannot create foreign key constraint as "local_table" is a postgres local table
+NOTICE:  executing the command locally: SELECT worker_apply_inter_shard_ddl_command (1504027, 'citus_local_tables_test_schema', 1504037, 'citus_local_tables_test_schema', 'ALTER TABLE citus_local_table_1 ADD CONSTRAINT fkey_c_to_local FOREIGN KEY(a) references local_table(a);')
 ALTER TABLE local_table
   ADD CONSTRAINT fkey_local_to_c FOREIGN KEY(a) references citus_local_table_1(a),
   ADD CONSTRAINT fkey_self FOREIGN KEY(a) references local_table(a);
-ERROR:  cannot create foreign key constraint as "local_table" is a postgres local table
+ERROR:  cannot execute ADD CONSTRAINT command with other subcommands
 ALTER TABLE local_table
   ADD COLUMN b int references citus_local_table_1(a),
   ADD COLUMN c int references local_table(a);
-ERROR:  cannot create foreign key constraint as "local_table" is a postgres local table
+ERROR:  cannot execute ADD COLUMN command with PRIMARY KEY, UNIQUE, FOREIGN and CHECK constraints
 CREATE TABLE local_table_4 (
   a int unique references citus_local_table_1(a),
   b int references local_table_4(a));
@@ -478,6 +476,8 @@ ORDER BY 1;
 TRUNCATE citus_local_table_1, citus_local_table_2, distributed_table, local_table, reference_table;
 NOTICE:  executing the command locally: TRUNCATE TABLE citus_local_tables_test_schema.citus_local_table_1_xxxxx CASCADE
 NOTICE:  executing the command locally: TRUNCATE TABLE citus_local_tables_test_schema.citus_local_table_2_xxxxx CASCADE
+NOTICE:  executing the command locally: TRUNCATE TABLE citus_local_tables_test_schema.local_table_xxxxx CASCADE
+NOTICE:  truncate cascades to table "citus_local_table_1_xxxxxxx"
 NOTICE:  executing the command locally: TRUNCATE TABLE citus_local_tables_test_schema.reference_table_xxxxx CASCADE
 -- test vacuum
 VACUUM citus_local_table_1;
@@ -485,6 +485,8 @@ VACUUM citus_local_table_1, distributed_table, local_table, reference_table;
 -- test drop
 DROP TABLE citus_local_table_1, citus_local_table_2, distributed_table, local_table, reference_table;
 NOTICE:  executing the command locally: DROP TABLE IF EXISTS citus_local_tables_test_schema.reference_table_xxxxx CASCADE
+NOTICE:  executing the command locally: DROP TABLE IF EXISTS citus_local_tables_test_schema.local_table_xxxxx CASCADE
+NOTICE:  drop cascades to constraint fkey_c_to_local_1504027 on table citus_local_tables_test_schema.citus_local_table_1_1504027
 NOTICE:  executing the command locally: DROP TABLE IF EXISTS citus_local_tables_test_schema.citus_local_table_2_xxxxx CASCADE
 NOTICE:  executing the command locally: DROP TABLE IF EXISTS citus_local_tables_test_schema.citus_local_table_1_xxxxx CASCADE
 -- test some other udf's with citus local tables
@@ -498,9 +500,9 @@ SELECT create_citus_local_table('citus_local_table_4');
 -- should work --
 -- insert some data & create an index for table size udf's
 INSERT INTO citus_local_table_4 VALUES (1), (2), (3);
-NOTICE:  executing the command locally: INSERT INTO citus_local_tables_test_schema.citus_local_table_4_1504037 AS citus_table_alias (a) VALUES (1), (2), (3)
+NOTICE:  executing the command locally: INSERT INTO citus_local_tables_test_schema.citus_local_table_4_1504038 AS citus_table_alias (a) VALUES (1), (2), (3)
 CREATE INDEX citus_local_table_4_idx ON citus_local_table_4(a);
-NOTICE:  executing the command locally: CREATE  INDEX   citus_local_table_4_idx_1504037 ON citus_local_tables_test_schema.citus_local_table_4_1504037 USING btree (a )
+NOTICE:  executing the command locally: CREATE  INDEX   citus_local_table_4_idx_1504038 ON citus_local_tables_test_schema.citus_local_table_4_1504038 USING btree (a )
 SELECT citus_table_size('citus_local_table_4');
  citus_table_size
 ---------------------------------------------------------------------
@@ -587,7 +589,7 @@ BEGIN;
   SELECT tableName FROM pg_catalog.pg_tables WHERE tablename LIKE 'citus_local_table_4%';
           tablename
 ---------------------------------------------------------------------
- citus_local_table_4_1504037
+ citus_local_table_4_1504038
 (1 row)
 
 ROLLBACK;
@@ -596,7 +598,7 @@ SELECT shardid, get_colocated_shard_array(shardid)
 FROM (SELECT shardid FROM pg_dist_shard WHERE logicalrelid='citus_local_table_4'::regclass) as shardid;
  shardid | get_colocated_shard_array
 ---------------------------------------------------------------------
- 1504037 | {1504037}
+ 1504038 | {1504038}
 (1 row)
 
 BEGIN;
@@ -626,7 +628,7 @@ ERROR:  cannot delete from table
 CREATE TABLE postgres_local_table (a int);
 SELECT master_append_table_to_shard(shardId, 'postgres_local_table', 'localhost', :master_port)
 FROM (SELECT shardid FROM pg_dist_shard WHERE logicalrelid='citus_local_table_4'::regclass) as shardid;
-ERROR:  cannot append to shardId 1504037
+ERROR:  cannot append to shardId 1504038
 -- return true
 SELECT citus_table_is_visible('citus_local_table_4'::regclass::oid);
  citus_table_is_visible
@@ -667,7 +669,7 @@ SELECT create_citus_local_table('referenced_table');
 (1 row)
 
 ALTER TABLE referencing_table ADD CONSTRAINT fkey_cl_to_cl FOREIGN KEY (a) REFERENCES referenced_table(a);
-NOTICE:  executing the command locally: SELECT worker_apply_inter_shard_ddl_command (1504038, 'citus_local_tables_test_schema', 1504039, 'citus_local_tables_test_schema', 'ALTER TABLE referencing_table ADD CONSTRAINT fkey_cl_to_cl FOREIGN KEY (a) REFERENCES referenced_table(a);')
+NOTICE:  executing the command locally: SELECT worker_apply_inter_shard_ddl_command (1504039, 'citus_local_tables_test_schema', 1504040, 'citus_local_tables_test_schema', 'ALTER TABLE referencing_table ADD CONSTRAINT fkey_cl_to_cl FOREIGN KEY (a) REFERENCES referenced_table(a);')
 -- observe the debug messages telling that we switch to sequential
 -- execution when truncating a citus local table that is referenced
 -- by another table

src/test/regress/expected/fkeys_between_local_ref.out

@@ -0,0 +1,231 @@
+\set VERBOSITY terse
+SET citus.next_shard_id TO 1518000;
+SET citus.shard_replication_factor TO 1;
+CREATE SCHEMA fkeys_between_local_ref;
+SET search_path TO fkeys_between_local_ref;
+SET client_min_messages to ERROR;
+-- create a view for testing
+CREATE VIEW citus_local_tables_in_schema AS
+SELECT logicalrelid FROM pg_dist_partition, pg_tables
+WHERE tablename=logicalrelid::regclass::text AND
+      schemaname='fkeys_between_local_ref' AND
+      partmethod = 'n' AND repmodel = 'c';
+-- remove coordinator if it is added to pg_dist_node and test
+-- behavior when coordinator is not added to metadata
+SELECT COUNT(master_remove_node(nodename, nodeport)) < 2
+FROM pg_dist_node WHERE nodename='localhost' AND nodeport=:master_port;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+create table ref (a int primary key);
+select create_reference_table('ref');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+-- creating local table that references to reference table is supported
+create table other (x int primary key, y int);
+-- creating reference table from a local table that references
+-- to reference table is supported
+alter table other add constraint fk foreign key (y) references ref (a) on delete cascade;
+select create_reference_table('other');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+drop table if exists ref, ref2 cascade;
+create table ref (a int primary key);
+create table ref2 (x int);
+alter table ref2 add constraint fk foreign key (x) references ref (a);
+select create_reference_table('ref');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+-- we can also define more foreign keys after creating reference
+-- table from referenced table
+alter table ref2 add constraint fk2 foreign key (x) references ref (a);
+-- then we can create reference table from referencing table
+select create_reference_table('ref2');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+drop table if exists ref, ref2, other cascade;
+-- add coordinator to pg_dist_node for rest of the tests
+SELECT 1 FROM master_add_node('localhost', :master_port, groupId => 0);
+ ?column?
+---------------------------------------------------------------------
+        1
+(1 row)
+
+CREATE TABLE local_table_1 (col_1 INT UNIQUE);
+CREATE TABLE local_table_2 (col_1 INT UNIQUE);
+CREATE TABLE local_table_3 (col_1 INT UNIQUE);
+CREATE TABLE local_table_4 (col_1 INT UNIQUE);
+INSERT INTO local_table_1 SELECT i FROM generate_series(195, 205) i;
+INSERT INTO local_table_2 SELECT i FROM generate_series(195, 205) i;
+INSERT INTO local_table_3 SELECT i FROM generate_series(195, 205) i;
+INSERT INTO local_table_4 SELECT i FROM generate_series(195, 205) i;
+--                                        _
+--                                       | |
+--                                       | v
+-- local_table_2 -> local_table_1 -> local_table_4
+--                      ^ |               |
+--                      | v               |
+--                  local_table_3 <--------
+ALTER TABLE local_table_2 ADD CONSTRAINT fkey_1 FOREIGN KEY (col_1) REFERENCES local_table_1(col_1);
+ALTER TABLE local_table_3 ADD CONSTRAINT fkey_2 FOREIGN KEY (col_1) REFERENCES local_table_1(col_1);
+ALTER TABLE local_table_1 ADD CONSTRAINT fkey_3 FOREIGN KEY (col_1) REFERENCES local_table_3(col_1);
+ALTER TABLE local_table_1 ADD CONSTRAINT fkey_4 FOREIGN KEY (col_1) REFERENCES local_table_4(col_1);
+ALTER TABLE local_table_4 ADD CONSTRAINT fkey_5 FOREIGN KEY (col_1) REFERENCES local_table_3(col_1);
+ALTER TABLE local_table_4 ADD CONSTRAINT fkey_6 FOREIGN KEY (col_1) REFERENCES local_table_4(col_1);
+CREATE TABLE reference_table_1(col_1 INT UNIQUE, col_2 INT);
+INSERT INTO reference_table_1 SELECT i FROM generate_series(195, 205) i;
+SELECT create_reference_table('reference_table_1');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+CREATE TABLE partitioned_table_1 (col_1 INT, col_2 INT) PARTITION BY RANGE (col_1);
+CREATE TABLE partitioned_table_1_100_200 PARTITION OF partitioned_table_1 FOR VALUES FROM (100) TO (200);
+CREATE TABLE partitioned_table_1_200_300 PARTITION OF partitioned_table_1 FOR VALUES FROM (200) TO (300);
+INSERT INTO partitioned_table_1 SELECT i FROM generate_series(195, 205) i;
+ALTER TABLE partitioned_table_1 ADD CONSTRAINT fkey_8 FOREIGN KEY (col_1) REFERENCES local_table_4(col_1);
+-- now that we attached partitioned table to graph below errors out
+-- since we cannot create citus local table from partitioned tables
+ALTER TABLE reference_table_1 ADD CONSTRAINT fkey_9 FOREIGN KEY (col_1) REFERENCES local_table_1(col_1);
+ERROR:  cannot create citus local table "partitioned_table_1", only regular tables and foreign tables are supported for citus local table creation
+ALTER TABLE partitioned_table_1 DROP CONSTRAINT fkey_8;
+BEGIN;
+  -- now that we detached partitioned table from graph, succeeds
+  ALTER TABLE reference_table_1 ADD CONSTRAINT fkey_10 FOREIGN KEY (col_1) REFERENCES local_table_1(col_1);
+  -- show that we converted all 4 local tables in this schema to citus local tables
+  SELECT COUNT(*)=4 FROM citus_local_tables_in_schema;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+ROLLBACK;
+-- this actually attempts to convert local tables to citus local tables but errors out
+-- as citus doesn't support defining foreign keys via add column commands
+ALTER TABLE local_table_1 ADD COLUMN col_3 INT REFERENCES reference_table_1(col_1);
+ERROR:  cannot execute ADD COLUMN command with PRIMARY KEY, UNIQUE, FOREIGN and CHECK constraints
+BEGIN;
+  -- define a foreign key so that all 4 local tables become citus local tables
+  ALTER TABLE local_table_1 ADD CONSTRAINT fkey_11 FOREIGN KEY (col_1) REFERENCES reference_table_1(col_1);
+  CREATE TABLE local_table_5 (col_1 INT UNIQUE);
+  -- now define foreign key from local to citus local table
+  ALTER TABLE local_table_5 ADD CONSTRAINT fkey_12 FOREIGN KEY (col_1) REFERENCES local_table_2(col_1);
+  -- now we have 5 citus local tables in this schema
+  SELECT COUNT(*)=5 FROM citus_local_tables_in_schema;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+ROLLBACK;
+-- they fail as local_table_99 does not exist
+ALTER TABLE local_table_99 ADD CONSTRAINT fkey FOREIGN KEY (col_1) REFERENCES reference_table_1(col_1);
+ERROR:  relation "local_table_99" does not exist
+ALTER TABLE local_table_1 ADD CONSTRAINT fkey FOREIGN KEY (col_1) REFERENCES local_table_99(col_1);
+ERROR:  relation "local_table_99" does not exist
+-- they fail as col_99 does not exist
+ALTER TABLE local_table_1 ADD CONSTRAINT fkey FOREIGN KEY (col_99) REFERENCES reference_table_1(col_1);
+ERROR:  column "col_99" referenced in foreign key constraint does not exist
+ALTER TABLE local_table_1 ADD CONSTRAINT fkey FOREIGN KEY (col_1) REFERENCES reference_table_1(col_99);
+ERROR:  column "col_99" referenced in foreign key constraint does not exist
+-- fails as col_2 does not have a unique/primary key constraint
+ALTER TABLE local_table_1 ADD CONSTRAINT fkey FOREIGN KEY (col_1) REFERENCES reference_table_1(col_2);
+ERROR:  there is no unique constraint matching given keys for referenced table "reference_table_1"
+CREATE TABLE reference_table_2 (col_1 INT UNIQUE, col_2 INT);
+INSERT INTO reference_table_2 SELECT i FROM generate_series(195, 205) i;
+BEGIN;
+  -- define foreign key when both ends are local tables
+  ALTER TABLE local_table_1 ADD CONSTRAINT fkey_11 FOREIGN KEY (col_1) REFERENCES reference_table_2(col_1);
+  -- we still don't convert local tables to citus local tables when
+  -- creating reference tables
+  SELECT create_reference_table('reference_table_2');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+  -- now defining another foreign key would convert local tables to
+  -- citus local tables as reference_table_2 is now a reference table
+  ALTER TABLE local_table_1 ADD CONSTRAINT fkey_12 FOREIGN KEY (col_1) REFERENCES reference_table_2(col_1);
+  -- now print metadata to show that everyting is fine
+  SELECT logicalrelid::text AS tablename, partmethod, repmodel FROM pg_dist_partition
+  WHERE logicalrelid::text IN (SELECT tablename FROM pg_tables WHERE schemaname='fkeys_between_local_ref')
+  ORDER BY tablename;
+     tablename     | partmethod | repmodel
+---------------------------------------------------------------------
+ local_table_1     | n          | c
+ local_table_2     | n          | c
+ local_table_3     | n          | c
+ local_table_4     | n          | c
+ reference_table_1 | n          | t
+ reference_table_2 | n          | t
+(6 rows)
+
+ROLLBACK;
+-- we don't support foreign keys from citus local to reference tables
+-- with ON DELETE/UPDATE CASCADE behavior, so below two errors out
+BEGIN;
+  SELECT create_reference_table('reference_table_2');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+  ALTER TABLE reference_table_2 ADD CONSTRAINT fkey_11 FOREIGN KEY (col_1) REFERENCES local_table_2(col_1) ON DELETE CASCADE;
+ERROR:  cannot define foreign key constraint, foreign keys from reference tables to citus local tables can only be defined with NO ACTION or RESTRICT behaviors
+ROLLBACK;
+BEGIN;
+  SELECT create_reference_table('reference_table_2');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+  ALTER TABLE reference_table_2 ADD CONSTRAINT fkey_local_to_ref FOREIGN KEY (col_1) REFERENCES local_table_2(col_1) ON UPDATE CASCADE;
+ERROR:  cannot define foreign key constraint, foreign keys from reference tables to citus local tables can only be defined with NO ACTION or RESTRICT behaviors
+ROLLBACK;
+-- but we support such foreign key behaviors when foreign key is from
+-- citus local to reference table
+BEGIN;
+  SELECT create_reference_table('reference_table_2');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+  ALTER TABLE local_table_2 ADD CONSTRAINT fkey_11 FOREIGN KEY (col_1) REFERENCES reference_table_2(col_1) ON DELETE CASCADE;
+  DELETE FROM reference_table_2 WHERE col_1=200;
+  -- we deleted one row as DELETE cascades, so we should have 10 rows
+  SELECT COUNT(*) FROM local_table_2;
+ count
+---------------------------------------------------------------------
+    10
+(1 row)
+
+ROLLBACK;
+BEGIN;
+  SELECT create_reference_table('reference_table_2');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+  ALTER TABLE local_table_2 ADD CONSTRAINT fkey_11 FOREIGN KEY (col_1) REFERENCES reference_table_2(col_1) ON UPDATE CASCADE;
+ROLLBACK;
+-- cleanup at exit
+DROP SCHEMA fkeys_between_local_ref CASCADE;

src/test/regress/multi_schedule

@@ -327,6 +327,7 @@ test: citus_local_tables
 test: multi_row_router_insert mixed_relkind_tests create_ref_dist_from_citus_local
 test: undistribute_table_cascade
 test: create_citus_local_table_cascade
+test: fkeys_between_local_ref
 
 test: remove_coordinator
 

src/test/regress/sql/citus_local_tables.sql

@@ -313,9 +313,6 @@ CREATE UNIQUE INDEX uniqueIndex2 ON "LocalTabLE.1!?!"(id);
 
 SET search_path TO citus_local_tables_test_schema;
 
--- any foreign key between citus local tables and other tables except reference tables cannot be set
--- more tests at ref_citus_local_fkeys.sql
-
 -- between citus local tables and distributed tables
 ALTER TABLE citus_local_table_1 ADD CONSTRAINT fkey_c_to_dist FOREIGN KEY(a) references distributed_table(a);
 ALTER TABLE distributed_table ADD CONSTRAINT fkey_dist_to_c FOREIGN KEY(a) references citus_local_table_1(a);

src/test/regress/sql/fkeys_between_local_ref.sql

@@ -0,0 +1,179 @@
+\set VERBOSITY terse
+
+SET citus.next_shard_id TO 1518000;
+SET citus.shard_replication_factor TO 1;
+
+CREATE SCHEMA fkeys_between_local_ref;
+SET search_path TO fkeys_between_local_ref;
+
+SET client_min_messages to ERROR;
+
+-- create a view for testing
+CREATE VIEW citus_local_tables_in_schema AS
+SELECT logicalrelid FROM pg_dist_partition, pg_tables
+WHERE tablename=logicalrelid::regclass::text AND
+      schemaname='fkeys_between_local_ref' AND
+      partmethod = 'n' AND repmodel = 'c';
+
+
+-- remove coordinator if it is added to pg_dist_node and test
+-- behavior when coordinator is not added to metadata
+SELECT COUNT(master_remove_node(nodename, nodeport)) < 2
+FROM pg_dist_node WHERE nodename='localhost' AND nodeport=:master_port;
+
+create table ref (a int primary key);
+select create_reference_table('ref');
+-- creating local table that references to reference table is supported
+create table other (x int primary key, y int);
+
+-- creating reference table from a local table that references
+-- to reference table is supported
+alter table other add constraint fk foreign key (y) references ref (a) on delete cascade;
+select create_reference_table('other');
+
+drop table if exists ref, ref2 cascade;
+
+create table ref (a int primary key);
+create table ref2 (x int);
+alter table ref2 add constraint fk foreign key (x) references ref (a);
+select create_reference_table('ref');
+-- we can also define more foreign keys after creating reference
+-- table from referenced table
+alter table ref2 add constraint fk2 foreign key (x) references ref (a);
+-- then we can create reference table from referencing table
+select create_reference_table('ref2');
+
+drop table if exists ref, ref2, other cascade;
+
+
+-- add coordinator to pg_dist_node for rest of the tests
+SELECT 1 FROM master_add_node('localhost', :master_port, groupId => 0);
+
+CREATE TABLE local_table_1 (col_1 INT UNIQUE);
+CREATE TABLE local_table_2 (col_1 INT UNIQUE);
+CREATE TABLE local_table_3 (col_1 INT UNIQUE);
+CREATE TABLE local_table_4 (col_1 INT UNIQUE);
+INSERT INTO local_table_1 SELECT i FROM generate_series(195, 205) i;
+INSERT INTO local_table_2 SELECT i FROM generate_series(195, 205) i;
+INSERT INTO local_table_3 SELECT i FROM generate_series(195, 205) i;
+INSERT INTO local_table_4 SELECT i FROM generate_series(195, 205) i;
+
+--                                        _
+--                                       | |
+--                                       | v
+-- local_table_2 -> local_table_1 -> local_table_4
+--                      ^ |               |
+--                      | v               |
+--                  local_table_3 <--------
+ALTER TABLE local_table_2 ADD CONSTRAINT fkey_1 FOREIGN KEY (col_1) REFERENCES local_table_1(col_1);
+ALTER TABLE local_table_3 ADD CONSTRAINT fkey_2 FOREIGN KEY (col_1) REFERENCES local_table_1(col_1);
+ALTER TABLE local_table_1 ADD CONSTRAINT fkey_3 FOREIGN KEY (col_1) REFERENCES local_table_3(col_1);
+ALTER TABLE local_table_1 ADD CONSTRAINT fkey_4 FOREIGN KEY (col_1) REFERENCES local_table_4(col_1);
+ALTER TABLE local_table_4 ADD CONSTRAINT fkey_5 FOREIGN KEY (col_1) REFERENCES local_table_3(col_1);
+ALTER TABLE local_table_4 ADD CONSTRAINT fkey_6 FOREIGN KEY (col_1) REFERENCES local_table_4(col_1);
+
+CREATE TABLE reference_table_1(col_1 INT UNIQUE, col_2 INT);
+INSERT INTO reference_table_1 SELECT i FROM generate_series(195, 205) i;
+SELECT create_reference_table('reference_table_1');
+
+CREATE TABLE partitioned_table_1 (col_1 INT, col_2 INT) PARTITION BY RANGE (col_1);
+CREATE TABLE partitioned_table_1_100_200 PARTITION OF partitioned_table_1 FOR VALUES FROM (100) TO (200);
+CREATE TABLE partitioned_table_1_200_300 PARTITION OF partitioned_table_1 FOR VALUES FROM (200) TO (300);
+INSERT INTO partitioned_table_1 SELECT i FROM generate_series(195, 205) i;
+
+ALTER TABLE partitioned_table_1 ADD CONSTRAINT fkey_8 FOREIGN KEY (col_1) REFERENCES local_table_4(col_1);
+
+-- now that we attached partitioned table to graph below errors out
+-- since we cannot create citus local table from partitioned tables
+ALTER TABLE reference_table_1 ADD CONSTRAINT fkey_9 FOREIGN KEY (col_1) REFERENCES local_table_1(col_1);
+
+ALTER TABLE partitioned_table_1 DROP CONSTRAINT fkey_8;
+
+BEGIN;
+  -- now that we detached partitioned table from graph, succeeds
+  ALTER TABLE reference_table_1 ADD CONSTRAINT fkey_10 FOREIGN KEY (col_1) REFERENCES local_table_1(col_1);
+
+  -- show that we converted all 4 local tables in this schema to citus local tables
+  SELECT COUNT(*)=4 FROM citus_local_tables_in_schema;
+ROLLBACK;
+
+-- this actually attempts to convert local tables to citus local tables but errors out
+-- as citus doesn't support defining foreign keys via add column commands
+ALTER TABLE local_table_1 ADD COLUMN col_3 INT REFERENCES reference_table_1(col_1);
+
+BEGIN;
+  -- define a foreign key so that all 4 local tables become citus local tables
+  ALTER TABLE local_table_1 ADD CONSTRAINT fkey_11 FOREIGN KEY (col_1) REFERENCES reference_table_1(col_1);
+
+  CREATE TABLE local_table_5 (col_1 INT UNIQUE);
+  -- now define foreign key from local to citus local table
+  ALTER TABLE local_table_5 ADD CONSTRAINT fkey_12 FOREIGN KEY (col_1) REFERENCES local_table_2(col_1);
+
+  -- now we have 5 citus local tables in this schema
+  SELECT COUNT(*)=5 FROM citus_local_tables_in_schema;
+ROLLBACK;
+
+-- they fail as local_table_99 does not exist
+ALTER TABLE local_table_99 ADD CONSTRAINT fkey FOREIGN KEY (col_1) REFERENCES reference_table_1(col_1);
+ALTER TABLE local_table_1 ADD CONSTRAINT fkey FOREIGN KEY (col_1) REFERENCES local_table_99(col_1);
+
+-- they fail as col_99 does not exist
+ALTER TABLE local_table_1 ADD CONSTRAINT fkey FOREIGN KEY (col_99) REFERENCES reference_table_1(col_1);
+ALTER TABLE local_table_1 ADD CONSTRAINT fkey FOREIGN KEY (col_1) REFERENCES reference_table_1(col_99);
+
+-- fails as col_2 does not have a unique/primary key constraint
+ALTER TABLE local_table_1 ADD CONSTRAINT fkey FOREIGN KEY (col_1) REFERENCES reference_table_1(col_2);
+
+CREATE TABLE reference_table_2 (col_1 INT UNIQUE, col_2 INT);
+INSERT INTO reference_table_2 SELECT i FROM generate_series(195, 205) i;
+
+BEGIN;
+  -- define foreign key when both ends are local tables
+  ALTER TABLE local_table_1 ADD CONSTRAINT fkey_11 FOREIGN KEY (col_1) REFERENCES reference_table_2(col_1);
+
+  -- we still don't convert local tables to citus local tables when
+  -- creating reference tables
+  SELECT create_reference_table('reference_table_2');
+
+  -- now defining another foreign key would convert local tables to
+  -- citus local tables as reference_table_2 is now a reference table
+  ALTER TABLE local_table_1 ADD CONSTRAINT fkey_12 FOREIGN KEY (col_1) REFERENCES reference_table_2(col_1);
+
+  -- now print metadata to show that everyting is fine
+  SELECT logicalrelid::text AS tablename, partmethod, repmodel FROM pg_dist_partition
+  WHERE logicalrelid::text IN (SELECT tablename FROM pg_tables WHERE schemaname='fkeys_between_local_ref')
+  ORDER BY tablename;
+ROLLBACK;
+
+-- we don't support foreign keys from citus local to reference tables
+-- with ON DELETE/UPDATE CASCADE behavior, so below two errors out
+
+BEGIN;
+  SELECT create_reference_table('reference_table_2');
+  ALTER TABLE reference_table_2 ADD CONSTRAINT fkey_11 FOREIGN KEY (col_1) REFERENCES local_table_2(col_1) ON DELETE CASCADE;
+ROLLBACK;
+
+BEGIN;
+  SELECT create_reference_table('reference_table_2');
+  ALTER TABLE reference_table_2 ADD CONSTRAINT fkey_local_to_ref FOREIGN KEY (col_1) REFERENCES local_table_2(col_1) ON UPDATE CASCADE;
+ROLLBACK;
+
+-- but we support such foreign key behaviors when foreign key is from
+-- citus local to reference table
+
+BEGIN;
+  SELECT create_reference_table('reference_table_2');
+  ALTER TABLE local_table_2 ADD CONSTRAINT fkey_11 FOREIGN KEY (col_1) REFERENCES reference_table_2(col_1) ON DELETE CASCADE;
+
+  DELETE FROM reference_table_2 WHERE col_1=200;
+  -- we deleted one row as DELETE cascades, so we should have 10 rows
+  SELECT COUNT(*) FROM local_table_2;
+ROLLBACK;
+
+BEGIN;
+  SELECT create_reference_table('reference_table_2');
+  ALTER TABLE local_table_2 ADD CONSTRAINT fkey_11 FOREIGN KEY (col_1) REFERENCES reference_table_2(col_1) ON UPDATE CASCADE;
+ROLLBACK;
+
+-- cleanup at exit
+DROP SCHEMA fkeys_between_local_ref CASCADE;