From a30c791eeb85f0d191c39601b48526e64bb477e8 Mon Sep 17 00:00:00 2001
From: Nitish Upreti <niupre@microsoft.com>
Date: Fri, 8 Jul 2022 22:23:38 -0700
Subject: [PATCH] Refactor code from review comments

---
 .../citus_split_shard_by_split_points.c       |  2 +-
 .../distributed/operations/shard_split.c      | 54 ++++++++++++++-----
 2 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/src/backend/distributed/operations/citus_split_shard_by_split_points.c b/src/backend/distributed/operations/citus_split_shard_by_split_points.c
index 59b7b26b7..da9c12497 100644
--- a/src/backend/distributed/operations/citus_split_shard_by_split_points.c
+++ b/src/backend/distributed/operations/citus_split_shard_by_split_points.c
@@ -77,11 +77,11 @@ LookupSplitMode(Oid shardSplitModeOid)
 	Datum enumLabelDatum = DirectFunctionCall1(enum_out, shardSplitModeOid);
 	char *enumLabel = DatumGetCString(enumLabelDatum);
 
+	/* Extend with other modes as we support them */
 	if (strncmp(enumLabel, "blocking", NAMEDATALEN) == 0)
 	{
 		shardSplitMode = BLOCKING_SPLIT;
 	}
-	/* Extend with other modes as we support them */
 	else
 	{
 		ereport(ERROR, (errmsg("Invalid split mode: %s. Expected split mode is blocking.",
diff --git a/src/backend/distributed/operations/shard_split.c b/src/backend/distributed/operations/shard_split.c
index 333d81e31..90889629b 100644
--- a/src/backend/distributed/operations/shard_split.c
+++ b/src/backend/distributed/operations/shard_split.c
@@ -38,10 +38,14 @@ static void ErrorIfCannotSplitShardExtended(SplitOperation splitOperation,
 											ShardInterval *shardIntervalToSplit,
 											List *shardSplitPointsList,
 											List *nodeIdsForPlacementList);
-static void CreateSplitShardsForShardGroup(WorkerNode *sourceShardNode,
-										   List *sourceColocatedShardIntervalList,
-										   List *shardGroupSplitIntervalListList,
+static void CreateAndCopySplitShardsForShardGroup(WorkerNode *sourceShardNode,
+												  List *sourceColocatedShardIntervalList,
+												  List *shardGroupSplitIntervalListList,
+												  List *workersForPlacementList);
+static void CreateSplitShardsForShardGroup(List *shardGroupSplitIntervalListList,
 										   List *workersForPlacementList);
+static void CreateAuxiliaryStructuresForShardGroup(List *shardGroupSplitIntervalListList,
+												   List *workersForPlacementList);
 static void CreateObjectOnPlacement(List *objectCreationCommandList,
 									WorkerNode *workerNode);
 static List *    CreateSplitIntervalsForShardGroup(List *sourceColocatedShardList,
@@ -420,11 +424,11 @@ BlockingShardSplit(SplitOperation splitOperation,
 	PG_TRY();
 	{
 		/*
-		 * Physically create split children, perform split copy and create auxillary structures.
+		 * Physically create split children, perform split copy and create auxiliary structures.
 		 * This includes: indexes, replicaIdentity. triggers and statistics.
 		 * Foreign key constraints are created after Metadata changes (see CreateForeignKeyConstraints).
 		 */
-		CreateSplitShardsForShardGroup(
+		CreateAndCopySplitShardsForShardGroup(
 			sourceShardToCopyNode,
 			sourceColocatedShardIntervalList,
 			shardGroupSplitIntervalListList,
@@ -465,9 +469,7 @@ BlockingShardSplit(SplitOperation splitOperation,
 
 /* Create ShardGroup split children on a list of corresponding workers. */
 static void
-CreateSplitShardsForShardGroup(WorkerNode *sourceShardNode,
-							   List *sourceColocatedShardIntervalList,
-							   List *shardGroupSplitIntervalListList,
+CreateSplitShardsForShardGroup(List *shardGroupSplitIntervalListList,
 							   List *workersForPlacementList)
 {
 	/* Iterate on shard interval list for shard group */
@@ -493,14 +495,20 @@ CreateSplitShardsForShardGroup(WorkerNode *sourceShardNode,
 			CreateObjectOnPlacement(splitShardCreationCommandList, workerPlacementNode);
 		}
 	}
+}
 
-	/* Perform Split Copy */
-	DoSplitCopy(sourceShardNode, sourceColocatedShardIntervalList,
-				shardGroupSplitIntervalListList, workersForPlacementList);
 
+/* Create ShardGroup auxiliary structures (indexes, stats, replicaindentities, triggers)
+ * on a list of corresponding workers.
+ */
+static void
+CreateAuxiliaryStructuresForShardGroup(List *shardGroupSplitIntervalListList,
+									   List *workersForPlacementList)
+{
 	/*
-	 * Create auxillary structures post copy.
+	 * Create auxiliary structures post copy.
 	 */
+	List *shardIntervalList = NULL;
 	foreach_ptr(shardIntervalList, shardGroupSplitIntervalListList)
 	{
 		/* Iterate on split shard interval list and corresponding placement worker */
@@ -523,6 +531,28 @@ CreateSplitShardsForShardGroup(WorkerNode *sourceShardNode,
 }
 
 
+/*
+ * Create ShardGroup split children, perform copy and create auxiliary structures
+ * on a list of corresponding workers.
+ */
+static void
+CreateAndCopySplitShardsForShardGroup(WorkerNode *sourceShardNode,
+									  List *sourceColocatedShardIntervalList,
+									  List *shardGroupSplitIntervalListList,
+									  List *workersForPlacementList)
+{
+	CreateSplitShardsForShardGroup(shardGroupSplitIntervalListList,
+								   workersForPlacementList);
+
+	DoSplitCopy(sourceShardNode, sourceColocatedShardIntervalList,
+				shardGroupSplitIntervalListList, workersForPlacementList);
+
+	/* Create auxiliary structures (indexes, stats, replicaindentities, triggers) */
+	CreateAuxiliaryStructuresForShardGroup(shardGroupSplitIntervalListList,
+										   workersForPlacementList);
+}
+
+
 /*
  * Perform Split Copy from source shard(s) to split children.
  * 'sourceShardNode'					: Source shard worker node.