Some stylistic improvements in the foreign keys to reference table

changes.

src/backend/distributed/commands/create_distributed_table.c

@@ -382,6 +382,15 @@ CreateDistributedTable(Oid relationId, Var *distributionColumn, char distributio
 		CreateTableMetadataOnWorkers(relationId);
 	}
 
+	/*
+	 * We've a custom way of foreign key graph invalidation,
+	 * see InvalidateForeignKeyGraph().
+	 */
+	if (TableReferenced(relationId) || TableReferencing(relationId))
+	{
+		InvalidateForeignKeyGraph();
+	}
+
 	/* if this table is partitioned table, distribute its partitions too */
 	if (PartitionedTable(relationId))
 	{
@@ -761,13 +770,8 @@ EnsureRelationCanBeDistributed(Oid relationId, Var *distributionColumn,
 	ErrorIfUnsupportedConstraint(relation, distributionMethod, distributionColumn,
 								 colocationId);
 
-	if (TableReferenced(relationId) || TableReferencing(relationId))
-	{
-		InvalidateForeignKeyGraph();
-	}
 
 	ErrorIfUnsupportedPolicy(relation);
-
 	relation_close(relation, NoLock);
 }
 

src/backend/distributed/executor/multi_utility.c

@@ -178,7 +178,7 @@ static void InvalidateForeignKeyGraphForDDL(void);
  * We need to run some of the commands sequentially if there is a foreign constraint
  * from/to reference table.
  */
-static bool ShouldExecuteAlterTableSequentially(Oid relationId, AlterTableCmd *command);
+static bool SetupExecutionModeForAlterTable(Oid relationId, AlterTableCmd *command);
 
 
 /*
@@ -1450,8 +1450,8 @@ PlanAlterTableStmt(AlterTableStmt *alterTableStatement, const char *alterTableCo
 			rightRelationId = RangeVarGetRelid(partitionCommand->name, NoLock, false);
 		}
 #endif
-		executeSequentially |= ShouldExecuteAlterTableSequentially(leftRelationId,
-																   command);
+		executeSequentially |= SetupExecutionModeForAlterTable(leftRelationId,
+															   command);
 	}
 
 	ddlJob = palloc0(sizeof(DDLJob));
@@ -4002,17 +4002,30 @@ ProcessDropTableStmt(DropStmt *dropTableStatement)
 
 
 /*
- * ShouldExecuteAlterTableSequentially checks if the given ALTER TABLE
- * statements should be executed sequentially when there is a foreign
- * constraint from a distributed table to a reference table.
- * In case of a column related ALTER TABLE operation, we check explicitly
- * if there is a foreign constraint on this column from/to a reference table.
- * Additionally, if the command is run inside a transaction block, we call
- * SetLocalMultiShardModifyModeToSequential so that the further commands
- * in the same transaction uses the same connections and does not error out.
+ * SetupExecutionModeForAlterTable is the function that is responsible
+ * for two things for practial purpose for not doing the same checks
+ * twice:
+ *     (a) For any command, decide and return whether we should
+ *         run the command in sequntial mode
+ *     (b) For commands in a transaction block, set the transaction local
+ *         multi-shard modify mode to sequential when necessary
+ *
+ * The commands that operate on the same reference table shard in parallel
+ * is in the interest of (a), where the return value indicates the executor
+ * to run the command sequentially to prevent self-deadlocks.
+ *
+ * The commands that both operate on the same reference table shard in parallel
+ * and cascades to run any parallel operation is in the interest of (b). By
+ * setting the multi-shard mode, we ensure that the cascading parallel commands
+ * are executed sequentially to prevent self-deadlocks.
+ *
+ * One final note on the function is that if the function decides to execute
+ * the command in sequential mode, and a parallel command has already been
+ * executed in the same transaction, the function errors out. See the comment
+ * in the function for the rationale.
  */
 static bool
-ShouldExecuteAlterTableSequentially(Oid relationId, AlterTableCmd *command)
+SetupExecutionModeForAlterTable(Oid relationId, AlterTableCmd *command)
 {
 	bool executeSequentially = false;
 	AlterTableType alterTableType = command->subtype;

src/backend/distributed/master/master_repair_shards.c

@@ -419,10 +419,7 @@ CopyShardForeignConstraintCommandListGrouped(ShardInterval *shardInterval,
 
 		if (PartitionMethod(referencedRelationId) == DISTRIBUTE_BY_NONE)
 		{
-			List *shardList = LoadShardList(referencedRelationId);
-			uint64 *shardIdPointer = (uint64 *) linitial(shardList);
-
-			referencedShardId = (*shardIdPointer);
+			referencedShardId = GetFirstShardId(referencedRelationId);
 		}
 		else
 		{
@@ -453,6 +450,21 @@ CopyShardForeignConstraintCommandListGrouped(ShardInterval *shardInterval,
 }
 
 
+/*
+ * GetFirstShardId is a helper function which returns the first
+ * shardId of the given distributed relation. The function doesn't
+ * sort the shardIds, so it is mostly useful for reference tables.
+ */
+uint64
+GetFirstShardId(Oid relationId)
+{
+	List *shardList = LoadShardList(relationId);
+	uint64 *shardIdPointer = (uint64 *) linitial(shardList);
+
+	return (*shardIdPointer);
+}
+
+
 /*
  * ConstuctQualifiedShardName creates the fully qualified name string of the
  * given shard in <schema>.<table_name>_<shard_id> format.

src/backend/distributed/master/master_stage_protocol.c

@@ -673,10 +673,7 @@ WorkerCreateShard(Oid relationId, int shardIndex, uint64 shardId, List *ddlComma
 		}
 		else if (PartitionMethod(referencedRelationId) == DISTRIBUTE_BY_NONE)
 		{
-			List *shardList = LoadShardList(referencedRelationId);
-			uint64 *shardIdPointer = (uint64 *) linitial(shardList);
-
-			referencedShardId = (*shardIdPointer);
+			referencedShardId = GetFirstShardId(referencedRelationId);
 		}
 		else
 		{

src/backend/distributed/transaction/relation_access_tracking.c

@@ -33,6 +33,8 @@
 bool EnforceForeignKeyRestrictions = true;
 
 #define PARALLEL_MODE_FLAG_OFFSET 3
+
+/* simply set parallel bits as defined below for select, dml and ddl */
 #define PARALLEL_ACCESS_MASK (int) (0 | \
 									(1 << (PLACEMENT_ACCESS_SELECT + \
 										   PARALLEL_MODE_FLAG_OFFSET)) | \

src/include/distributed/master_metadata_utility.h

@@ -159,6 +159,7 @@ extern void EnsureSuperUser(void);
 extern void EnsureReplicationSettings(Oid relationId, char replicationModel);
 extern bool RegularTable(Oid relationId);
 extern char * ConstructQualifiedShardName(ShardInterval *shardInterval);
+extern uint64 GetFirstShardId(Oid relationId);
 extern Datum StringToDatum(char *inputString, Oid dataType);
 extern char * DatumToString(Datum datum, Oid dataType);
 

src/test/regress/expected/foreign_key_restriction_enforcement.out

@@ -1001,12 +1001,12 @@ DEBUG:  schema "test_fkey_to_ref_in_tx" already exists, skipping
 DETAIL:  NOTICE from localhost:57638
 DEBUG:  schema "test_fkey_to_ref_in_tx" already exists, skipping
 DETAIL:  NOTICE from localhost:57637
-DEBUG:  switching to sequential query execution mode
-DETAIL:  Reference relation "test_table_1" is modified, which might lead to data inconsistencies or distributed deadlocks via parallel accesses to hash distributed relations due to foreign keys. Any parallel modification to those hash distributed relations in the same transaction can only be executed in sequential query execution mode
 DEBUG:  schema "test_fkey_to_ref_in_tx" already exists, skipping
 DETAIL:  NOTICE from localhost:57637
 DEBUG:  schema "test_fkey_to_ref_in_tx" already exists, skipping
 DETAIL:  NOTICE from localhost:57638
+DEBUG:  switching to sequential query execution mode
+DETAIL:  Reference relation "test_table_1" is modified, which might lead to data inconsistencies or distributed deadlocks via parallel accesses to hash distributed relations due to foreign keys. Any parallel modification to those hash distributed relations in the same transaction can only be executed in sequential query execution mode
 NOTICE:  Copying data from local table...
 DEBUG:  Copied 101 rows
  create_reference_table