From ae963a1e945bad1932221990d72b09deac42dc20 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy 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