From 0932ea9a463afccbcc7dd7a38020b07c910502bd Mon Sep 17 00:00:00 2001
From: Burak Velioglu <buvelio@microsoft.com>
Date: Thu, 20 Jan 2022 13:19:14 +0300
Subject: [PATCH] Address reviews

---
 src/backend/distributed/metadata/dependency.c |  1 +
 .../distributed/metadata/metadata_sync.c      | 72 ++++++++++---------
 2 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/src/backend/distributed/metadata/dependency.c b/src/backend/distributed/metadata/dependency.c
index 820cb848f..0a3236a8a 100644
--- a/src/backend/distributed/metadata/dependency.c
+++ b/src/backend/distributed/metadata/dependency.c
@@ -706,6 +706,7 @@ SupportedDependencyByCitus(const ObjectAddress *address)
 /*
  * IsTableOwnedByExtension returns whether the table with the given relation ID is
  * owned by an extension.
+ * TODO: Check what if the extension is not distributed?
  */
 bool
 IsTableOwnedByExtension(Oid relationId)
diff --git a/src/backend/distributed/metadata/metadata_sync.c b/src/backend/distributed/metadata/metadata_sync.c
index 4ac378ff1..853633705 100644
--- a/src/backend/distributed/metadata/metadata_sync.c
+++ b/src/backend/distributed/metadata/metadata_sync.c
@@ -659,17 +659,6 @@ CitusTableMetadataCreateCommandList(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);
@@ -1906,28 +1895,37 @@ InterTableRelationshipOfRelationCommandList(Oid relationId)
 void
 CreateShellTableOnWorkers(Oid relationId)
 {
-	/* if the table is owned by an extension we don't create */
-	bool tableOwnedByExtension = IsTableOwnedByExtension(relationId);
-	if (tableOwnedByExtension)
-	{
-		return;
-	}
-
 	List *commandList = NIL;
-	IncludeSequenceDefaults includeSequenceDefaults = WORKER_NEXTVAL_SEQUENCE_DEFAULTS;
 
-	List *tableDDLCommands = GetFullTableCreationCommands(relationId,
-														  includeSequenceDefaults);
-	TableDDLCommand *tableDDLCommand = NULL;
-	foreach_ptr(tableDDLCommand, tableDDLCommands)
+	/*
+	 * If the table is owned by an extension we only create truncate trigger,
+	 * otherwise we create shell table and sequence dependency as well.
+	 */
+	bool tableOwnedByExtension = IsTableOwnedByExtension(relationId);
+	if (!tableOwnedByExtension)
 	{
-		Assert(CitusIsA(tableDDLCommand, TableDDLCommand));
-		commandList = lappend(commandList, GetTableDDLCommand(tableDDLCommand));
+		IncludeSequenceDefaults includeSequenceDefaults =
+			WORKER_NEXTVAL_SEQUENCE_DEFAULTS;
+
+		List *tableDDLCommands = GetFullTableCreationCommands(relationId,
+															  includeSequenceDefaults);
+		TableDDLCommand *tableDDLCommand = NULL;
+		foreach_ptr(tableDDLCommand, tableDDLCommands)
+		{
+			Assert(CitusIsA(tableDDLCommand, TableDDLCommand));
+			commandList = lappend(commandList, GetTableDDLCommand(tableDDLCommand));
+		}
+
+		/* command to associate sequences with table */
+		List *sequenceDependencyCommandList = SequenceDependencyCommandList(relationId);
+		commandList = list_concat(commandList, sequenceDependencyCommandList);
 	}
 
-	/* command to associate sequences with table */
-	List *sequenceDependencyCommandList = SequenceDependencyCommandList(relationId);
-	commandList = list_concat(commandList, sequenceDependencyCommandList);
+	if (!IsForeignTable(relationId))
+	{
+		char *truncateTriggerCreateCommand = TruncateTriggerCreateCommand(relationId);
+		commandList = lappend(commandList, truncateTriggerCreateCommand);
+	}
 
 	/* prevent recursive propagation */
 	SendCommandToWorkersWithMetadata(DISABLE_DDL_PROPAGATION);
@@ -1938,13 +1936,17 @@ 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);
+	/* once shell table is created, mark the object as distributed as well */
+	if (!tableOwnedByExtension)
+	{
+		/*
+		 * 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);
+	}
 }