From 077c784fe941f747b6fa32049c5c3dd0fde2ca11 Mon Sep 17 00:00:00 2001
From: SaitTalhaNisanci <s.talhanisanci@gmail.com>
Date: Sun, 14 Jun 2020 14:25:58 +0300
Subject: [PATCH] Create EnsureTableCanBeCreated for some checks (#3839)

---
 .../commands/create_distributed_table.c       | 63 +++++++++----------
 1 file changed, 31 insertions(+), 32 deletions(-)

diff --git a/src/backend/distributed/commands/create_distributed_table.c b/src/backend/distributed/commands/create_distributed_table.c
index 7f10a40fb..6b19ec636 100644
--- a/src/backend/distributed/commands/create_distributed_table.c
+++ b/src/backend/distributed/commands/create_distributed_table.c
@@ -106,6 +106,7 @@ static void EnsureLocalTableEmptyIfNecessary(Oid relationId, char distributionMe
 											 bool viaDeprecatedAPI);
 static bool ShouldLocalTableBeEmpty(Oid relationId, char distributionMethod, bool
 									viaDeprecatedAPI);
+static void EnsureCitusTableCanBeCreated(Oid relationOid);
 static bool LocalTableEmpty(Oid tableId);
 static void CopyLocalDataIntoShards(Oid relationId);
 static List * TupleDescColumnNameList(TupleDesc tupleDescriptor);
@@ -139,15 +140,15 @@ master_create_distributed_table(PG_FUNCTION_ARGS)
 	text *distributionColumnText = PG_GETARG_TEXT_P(1);
 	Oid distributionMethodOid = PG_GETARG_OID(2);
 
+	CheckCitusVersion(ERROR);
+
+	EnsureCitusTableCanBeCreated(relationId);
+
 	char *colocateWithTableName = NULL;
 	bool viaDeprecatedAPI = true;
 	ObjectAddress tableAddress = { 0 };
 
 
-	CheckCitusVersion(ERROR);
-	EnsureCoordinator();
-	EnsureTableOwner(relationId);
-
 	/*
 	 * distributed tables might have dependencies on different objects, since we create
 	 * shards for a distributed table via multiple sessions these objects will be created
@@ -170,13 +171,6 @@ master_create_distributed_table(PG_FUNCTION_ARGS)
 							   "relation does not exist")));
 	}
 
-	/*
-	 * We should do this check here since the codes in the following lines rely
-	 * on this relation to have a supported relation kind. More extensive checks
-	 * will be performed in CreateDistributedTable.
-	 */
-	EnsureRelationKindSupported(relationId);
-
 	char *distributionColumnName = text_to_cstring(distributionColumnText);
 	Var *distributionColumn = BuildDistributionKeyFromColumnName(relation,
 																 distributionColumnName);
@@ -202,18 +196,16 @@ create_distributed_table(PG_FUNCTION_ARGS)
 {
 	ObjectAddress tableAddress = { 0 };
 
-
 	bool viaDeprecatedAPI = false;
 
-	CheckCitusVersion(ERROR);
-	EnsureCoordinator();
-
 	Oid relationId = PG_GETARG_OID(0);
 	text *distributionColumnText = PG_GETARG_TEXT_P(1);
 	Oid distributionMethodOid = PG_GETARG_OID(2);
 	text *colocateWithTableNameText = PG_GETARG_TEXT_P(3);
 
-	EnsureTableOwner(relationId);
+	CheckCitusVersion(ERROR);
+
+	EnsureCitusTableCanBeCreated(relationId);
 
 	/*
 	 * distributed tables might have dependencies on different objects, since we create
@@ -237,13 +229,6 @@ create_distributed_table(PG_FUNCTION_ARGS)
 							   "relation does not exist")));
 	}
 
-	/*
-	 * We should do this check here since the codes in the following lines rely
-	 * on this relation to have a supported relation kind. More extensive checks
-	 * will be performed in CreateDistributedTable.
-	 */
-	EnsureRelationKindSupported(relationId);
-
 	char *distributionColumnName = text_to_cstring(distributionColumnText);
 	Var *distributionColumn = BuildDistributionKeyFromColumnName(relation,
 																 distributionColumnName);
@@ -277,9 +262,9 @@ create_reference_table(PG_FUNCTION_ARGS)
 
 	bool viaDeprecatedAPI = false;
 
-	EnsureCoordinator();
 	CheckCitusVersion(ERROR);
-	EnsureTableOwner(relationId);
+
+	EnsureCitusTableCanBeCreated(relationId);
 
 	/*
 	 * distributed tables might have dependencies on different objects, since we create
@@ -297,13 +282,6 @@ create_reference_table(PG_FUNCTION_ARGS)
 	 */
 	Relation relation = relation_open(relationId, ExclusiveLock);
 
-	/*
-	 * We should do this check here since the codes in the following lines rely
-	 * on this relation to have a supported relation kind. More extensive checks
-	 * will be performed in CreateDistributedTable.
-	 */
-	EnsureRelationKindSupported(relationId);
-
 	List *workerNodeList = ActivePrimaryNodeList(ShareLock);
 	int workerCount = list_length(workerNodeList);
 
@@ -326,6 +304,27 @@ create_reference_table(PG_FUNCTION_ARGS)
 }
 
 
+/*
+ * EnsureCitusTableCanBeCreated checks if
+ * - we are on the coordinator
+ * - the current user is the owner of the table
+ * - relation kind is supported
+ */
+static void
+EnsureCitusTableCanBeCreated(Oid relationOid)
+{
+	EnsureCoordinator();
+	EnsureTableOwner(relationOid);
+
+	/*
+	 * We should do this check here since the codes in the following lines rely
+	 * on this relation to have a supported relation kind. More extensive checks
+	 * will be performed in CreateDistributedTable.
+	 */
+	EnsureRelationKindSupported(relationOid);
+}
+
+
 /*
  * CreateDistributedTable creates distributed table in the given configuration.
  * This functions contains all necessary logic to create distributed tables. It