On Mon, Dec 21, 2020 at 12:01:38PM +0530, Bharath Rupireddy wrote: > On Fri, Dec 18, 2020 at 8:15 AM Bharath Rupireddy >> I tried to make it consistent by issuing NOTICE (not an error) even >> for EXPLAIN/EXPLAIN ANALYZE IF NOT EXISTS case. If issue notice and >> exit from the ExplainOneUtility, we could output an empty plan to the >> user because, by now ExplainResultDesc would have been called at the >> start of the explain via PortalStart(). I didn't find a clean way of >> coding if we are not okay to show notice and empty plan to the user. >> >> Any suggestions on achieving above?
I was looking at your patch today, and I actually found the conclusion to output an empty plan while issuing a NOTICE to be quite intuitive if the caller uses IF NOT EXISTS with EXPLAIN. > Attaching v3 patch that also contains test cases. Please review it further. Thanks for adding some test cases! Some of them were exact duplicates, so it is possible to reduce the number of queries without impacting the coverage. I have also chosen a query that forces an error within the planner. Please see the attached. IF NOT EXISTS implies that CTAS or CREATE MATVIEW will never ERROR if the relation already exists, with or without EXPLAIN, EXECUTE or WITH NO DATA, so that gets us a consistent behavior across all the patterns. Note: I'd like to think that we could choose a better name for CheckRelExistenceInCTAS(). -- Michael
From 606520d6f023163024ec5f215b45e9a89f093884 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 22 Dec 2020 17:35:22 +0900
Subject: [PATCH v4] 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/include/commands/createas.h | 2 +
src/backend/commands/createas.c | 56 +++++++++++++++++------
src/backend/commands/explain.c | 8 ++++
src/test/regress/expected/matview.out | 38 +++++++++++++++
src/test/regress/expected/select_into.out | 31 +++++++++++++
src/test/regress/sql/matview.sql | 23 ++++++++++
src/test/regress/sql/select_into.sql | 16 +++++++
7 files changed, 159 insertions(+), 15 deletions(-)
diff --git a/src/include/commands/createas.h b/src/include/commands/createas.h
index 7629230254..404adf814b 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);
+
#endif /* CREATEAS_H */
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 6bf6c5a310..2c21b1b87f 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -239,21 +239,9 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
PlannedStmt *plan;
QueryDesc *queryDesc;
- if (stmt->if_not_exists)
- {
- Oid nspid;
-
- nspid = RangeVarGetCreationNamespace(stmt->into->rel);
-
- 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;
- }
- }
+ /* Check if the to-be-created relation exists or not */
+ if (!CheckRelExistenceInCTAS(stmt))
+ return InvalidObjectAddress;
/*
* Create the tuple receiver object and insert info it will need
@@ -400,6 +388,44 @@ GetIntoRelEFlags(IntoClause *intoClause)
return flags;
}
+/*
+ * CheckRelExistenceInCTAS --- check existence of relation in CREATE TABLE AS
+ *
+ * This routine can be used as a fast-exit path when checking if a CTAS query
+ * needs to be executed or not. Returns true if the relation can be created.
+ */
+bool
+CheckRelExistenceInCTAS(CreateTableAsStmt *ctas)
+{
+ Oid nspid;
+ IntoClause *into = ctas->into;
+
+ nspid = RangeVarGetCreationNamespace(into->rel);
+
+ if (get_relname_relid(into->rel->relname, nspid))
+ {
+ if (!ctas->if_not_exists)
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_TABLE),
+ errmsg("relation \"%s\" already exists",
+ into->rel->relname)));
+
+ /*
+ * The relation exists and IF NOT EXISTS has been specified,
+ * so let the caller know about that and let the processing
+ * continue.
+ */
+ ereport(NOTICE,
+ (errcode(ERRCODE_DUPLICATE_TABLE),
+ errmsg("relation \"%s\" already exists, skipping",
+ into->rel->relname)));
+ return false;
+ }
+
+ /* Relation does not exist, so it can be created */
+ return true;
+}
+
/*
* CreateIntoRelDestReceiver -- create a suitable DestReceiver object
*
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 43f9b01e83..6589b99b20 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -435,6 +435,14 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es,
CreateTableAsStmt *ctas = (CreateTableAsStmt *) utilityStmt;
List *rewritten;
+ /*
+ * Check if the relation to-be-created exists or not. This is done
+ * at this early stage to avoid query planning or execution as it
+ * may not be necessary.
+ */
+ if (!CheckRelExistenceInCTAS(ctas))
+ return;
+
rewritten = QueryRewrite(castNode(Query, copyObject(ctas->query)));
Assert(list_length(rewritten) == 1);
ExplainOneQuery(linitial_node(Query, rewritten),
diff --git a/src/test/regress/expected/matview.out b/src/test/regress/expected/matview.out
index 2c0760404d..4b3a2e0cb7 100644
--- a/src/test/regress/expected/matview.out
+++ b/src/test/regress/expected/matview.out
@@ -630,3 +630,41 @@ drop cascades to materialized view matview_schema.mv_withdata2
drop cascades to materialized view matview_schema.mv_nodata1
drop cascades to materialized view matview_schema.mv_nodata2
DROP USER regress_matview_user;
+-- CREATE MATERIALIZED VIEW ... IF NOT EXISTS
+CREATE MATERIALIZED VIEW matview_ine_tab AS SELECT 1;
+CREATE MATERIALIZED VIEW matview_ine_tab AS SELECT 1 / 0; -- error
+ERROR: relation "matview_ine_tab" already exists
+CREATE MATERIALIZED VIEW IF NOT EXISTS matview_ine_tab AS
+ SELECT 1 / 0; -- ok
+NOTICE: relation "matview_ine_tab" already exists, skipping
+CREATE MATERIALIZED VIEW matview_ine_tab AS
+ SELECT 1 / 0 WITH NO DATA; -- error
+ERROR: relation "matview_ine_tab" already exists
+CREATE MATERIALIZED VIEW IF NOT EXISTS matview_ine_tab AS
+ SELECT 1 / 0 WITH NO DATA; -- ok
+NOTICE: relation "matview_ine_tab" already exists, skipping
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE MATERIALIZED VIEW matview_ine_tab AS
+ SELECT 1 / 0; -- error
+ERROR: relation "matview_ine_tab" already exists
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE MATERIALIZED VIEW IF NOT EXISTS matview_ine_tab AS
+ SELECT 1 / 0; -- ok
+NOTICE: relation "matview_ine_tab" already exists, skipping
+ QUERY PLAN
+------------
+(0 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE MATERIALIZED VIEW matview_ine_tab AS
+ SELECT 1 / 0 WITH NO DATA; -- error
+ERROR: relation "matview_ine_tab" already exists
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE MATERIALIZED VIEW IF NOT EXISTS matview_ine_tab AS
+ SELECT 1 / 0 WITH NO DATA; -- ok
+NOTICE: relation "matview_ine_tab" already exists, skipping
+ QUERY PLAN
+------------
+(0 rows)
+
+DROP MATERIALIZED VIEW matview_ine_tab;
diff --git a/src/test/regress/expected/select_into.out b/src/test/regress/expected/select_into.out
index bf5c6bea04..4e7ed8a339 100644
--- a/src/test/regress/expected/select_into.out
+++ b/src/test/regress/expected/select_into.out
@@ -178,3 +178,34 @@ INSERT INTO b SELECT 1 INTO f;
ERROR: SELECT ... INTO is not allowed here
LINE 1: INSERT INTO b SELECT 1 INTO f;
^
+-- Test CREATE TABLE AS ... IF NOT EXISTS
+CREATE TABLE ctas_ine_tbl AS SELECT 1;
+CREATE TABLE ctas_ine_tbl AS SELECT 1 / 0; -- error
+ERROR: relation "ctas_ine_tbl" already exists
+CREATE TABLE IF NOT EXISTS ctas_ine_tbl AS SELECT 1 / 0; -- ok
+NOTICE: relation "ctas_ine_tbl" already exists, skipping
+CREATE TABLE ctas_ine_tbl AS SELECT 1 / 0 WITH NO DATA; -- error
+ERROR: relation "ctas_ine_tbl" already exists
+CREATE TABLE IF NOT EXISTS ctas_ine_tbl AS SELECT 1 / 0 WITH NO DATA; -- ok
+NOTICE: relation "ctas_ine_tbl" already exists, skipping
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE TABLE ctas_ine_tbl AS SELECT 1 / 0; -- error
+ERROR: relation "ctas_ine_tbl" already exists
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE TABLE IF NOT EXISTS ctas_ine_tbl AS SELECT 1 / 0; -- ok
+NOTICE: relation "ctas_ine_tbl" already exists, skipping
+ QUERY PLAN
+------------
+(0 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE TABLE ctas_ine_tbl AS SELECT 1 / 0 WITH NO DATA; -- error
+ERROR: relation "ctas_ine_tbl" already exists
+EXPLAIN (COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE TABLE IF NOT EXISTS ctas_ine_tbl AS SELECT 1 / 0 WITH NO DATA; -- ok
+NOTICE: relation "ctas_ine_tbl" already exists, skipping
+ QUERY PLAN
+------------
+(0 rows)
+
+DROP TABLE ctas_ine_tbl;
diff --git a/src/test/regress/sql/matview.sql b/src/test/regress/sql/matview.sql
index 70c4954d89..4a4bd0d6b6 100644
--- a/src/test/regress/sql/matview.sql
+++ b/src/test/regress/sql/matview.sql
@@ -264,3 +264,26 @@ ALTER DEFAULT PRIVILEGES FOR ROLE regress_matview_user
DROP SCHEMA matview_schema CASCADE;
DROP USER regress_matview_user;
+
+-- CREATE MATERIALIZED VIEW ... IF NOT EXISTS
+CREATE MATERIALIZED VIEW matview_ine_tab AS SELECT 1;
+CREATE MATERIALIZED VIEW matview_ine_tab AS SELECT 1 / 0; -- error
+CREATE MATERIALIZED VIEW IF NOT EXISTS matview_ine_tab AS
+ SELECT 1 / 0; -- ok
+CREATE MATERIALIZED VIEW matview_ine_tab AS
+ SELECT 1 / 0 WITH NO DATA; -- error
+CREATE MATERIALIZED VIEW IF NOT EXISTS matview_ine_tab AS
+ SELECT 1 / 0 WITH NO DATA; -- ok
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE MATERIALIZED VIEW matview_ine_tab AS
+ SELECT 1 / 0; -- error
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE MATERIALIZED VIEW IF NOT EXISTS matview_ine_tab AS
+ SELECT 1 / 0; -- ok
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE MATERIALIZED VIEW matview_ine_tab AS
+ SELECT 1 / 0 WITH NO DATA; -- error
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE MATERIALIZED VIEW IF NOT EXISTS matview_ine_tab AS
+ SELECT 1 / 0 WITH NO DATA; -- ok
+DROP MATERIALIZED VIEW matview_ine_tab;
diff --git a/src/test/regress/sql/select_into.sql b/src/test/regress/sql/select_into.sql
index 6c170ef968..45a268260c 100644
--- a/src/test/regress/sql/select_into.sql
+++ b/src/test/regress/sql/select_into.sql
@@ -115,3 +115,19 @@ COPY (SELECT 1 INTO frak UNION SELECT 2) TO 'blob';
SELECT * FROM (SELECT 1 INTO f) bar;
CREATE VIEW foo AS SELECT 1 INTO b;
INSERT INTO b SELECT 1 INTO f;
+
+-- Test CREATE TABLE AS ... IF NOT EXISTS
+CREATE TABLE ctas_ine_tbl AS SELECT 1;
+CREATE TABLE ctas_ine_tbl AS SELECT 1 / 0; -- error
+CREATE TABLE IF NOT EXISTS ctas_ine_tbl AS SELECT 1 / 0; -- ok
+CREATE TABLE ctas_ine_tbl AS SELECT 1 / 0 WITH NO DATA; -- error
+CREATE TABLE IF NOT EXISTS ctas_ine_tbl AS SELECT 1 / 0 WITH NO DATA; -- ok
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE TABLE ctas_ine_tbl AS SELECT 1 / 0; -- error
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE TABLE IF NOT EXISTS ctas_ine_tbl AS SELECT 1 / 0; -- ok
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE TABLE ctas_ine_tbl AS SELECT 1 / 0 WITH NO DATA; -- error
+EXPLAIN (COSTS OFF, SUMMARY OFF, TIMING OFF)
+ CREATE TABLE IF NOT EXISTS ctas_ine_tbl AS SELECT 1 / 0 WITH NO DATA; -- ok
+DROP TABLE ctas_ine_tbl;
--
2.29.2
signature.asc
Description: PGP signature
