Jim Jones <[email protected]> writes:
> v8 attached.
Pushed with some more editing; for instance there were a bunch
of comments still oriented toward throwing an error. I also
still thought the tests were pretty duplicative.
One note is that I took out
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
and just let it default to ERRCODE_SUCCESSFUL_COMPLETION,
which is what happens for the longstanding message about
temp views too. FEATURE_NOT_SUPPORTED seemed less than
apropos, and besides this is an error SQLSTATE code not
a notice/warning code. I don't see any existing SQLSTATE
that seems on-point here; is it worth inventing one?
Before we leave the topic, here's a quick draft of a patch
to make temp-view detection use the dependency infrastructure
instead of isQueryUsingTempRelation(). That function is a
really old, crufty hack that fails to catch all dependencies.
So this can be sold on the basis of being a functional
improvement as well as adding detail to the notice messages.
It's slightly annoying that this patch means we're going to do
collectDependenciesOfExpr twice on the view body, but the place
where we'll do that again to update pg_depend is so far removed
from DefineView() that it seems unduly invasive to try to pass
down the dependency data. I think it's not that expensive anyway;
collectDependenciesOfExpr just walks the query tree, I don't believe
there's any catalog access inside it.
I'm also not super satisfied with the wording of the message for
the matview case:
if (query_uses_temp_object(query, &temp_object))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("materialized views must not use temporary
objects"),
errdetail("This view depends on temporary %s.",
getObjectDescription(&temp_object, false))));
The "It" antecedent doesn't work here because the errmsg doesn't
state the matview's name, which is also true of a bunch of nearby
errors. I feel like not a lot of work went into those messages;
as evidenced by the fact that neither this one nor the other ones
get tested at all. But I'm not sure I want to do the work to make
that situation better.
regards, tom lane
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index fba5b2e0d09..8e70a85a3f7 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -2479,6 +2479,31 @@ find_temp_object(const ObjectAddresses *addrs, bool local_temp_okay,
return false;
}
+/*
+ * query_uses_temp_object - convenience wrapper for find_temp_object
+ *
+ * If the Query includes any use of a temporary object, fill *temp_object
+ * with the address of one such object and return true.
+ */
+bool
+query_uses_temp_object(Query *query, ObjectAddress *temp_object)
+{
+ bool result;
+ ObjectAddresses *addrs;
+
+ addrs = new_object_addresses();
+
+ /* Collect all dependencies from the Query */
+ collectDependenciesOfExpr(addrs, (Node *) query, NIL);
+
+ /* Look for one that is temp */
+ result = find_temp_object(addrs, false, temp_object);
+
+ free_object_addresses(addrs);
+
+ return result;
+}
+
/*
* Given an array of dependency references, eliminate any duplicates.
*/
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index 6f0301555e0..4cc2af7b5ec 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -22,7 +22,6 @@
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "parser/analyze.h"
-#include "parser/parse_relation.h"
#include "rewrite/rewriteDefine.h"
#include "rewrite/rewriteHandler.h"
#include "rewrite/rewriteSupport.h"
@@ -362,6 +361,7 @@ DefineView(ViewStmt *stmt, const char *queryString,
ListCell *cell;
bool check_option;
ObjectAddress address;
+ ObjectAddress temp_object;
/*
* Run parse analysis to convert the raw parse tree to a Query. Note this
@@ -484,12 +484,14 @@ DefineView(ViewStmt *stmt, const char *queryString,
*/
view = copyObject(stmt->view); /* don't corrupt original command */
if (view->relpersistence == RELPERSISTENCE_PERMANENT
- && isQueryUsingTempRelation(viewParse))
+ && query_uses_temp_object(viewParse, &temp_object))
{
view->relpersistence = RELPERSISTENCE_TEMP;
ereport(NOTICE,
(errmsg("view \"%s\" will be a temporary view",
- view->relname)));
+ view->relname),
+ errdetail("It depends on temporary %s.",
+ getObjectDescription(&temp_object, false))));
}
/*
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 3b392b084ad..7843a0c857e 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -25,6 +25,7 @@
#include "postgres.h"
#include "access/sysattr.h"
+#include "catalog/dependency.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
#include "commands/defrem.h"
@@ -3129,6 +3130,8 @@ transformCreateTableAsStmt(ParseState *pstate, CreateTableAsStmt *stmt)
/* additional work needed for CREATE MATERIALIZED VIEW */
if (stmt->objtype == OBJECT_MATVIEW)
{
+ ObjectAddress temp_object;
+
/*
* Prohibit a data-modifying CTE in the query used to create a
* materialized view. It's not sufficiently clear what the user would
@@ -3144,10 +3147,12 @@ transformCreateTableAsStmt(ParseState *pstate, CreateTableAsStmt *stmt)
* creation query. It would be hard to refresh data or incrementally
* maintain it if a source disappeared.
*/
- if (isQueryUsingTempRelation(query))
+ if (query_uses_temp_object(query, &temp_object))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("materialized views must not use temporary tables or views")));
+ errmsg("materialized views must not use temporary objects"),
+ errdetail("This view depends on temporary %s.",
+ getObjectDescription(&temp_object, false))));
/*
* A materialized view would either need to save parameters for use in
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 3c80bf1b9ce..d544a69fc80 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -18,11 +18,9 @@
#include "access/htup_details.h"
#include "access/relation.h"
-#include "access/sysattr.h"
#include "access/table.h"
#include "catalog/heap.h"
#include "catalog/namespace.h"
-#include "catalog/pg_type.h"
#include "funcapi.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
@@ -33,7 +31,6 @@
#include "storage/lmgr.h"
#include "utils/builtins.h"
#include "utils/lsyscache.h"
-#include "utils/rel.h"
#include "utils/syscache.h"
#include "utils/varlena.h"
@@ -103,7 +100,6 @@ static void expandTupleDesc(TupleDesc tupdesc, Alias *eref,
static int specialAttNum(const char *attname);
static bool rte_visible_if_lateral(ParseState *pstate, RangeTblEntry *rte);
static bool rte_visible_if_qualified(ParseState *pstate, RangeTblEntry *rte);
-static bool isQueryUsingTempRelation_walker(Node *node, void *context);
/*
@@ -3922,53 +3918,6 @@ rte_visible_if_qualified(ParseState *pstate, RangeTblEntry *rte)
}
-/*
- * Examine a fully-parsed query, and return true iff any relation underlying
- * the query is a temporary relation (table, view, or materialized view).
- */
-bool
-isQueryUsingTempRelation(Query *query)
-{
- return isQueryUsingTempRelation_walker((Node *) query, NULL);
-}
-
-static bool
-isQueryUsingTempRelation_walker(Node *node, void *context)
-{
- if (node == NULL)
- return false;
-
- if (IsA(node, Query))
- {
- Query *query = (Query *) node;
- ListCell *rtable;
-
- foreach(rtable, query->rtable)
- {
- RangeTblEntry *rte = lfirst(rtable);
-
- if (rte->rtekind == RTE_RELATION)
- {
- Relation rel = table_open(rte->relid, AccessShareLock);
- char relpersistence = rel->rd_rel->relpersistence;
-
- table_close(rel, AccessShareLock);
- if (relpersistence == RELPERSISTENCE_TEMP)
- return true;
- }
- }
-
- return query_tree_walker(query,
- isQueryUsingTempRelation_walker,
- context,
- QTW_IGNORE_JOINALIASES);
- }
-
- return expression_tree_walker(node,
- isQueryUsingTempRelation_walker,
- context);
-}
-
/*
* addRTEPermissionInfo
* Creates RTEPermissionInfo for a given RTE and adds it into the
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index 81309b8ce32..06a8761e3fe 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -127,6 +127,8 @@ extern bool find_temp_object(const ObjectAddresses *addrs,
bool local_temp_okay,
ObjectAddress *foundobj);
+extern bool query_uses_temp_object(Query *query, ObjectAddress *temp_object);
+
extern ObjectAddresses *new_object_addresses(void);
extern void add_exact_object_address(const ObjectAddress *object,
diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h
index d59599cf242..aceb43f995a 100644
--- a/src/include/parser/parse_relation.h
+++ b/src/include/parser/parse_relation.h
@@ -127,6 +127,5 @@ extern int attnameAttNum(Relation rd, const char *attname, bool sysColOK);
extern const NameData *attnumAttName(Relation rd, int attid);
extern Oid attnumTypeId(Relation rd, int attid);
extern Oid attnumCollationId(Relation rd, int attid);
-extern bool isQueryUsingTempRelation(Query *query);
#endif /* PARSE_RELATION_H */
diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
index 49dd13c345c..6891f27b893 100644
--- a/src/test/regress/expected/create_view.out
+++ b/src/test/regress/expected/create_view.out
@@ -114,6 +114,7 @@ CREATE VIEW v1 AS SELECT * FROM base_table;
-- should be created in temp object schema
CREATE VIEW v1_temp AS SELECT * FROM temp_table;
NOTICE: view "v1_temp" will be a temporary view
+DETAIL: It depends on temporary table temp_table.
-- should be created in temp object schema
CREATE TEMP VIEW v2_temp AS SELECT * FROM base_table;
-- should be created in temp_views schema
@@ -121,6 +122,7 @@ CREATE VIEW temp_view_test.v2 AS SELECT * FROM base_table;
-- should fail
CREATE VIEW temp_view_test.v3_temp AS SELECT * FROM temp_table;
NOTICE: view "v3_temp" will be a temporary view
+DETAIL: It depends on temporary table temp_table.
ERROR: cannot create temporary relation in non-temporary schema
-- should fail
CREATE SCHEMA test_view_schema
@@ -139,12 +141,14 @@ CREATE VIEW v4_temp AS
FROM base_table t1, temp_table t2
WHERE t1.id = t2.id;
NOTICE: view "v4_temp" will be a temporary view
+DETAIL: It depends on temporary table temp_table.
-- should be temp
CREATE VIEW v5_temp AS
SELECT t1.a AS t1_a, t2.a AS t2_a, t3.a AS t3_a
FROM base_table t1, base_table2 t2, temp_table t3
WHERE t1.id = t2.id and t2.id = t3.id;
NOTICE: view "v5_temp" will be a temporary view
+DETAIL: It depends on temporary table temp_table.
-- subqueries
CREATE VIEW v4 AS SELECT * FROM base_table WHERE id IN (SELECT id FROM base_table2);
CREATE VIEW v5 AS SELECT t1.id, t2.a FROM base_table t1, (SELECT * FROM base_table2) t2;
@@ -153,25 +157,33 @@ CREATE VIEW v7 AS SELECT * FROM base_table WHERE NOT EXISTS (SELECT 1 FROM base_
CREATE VIEW v8 AS SELECT * FROM base_table WHERE EXISTS (SELECT 1);
CREATE VIEW v6_temp AS SELECT * FROM base_table WHERE id IN (SELECT id FROM temp_table);
NOTICE: view "v6_temp" will be a temporary view
+DETAIL: It depends on temporary table temp_table.
CREATE VIEW v7_temp AS SELECT t1.id, t2.a FROM base_table t1, (SELECT * FROM temp_table) t2;
NOTICE: view "v7_temp" will be a temporary view
+DETAIL: It depends on temporary table temp_table.
CREATE VIEW v8_temp AS SELECT * FROM base_table WHERE EXISTS (SELECT 1 FROM temp_table);
NOTICE: view "v8_temp" will be a temporary view
+DETAIL: It depends on temporary table temp_table.
CREATE VIEW v9_temp AS SELECT * FROM base_table WHERE NOT EXISTS (SELECT 1 FROM temp_table);
NOTICE: view "v9_temp" will be a temporary view
+DETAIL: It depends on temporary table temp_table.
-- a view should also be temporary if it references a temporary view
CREATE VIEW v10_temp AS SELECT * FROM v7_temp;
NOTICE: view "v10_temp" will be a temporary view
+DETAIL: It depends on temporary view v7_temp.
CREATE VIEW v11_temp AS SELECT t1.id, t2.a FROM base_table t1, v10_temp t2;
NOTICE: view "v11_temp" will be a temporary view
+DETAIL: It depends on temporary view v10_temp.
CREATE VIEW v12_temp AS SELECT true FROM v11_temp;
NOTICE: view "v12_temp" will be a temporary view
+DETAIL: It depends on temporary view v11_temp.
-- a view should also be temporary if it references a temporary sequence
CREATE SEQUENCE seq1;
CREATE TEMPORARY SEQUENCE seq1_temp;
CREATE VIEW v9 AS SELECT seq1.is_called FROM seq1;
CREATE VIEW v13_temp AS SELECT seq1_temp.is_called FROM seq1_temp;
NOTICE: view "v13_temp" will be a temporary view
+DETAIL: It depends on temporary sequence seq1_temp.
SELECT relname FROM pg_class
WHERE relname LIKE 'v_'
AND relnamespace = (SELECT oid FROM pg_namespace WHERE nspname = 'temp_view_test')
@@ -217,15 +229,19 @@ CREATE TEMP TABLE tt (num2 int, value text);
CREATE VIEW nontemp1 AS SELECT * FROM t1 CROSS JOIN t2;
CREATE VIEW temporal1 AS SELECT * FROM t1 CROSS JOIN tt;
NOTICE: view "temporal1" will be a temporary view
+DETAIL: It depends on temporary table tt.
CREATE VIEW nontemp2 AS SELECT * FROM t1 INNER JOIN t2 ON t1.num = t2.num2;
CREATE VIEW temporal2 AS SELECT * FROM t1 INNER JOIN tt ON t1.num = tt.num2;
NOTICE: view "temporal2" will be a temporary view
+DETAIL: It depends on temporary table tt.
CREATE VIEW nontemp3 AS SELECT * FROM t1 LEFT JOIN t2 ON t1.num = t2.num2;
CREATE VIEW temporal3 AS SELECT * FROM t1 LEFT JOIN tt ON t1.num = tt.num2;
NOTICE: view "temporal3" will be a temporary view
+DETAIL: It depends on temporary table tt.
CREATE VIEW nontemp4 AS SELECT * FROM t1 LEFT JOIN t2 ON t1.num = t2.num2 AND t2.value = 'xxx';
CREATE VIEW temporal4 AS SELECT * FROM t1 LEFT JOIN tt ON t1.num = tt.num2 AND tt.value = 'xxx';
NOTICE: view "temporal4" will be a temporary view
+DETAIL: It depends on temporary table tt.
SELECT relname FROM pg_class
WHERE relname LIKE 'nontemp%'
AND relnamespace = (SELECT oid FROM pg_namespace WHERE nspname = 'testviewschm2')
@@ -272,6 +288,7 @@ BETWEEN (SELECT d FROM tbl2 WHERE c = 1) AND (SELECT e FROM tbl3 WHERE f = 2)
AND EXISTS (SELECT g FROM tbl4 LEFT JOIN tbl3 ON tbl4.h = tbl3.f)
AND NOT EXISTS (SELECT g FROM tbl4 LEFT JOIN tmptbl ON tbl4.h = tmptbl.j);
NOTICE: view "mytempview" will be a temporary view
+DETAIL: It depends on temporary table tmptbl.
SELECT count(*) FROM pg_class where relname LIKE 'mytempview'
And relnamespace IN (SELECT OID FROM pg_namespace WHERE nspname LIKE 'pg_temp%');
count
diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out
index 398cf6965e0..39d35a195bc 100644
--- a/src/test/regress/expected/groupingsets.out
+++ b/src/test/regress/expected/groupingsets.out
@@ -602,6 +602,7 @@ explain (costs off)
CREATE VIEW gstest_view AS select a, b, grouping(a,b), sum(c), count(*), max(c)
from gstest2 group by rollup ((a,b,c),(c,d));
NOTICE: view "gstest_view" will be a temporary view
+DETAIL: It depends on temporary table gstest2.
select pg_get_viewdef('gstest_view'::regclass, true);
pg_get_viewdef
---------------------------------------
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index 9e2f53726f5..7a04d3a7a9f 100644
--- a/src/test/regress/expected/window.out
+++ b/src/test/regress/expected/window.out
@@ -5497,6 +5497,7 @@ FROM planets
WINDOW w AS (ORDER BY name)
;
NOTICE: view "planets_view" will be a temporary view
+DETAIL: It depends on temporary table planets.
SELECT pg_get_viewdef('planets_view');
pg_get_viewdef
--------------------------------------------------
