From ae963a1e945bad1932221990d72b09deac42dc20 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Thu, 17 Dec 2020 14:13:27 +0530
Subject: [PATCH v2] Fail Fast In CTAS/CMV If Relation Already Exists

Currently, for CTAS or CREATE MATERIALIZED VIEW(CMV) without
IF-NOT-EXISTS clause, the existence of the relation (either
table or materialized view) gets checked during execution and an
error is thrown there. All the unnecessary rewrite and planning
for the SELECT part of the query have happened just to fail
later. However, if IF-NOT-EXISTS clause is present, then a notice
is issued and returned immediately without rewrite and planning
further. This seems somewhat inconsistent.

This patch propose to check the relation existence early in
ExecCreateTableAs() as well as in ExplainOneUtility() and
throw an error in case it exists already to avoid unnecessary
rewrite, planning and execution of the SELECT part.
---
 src/backend/commands/createas.c | 69 ++++++++++++++++++++++++++-------
 src/backend/commands/explain.c  | 12 ++++++
 src/include/commands/createas.h |  2 +
 3 files changed, 69 insertions(+), 14 deletions(-)

diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 6bf6c5a310..2f4d865494 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -238,22 +238,21 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
 	List	   *rewritten;
 	PlannedStmt *plan;
 	QueryDesc  *queryDesc;
+	bool		rel_exists;
 
-	if (stmt->if_not_exists)
-	{
-		Oid			nspid;
-
-		nspid = RangeVarGetCreationNamespace(stmt->into->rel);
+	/*
+	 * Check if the relation that is going to get created exists already. If
+	 * yes and there is no if-not-exists clause, then emit error early so that
+	 * we can avoid planning for the SELECT part unnecessarily which happens
+	 * otherwise only to fail later.
+	 *
+	 * If there is if-not-exists clause, we issue a NOTICE and return from here
+	 * with invalid object address.
+	 */
+	rel_exists = CheckRelExistenceInCTAS(stmt, false);
 
-		if (get_relname_relid(stmt->into->rel->relname, nspid))
-		{
-			ereport(NOTICE,
-					(errcode(ERRCODE_DUPLICATE_TABLE),
-					 errmsg("relation \"%s\" already exists, skipping",
-							stmt->into->rel->relname)));
-			return InvalidObjectAddress;
-		}
-	}
+	if (stmt->if_not_exists && rel_exists)
+		return InvalidObjectAddress;
 
 	/*
 	 * Create the tuple receiver object and insert info it will need
@@ -400,6 +399,48 @@ GetIntoRelEFlags(IntoClause *intoClause)
 	return flags;
 }
 
+/*
+ * CheckRelExistenceInCTAS --- check whether a specified relation in CTAS
+ * already exists.
+ *
+ * We do this before any planning occurs for the SELECT part to avoid planning
+ * cost. If the relation exists: 1) issue NOTICE and return true for plain i.e.
+ * non-explain queries with if-not-exists clause. 2) emit an ERROR for both
+ * plain queries without if-not-exists clause and explain queries irrespective
+ * of if-not-exists clause. Return false if the relation does not exist.
+ */
+bool CheckRelExistenceInCTAS(CreateTableAsStmt *ctas, bool is_explain)
+{
+	Oid nspid;
+
+	nspid = RangeVarGetCreationNamespace(ctas->into->rel);
+
+	if (get_relname_relid(ctas->into->rel->relname, nspid))
+	{
+		/*
+		 * Issue notice in case if-not-exists clause is present, otherwise an
+		 * error is emitted. However for explains we throw error irrespective
+		 * of if-not-exists clause, as we do not want to show a notice and an
+		 * empty plan in the output which can happen if a notice is issued.
+		 */
+		if (!is_explain && ctas->if_not_exists)
+			ereport(NOTICE,
+					(errcode(ERRCODE_DUPLICATE_TABLE),
+					 errmsg("relation \"%s\" already exists, skipping",
+							ctas->into->rel->relname)));
+		else
+			ereport(ERROR,
+					(errcode(ERRCODE_DUPLICATE_TABLE),
+					 errmsg("relation \"%s\" already exists",
+							ctas->into->rel->relname)));
+
+		return true;
+	}
+
+	/* Relation does not exist. */
+	return false;
+}
+
 /*
  * CreateIntoRelDestReceiver -- create a suitable DestReceiver object
  *
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 43f9b01e83..9238a4a3a6 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -435,6 +435,18 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es,
 		CreateTableAsStmt *ctas = (CreateTableAsStmt *) utilityStmt;
 		List	   *rewritten;
 
+		/*
+		 * Check if the relation that is going to get created exists already.
+		 * If yes, then emit error early so that we can avoid planning for
+		 * the SELECT part unnecessarily which happens otherwise only to fail
+		 * later.
+		 *
+		 * We intentionally ignore the return value of CheckRelExistenceInCTAS
+		 * as it returns for explains only when the relation does not exist
+		 * otherwise it throws an error and does not come here.
+		 */
+		(void) CheckRelExistenceInCTAS(ctas, true);
+
 		rewritten = QueryRewrite(castNode(Query, copyObject(ctas->query)));
 		Assert(list_length(rewritten) == 1);
 		ExplainOneQuery(linitial_node(Query, rewritten),
diff --git a/src/include/commands/createas.h b/src/include/commands/createas.h
index 7629230254..33d4d9701f 100644
--- a/src/include/commands/createas.h
+++ b/src/include/commands/createas.h
@@ -29,4 +29,6 @@ extern int	GetIntoRelEFlags(IntoClause *intoClause);
 
 extern DestReceiver *CreateIntoRelDestReceiver(IntoClause *intoClause);
 
+extern bool CheckRelExistenceInCTAS(CreateTableAsStmt *ctas, bool is_explain);
+
 #endif							/* CREATEAS_H */
-- 
2.25.1

