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

12 files changed, 71 insertions, 61 deletions

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
@@ -2480,6 +2480,31 @@ find_temp_object(const ObjectAddresses *addrs, bool local_temp_okay,
 }
 
 /*
+ * 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.
  */
 static void
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);
 
 
 /*
@@ -3923,53 +3919,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
  *		provided list.
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..bc4f79938b3 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;
+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 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/matview.out b/src/test/regress/expected/matview.out
index c56c9fa3a25..d2b7c88657d 100644
--- 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 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                  
 --------------------------------------------------
diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql
index ae6841308b9..75b8cd5d0fb 100644
--- 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 6704eeae2df..934426b9ae8 100644
--- 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;