From 97a1720a1793a61e01e65d9fc0888d0a3d0e1b77 Mon Sep 17 00:00:00 2001
From: Burak Velioglu <buvelio@microsoft.com>
Date: Wed, 19 Jan 2022 23:31:14 +0300
Subject: [PATCH] Address reviews

---
 .../citus_add_local_table_to_metadata.c       |  8 +---
 .../commands/create_distributed_table.c       |  5 +--
 .../distributed/metadata/metadata_sync.c      | 41 +++++++++++++++----
 src/include/distributed/metadata_sync.h       |  1 +
 4 files changed, 37 insertions(+), 18 deletions(-)

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 20486c332..c4bdf946b 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
@@ -1241,13 +1241,7 @@ FinalizeCitusLocalTableCreation(Oid relationId, List *dependentSequenceList)
 
 	if (ShouldSyncTableMetadata(relationId))
 	{
-		CreateShellTableOnWorkers(relationId);
-		ObjectAddress relationAddress = { 0 };
-		ObjectAddressSet(relationAddress, RelationRelationId, relationId);
-		MarkObjectDistributed(&relationAddress);
-
-		CreateTableMetadataOnWorkers(relationId);
-		CreateInterTableRelationshipOfRelationOnWorkers(relationId);
+		SyncCitusTableMetadata(relationId);
 	}
 
 	/*
diff --git a/src/backend/distributed/commands/create_distributed_table.c b/src/backend/distributed/commands/create_distributed_table.c
index 1ee67fe46..01f4cd739 100644
--- a/src/backend/distributed/commands/create_distributed_table.c
+++ b/src/backend/distributed/commands/create_distributed_table.c
@@ -536,10 +536,7 @@ CreateDistributedTable(Oid relationId, Var *distributionColumn, char distributio
 
 	if (ShouldSyncTableMetadata(relationId))
 	{
-		CreateShellTableOnWorkers(relationId);
-		MarkObjectDistributed(&tableAddress);
-		CreateTableMetadataOnWorkers(relationId);
-		CreateInterTableRelationshipOfRelationOnWorkers(relationId);
+		SyncCitusTableMetadata(relationId);
 	}
 
 	/*
diff --git a/src/backend/distributed/metadata/metadata_sync.c b/src/backend/distributed/metadata/metadata_sync.c
index 849981b34..be3381d43 100644
--- a/src/backend/distributed/metadata/metadata_sync.c
+++ b/src/backend/distributed/metadata/metadata_sync.c
@@ -239,6 +239,21 @@ SyncNodeMetadataToNode(const char *nodeNameString, int32 nodePort)
 }
 
 
+/*
+ * SyncCitusTableMetadata syncs citus table metadata to worker nodes with metadata.
+ * Our definition of metadata includes the shell table and its inter relations with
+ * other shell tables, corresponding pg_dist_object, pg_dist_partiton, pg_dist_shard
+ * and pg_dist_shard placement entries.
+ */
+void
+SyncCitusTableMetadata(Oid relationId)
+{
+	CreateShellTableOnWorkers(relationId);
+	CreateTableMetadataOnWorkers(relationId);
+	CreateInterTableRelationshipOfRelationOnWorkers(relationId);
+}
+
+
 /*
  * EnsureSequentialModeMetadataOperations makes sure that the current transaction is
  * already in sequential mode, or can still safely be put in sequential mode,
@@ -650,6 +665,17 @@ GetDistributedTableMetadataEvents(Oid relationId)
 	char *metadataCommand = DistributionCreateCommand(cacheEntry);
 	commandList = lappend(commandList, metadataCommand);
 
+	/*
+	 * Commands to create the truncate trigger of the table. We are creating
+	 * that as a part of metadata since truncate trigger handles the metadata
+	 * while dropping the table.
+	 */
+	if (!IsForeignTable(relationId))
+	{
+		char *truncateTriggerCreateCommand = TruncateTriggerCreateCommand(relationId);
+		commandList = lappend(commandList, truncateTriggerCreateCommand);
+	}
+
 	/* commands to insert pg_dist_shard & pg_dist_placement entries */
 	List *shardIntervalList = LoadShardIntervalList(relationId);
 	List *shardMetadataInsertCommandList = ShardListInsertCommand(shardIntervalList);
@@ -1899,13 +1925,6 @@ CreateShellTableOnWorkers(Oid relationId)
 	List *sequenceDependencyCommandList = SequenceDependencyCommandList(relationId);
 	commandList = list_concat(commandList, sequenceDependencyCommandList);
 
-	/* commands to create the truncate trigger of the table */
-	if (!IsForeignTable(relationId))
-	{
-		char *truncateTriggerCreateCommand = TruncateTriggerCreateCommand(relationId);
-		commandList = lappend(commandList, truncateTriggerCreateCommand);
-	}
-
 	/* prevent recursive propagation */
 	SendCommandToWorkersWithMetadata(DISABLE_DDL_PROPAGATION);
 
@@ -1914,6 +1933,14 @@ CreateShellTableOnWorkers(Oid relationId)
 	{
 		SendCommandToWorkersWithMetadata(command);
 	}
+
+	/*
+	 * Mark the table object as distributed at the end as we need to propagate
+	 * that table to new nodes anyway.
+	 */
+	ObjectAddress relationAddress = { 0 };
+	ObjectAddressSet(relationAddress, RelationRelationId, relationId);
+	MarkObjectDistributed(&relationAddress);
 }
 
 
diff --git a/src/include/distributed/metadata_sync.h b/src/include/distributed/metadata_sync.h
index 1fd4fe790..bfdc8f506 100644
--- a/src/include/distributed/metadata_sync.h
+++ b/src/include/distributed/metadata_sync.h
@@ -29,6 +29,7 @@ typedef enum
 
 /* Functions declarations for metadata syncing */
 extern void SyncNodeMetadataToNode(const char *nodeNameString, int32 nodePort);
+extern void SyncCitusTableMetadata(Oid relationId);
 extern bool ClusterHasKnownMetadataWorkers(void);
 extern char * LocalGroupIdUpdateCommand(int32 groupId);
 extern bool ShouldSyncTableMetadata(Oid relationId);