Improve detection of implicitly-temporary views.

We've long had a practice of making views temporary by default if they
reference any temporary tables.  However the implementation was pretty
incomplete, in that it only searched for RangeTblEntry references to
temp relations.  Uses of temporary types, regclass constants, etc
were not detected even though the dependency mechanism considers them
grounds for dropping the view.  Thus a view not believed to be temp
could silently go away at session exit anyhow.

To improve matters, replace the ad-hoc isQueryUsingTempRelation()
logic with use of the dependency-based infrastructure introduced by
commit 572c40ba9.  This is complete by definition, and it's less code
overall.

While we're at it, we can also extend the warning NOTICE (or ERROR
in the case of a materialized view) to mention one of the temp
objects motivating the classification of the view as temp, as was
done for functions in 572c40ba9.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
Discussion: https://postgr.es/m/19cf6ae1-04cd-422c-a760-d7e75fe6cba9@uni-muenster.de

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index fba5b2e0d099c8f6adc1d3b5f2d3fb0458993bce..8e70a85a3f73096bf35c5c0003747a0e14020894 100644 (file)
--- 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 6f0301555e0aec9637a4cd4299232e59ddb1d369..4cc2af7b5ecdc81437b206f0f0cc192f28e78bbb 100644 (file)
--- 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 3b392b084ad63bf02431e5e57807d584863b2ad8..7843a0c857e6f09b82c30f62fa8db24f6835840b 100644 (file)
--- 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 3c80bf1b9ce50fc62b1bd5799c6c366f7c4a5db0..d544a69fc80fa3c5faf86c9c94015737fc5b2e49 100644 (file)
--- 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 81309b8ce3263dca0577fd18fda64ccd83d37b4d..06a8761e3feac14f4538d09f1b77d778612ca425 100644 (file)
--- 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 d59599cf2424eadfe38ebcbe2b0052b7dab6a731..aceb43f995a11c9988992cc5733c0b33fa49fe54 100644 (file)
--- 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 49dd13c345cc5ec1cca6aa19b732f428aa8c4ac4..bc4f79938b360de47420ccd08cb3024a4b8c4b44 100644 (file)
--- 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;
+CREATE VIEW v9 AS SELECT nextval('seq1');
+CREATE VIEW v13_temp AS SELECT nextval('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 398cf6965e007ccfb81a2352807109150ee6493c..39d35a195bce888b9ac582be616ea38e79f1553b 100644 (file)
--- 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/matview.out b/src/test/regress/expected/matview.out
index c56c9fa3a254494f180f68536ecf3ac798a4f755..d2b7c88657d102534c2afeb2193529a1eb16c6f9 100644 (file)
--- a/src/test/regress/expected/matview.out
+++ b/src/test/regress/expected/matview.out
@@ -298,6 +298,11 @@ ERROR:  CONCURRENTLY and WITH NO DATA options cannot be used together
 -- no tuple locks on materialized views
 SELECT * FROM mvtest_tvvm FOR SHARE;
 ERROR:  cannot lock rows in materialized view "mvtest_tvvm"
+-- we don't support temp materialized views, so disallow this case:
+CREATE TEMP TABLE mvtest_temp_t (id int NOT NULL, type text NOT NULL, amt numeric NOT NULL);
+CREATE MATERIALIZED VIEW mvtest_temp_tm AS SELECT * FROM mvtest_temp_t;
+ERROR:  materialized views must not use temporary objects
+DETAIL:  This view depends on temporary table mvtest_temp_t.
 -- test join of mv and view
 SELECT type, m.totamt AS mtot, v.totamt AS vtot FROM mvtest_tm m LEFT JOIN mvtest_tv v USING (type) ORDER BY type;
  type | mtot | vtot 

diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index 9e2f53726f5e05b864f6dfdc8d163743d77ec2cc..7a04d3a7a9f615258956b66caa5da9d8e21a429c 100644 (file)
--- 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                  
 --------------------------------------------------

diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql
index ae6841308b9b6149999f5aa7605a4dc054f90972..75b8cd5d0fb3f94ca15329a48ef0da9c4c672434 100644 (file)
--- a/src/test/regress/sql/create_view.sql
+++ b/src/test/regress/sql/create_view.sql
@@ -179,8 +179,8 @@ CREATE VIEW v12_temp AS SELECT true FROM 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;
+CREATE VIEW v9 AS SELECT nextval('seq1');
+CREATE VIEW v13_temp AS SELECT nextval('seq1_temp');
 
 SELECT relname FROM pg_class
     WHERE relname LIKE 'v_'

diff --git a/src/test/regress/sql/matview.sql b/src/test/regress/sql/matview.sql
index 6704eeae2df84d0b6743b9a0383572b083c42830..934426b9ae8c1fe4c01f673446fb2f08ae1380ca 100644 (file)
--- a/src/test/regress/sql/matview.sql
+++ b/src/test/regress/sql/matview.sql
@@ -98,6 +98,10 @@ REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_tvmm WITH NO DATA;
 -- no tuple locks on materialized views
 SELECT * FROM mvtest_tvvm FOR SHARE;
 
+-- we don't support temp materialized views, so disallow this case:
+CREATE TEMP TABLE mvtest_temp_t (id int NOT NULL, type text NOT NULL, amt numeric NOT NULL);
+CREATE MATERIALIZED VIEW mvtest_temp_tm AS SELECT * FROM mvtest_temp_t;
+
 -- test join of mv and view
 SELECT type, m.totamt AS mtot, v.totamt AS vtot FROM mvtest_tm m LEFT JOIN mvtest_tv v USING (type) ORDER BY type;