Issue a NOTICE if a created function depends on any temp objects.

We don't have an official concept of temporary functions.  (You can
make one explicitly in pg_temp, but then you have to explicitly
schema-qualify it on every call.)  However, until now we were quite
laissez-faire about whether a non-temporary function could depend on
a temporary object, such as a temp table or view.  If one does,
it will silently go away at end of session, due to the automatic
DROP ... CASCADE on the session's temporary objects.  People have
complained that that's surprising; however, we can't really forbid
it because other people (including our own regression tests) rely
on being able to do it.  Let's compromise by emitting a NOTICE
at CREATE FUNCTION time.  This is somewhat comparable to our
ancient practice of emitting a NOTICE when forcing a view to
become temp because it depends on temp tables.

Along the way, refactor recordDependencyOnExpr() so that the
dependencies of an expression can be combined with other
dependencies, instead of being emitted separately and perhaps
duplicatively.

We should probably make the implementation of temp-by-default
views use the same infrastructure used here, but that's for
another patch.  It's unclear whether there are any other object
classes that deserve similar treatment.

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

11 files changed, 176 insertions, 19 deletions

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 7dded634eb8..fba5b2e0d09 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -22,6 +22,7 @@
 #include "catalog/dependency.h"
 #include "catalog/heap.h"
 #include "catalog/index.h"
+#include "catalog/namespace.h"
 #include "catalog/objectaccess.h"
 #include "catalog/pg_am.h"
 #include "catalog/pg_amop.h"
@@ -1554,25 +1555,57 @@ recordDependencyOnExpr(const ObjectAddress *depender,
 					   Node *expr, List *rtable,
 					   DependencyType behavior)
 {
-	find_expr_references_context context;
+	ObjectAddresses *addrs;
 
-	context.addrs = new_object_addresses();
+	addrs = new_object_addresses();
 
-	/* Set up interpretation for Vars at varlevelsup = 0 */
-	context.rtables = list_make1(rtable);
+	/* Collect all dependencies from the expression */
+	collectDependenciesOfExpr(addrs, expr, rtable);
 
-	/* Scan the expression tree for referenceable objects */
-	find_expr_references_walker(expr, &context);
-
-	/* Remove any duplicates */
-	eliminate_duplicate_dependencies(context.addrs);
+	/* Remove duplicates */
+	eliminate_duplicate_dependencies(addrs);
 
 	/* And record 'em */
 	recordMultipleDependencies(depender,
-							   context.addrs->refs, context.addrs->numrefs,
+							   addrs->refs, addrs->numrefs,
 							   behavior);
 
-	free_object_addresses(context.addrs);
+	free_object_addresses(addrs);
+}
+
+/*
+ * collectDependenciesOfExpr - collect expression dependencies
+ *
+ * This function analyzes an expression or query in node-tree form to
+ * find all the objects it refers to (tables, columns, operators,
+ * functions, etc.) and adds them to the provided ObjectAddresses
+ * structure. Unlike recordDependencyOnExpr, this function does not
+ * immediately record the dependencies, allowing the caller to add to,
+ * filter, or modify the collected dependencies before recording them.
+ *
+ * rtable is the rangetable to be used to interpret Vars with varlevelsup=0.
+ * It can be NIL if no such variables are expected.
+ *
+ * Note: the returned list may well contain duplicates.  The caller should
+ * de-duplicate before recording the dependencies.  Within this file, callers
+ * must call eliminate_duplicate_dependencies().  External callers typically
+ * go through record_object_address_dependencies() which will see to that.
+ * This choice allows collecting dependencies from multiple sources without
+ * redundant de-duplication work.
+ */
+void
+collectDependenciesOfExpr(ObjectAddresses *addrs,
+						  Node *expr, List *rtable)
+{
+	find_expr_references_context context;
+
+	context.addrs = addrs;
+
+	/* Set up interpretation for Vars at varlevelsup = 0 */
+	context.rtables = list_make1(rtable);
+
+	/* Scan the expression tree for referenceable objects */
+	find_expr_references_walker(expr, &context);
 }
 
 /*
@@ -2403,6 +2436,50 @@ process_function_rte_ref(RangeTblEntry *rte, AttrNumber attnum,
 }
 
 /*
+ * find_temp_object - search an array of dependency references for temp objects
+ *
+ * Scan an ObjectAddresses array for references to temporary objects (objects
+ * in temporary namespaces), ignoring those in our own temp namespace if
+ * local_temp_okay is true.  If one is found, return true after storing its
+ * address in *foundobj.
+ *
+ * Current callers only use this to deliver helpful notices, so reporting
+ * one such object seems sufficient.  We return the first one, which should
+ * be a stable result for a given query since it depends only on the order
+ * in which this module searches query trees.  (However, it's important to
+ * call this before de-duplicating the objects, else OID order would affect
+ * the result.)
+ */
+bool
+find_temp_object(const ObjectAddresses *addrs, bool local_temp_okay,
+				 ObjectAddress *foundobj)
+{
+	for (int i = 0; i < addrs->numrefs; i++)
+	{
+		const ObjectAddress *thisobj = addrs->refs + i;
+		Oid			objnamespace;
+
+		/*
+		 * Use get_object_namespace() to see if this object belongs to a
+		 * schema.  If not, we can skip it.
+		 */
+		objnamespace = get_object_namespace(thisobj);
+
+		/*
+		 * If the object is in a temporary namespace, complain, except if
+		 * local_temp_okay and it's our own temp namespace.
+		 */
+		if (OidIsValid(objnamespace) && isAnyTempNamespace(objnamespace) &&
+			!(local_temp_okay && isTempNamespace(objnamespace)))
+		{
+			*foundobj = *thisobj;
+			return true;
+		}
+	}
+	return false;
+}
+
+/*
  * Given an array of dependency references, eliminate any duplicates.
  */
 static void
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index b89b9ccda0e..d608f37d361 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -20,6 +20,7 @@
 #include "catalog/catalog.h"
 #include "catalog/dependency.h"
 #include "catalog/indexing.h"
+#include "catalog/namespace.h"
 #include "catalog/objectaccess.h"
 #include "catalog/pg_language.h"
 #include "catalog/pg_namespace.h"
@@ -141,7 +142,8 @@ ProcedureCreate(const char *procedureName,
 	TupleDesc	tupDesc;
 	bool		is_update;
 	ObjectAddress myself,
-				referenced;
+				referenced,
+				temp_object;
 	char	   *detailmsg;
 	int			i;
 	ObjectAddresses *addrs;
@@ -658,17 +660,40 @@ ProcedureCreate(const char *procedureName,
 		add_exact_object_address(&referenced, addrs);
 	}
 
-	record_object_address_dependencies(&myself, addrs, DEPENDENCY_NORMAL);
-	free_object_addresses(addrs);
-
-	/* dependency on SQL routine body */
+	/* dependencies appearing in new-style SQL routine body */
 	if (languageObjectId == SQLlanguageId && prosqlbody)
-		recordDependencyOnExpr(&myself, prosqlbody, NIL, DEPENDENCY_NORMAL);
+		collectDependenciesOfExpr(addrs, prosqlbody, NIL);
 
 	/* dependency on parameter default expressions */
 	if (parameterDefaults)
-		recordDependencyOnExpr(&myself, (Node *) parameterDefaults,
-							   NIL, DEPENDENCY_NORMAL);
+		collectDependenciesOfExpr(addrs, (Node *) parameterDefaults, NIL);
+
+	/*
+	 * Now that we have all the normal dependencies, thumb through them and
+	 * warn if any are to temporary objects.  This informs the user if their
+	 * supposedly non-temp function will silently go away at session exit, due
+	 * to a dependency on a temp object.  However, do not complain when a
+	 * function created in our own pg_temp namespace refers to other objects
+	 * in that namespace, since then they'll have similar lifespans anyway.
+	 */
+	if (find_temp_object(addrs, isTempNamespace(procNamespace), &temp_object))
+		ereport(NOTICE,
+				(errmsg("function \"%s\" will be effectively temporary",
+						procedureName),
+				 errdetail("It depends on temporary %s.",
+						   getObjectDescription(&temp_object, false))));
+
+	/*
+	 * Now record all normal dependencies at once.  This will also remove any
+	 * duplicates in the list.  (Role and extension dependencies are handled
+	 * separately below.  Role dependencies would have to be separate anyway
+	 * since they are shared dependencies.  An extension dependency could be
+	 * folded into the addrs list, but pg_depend.c doesn't make that easy, and
+	 * it won't duplicate anything we've collected so far anyway.)
+	 */
+	record_object_address_dependencies(&myself, addrs, DEPENDENCY_NORMAL);
+
+	free_object_addresses(addrs);
 
 	/* dependency on owner */
 	if (!is_update)
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 0335e982b31..59d00638ee6 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -153,6 +153,8 @@ compute_return_type(TypeName *returnType, Oid languageOid,
 		address = TypeShellMake(typname, namespaceId, GetUserId());
 		rettype = address.objectId;
 		Assert(OidIsValid(rettype));
+		/* Ensure the new shell type is visible to ProcedureCreate */
+		CommandCounterIncrement();
 	}
 
 	aclresult = object_aclcheck(TypeRelationId, rettype, GetUserId(), ACL_USAGE);
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 5979580139f..47d5047fe8b 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1742,6 +1742,9 @@ DefineRange(ParseState *pstate, CreateRangeStmt *stmt)
 			   false,			/* Type NOT NULL */
 			   InvalidOid);		/* typcollation */
 
+	/* Ensure these new types are visible to ProcedureCreate */
+	CommandCounterIncrement();
+
 	/* And create the constructor functions for this range type */
 	makeRangeConstructors(typeName, typeNamespace, typoid, rangeSubtype);
 	makeMultirangeConstructors(multirangeTypeName, typeNamespace,
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index 0ea7ccf5243..81309b8ce32 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -114,12 +114,19 @@ extern void recordDependencyOnExpr(const ObjectAddress *depender,
 								   Node *expr, List *rtable,
 								   DependencyType behavior);
 
+extern void collectDependenciesOfExpr(ObjectAddresses *addrs,
+									  Node *expr, List *rtable);
+
 extern void recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
 											Node *expr, Oid relId,
 											DependencyType behavior,
 											DependencyType self_behavior,
 											bool reverse_self);
 
+extern bool find_temp_object(const ObjectAddresses *addrs,
+							 bool local_temp_okay,
+							 ObjectAddress *foundobj);
+
 extern ObjectAddresses *new_object_addresses(void);
 
 extern void add_exact_object_address(const ObjectAddress *object,
diff --git a/src/test/isolation/expected/temp-schema-cleanup.out b/src/test/isolation/expected/temp-schema-cleanup.out
index d10aee53a80..a16d30ffefa 100644
--- a/src/test/isolation/expected/temp-schema-cleanup.out
+++ b/src/test/isolation/expected/temp-schema-cleanup.out
@@ -25,6 +25,8 @@ exec
     
 (1 row)
 
+s1: NOTICE:  function "uses_a_temp_type" will be effectively temporary
+DETAIL:  It depends on temporary type just_give_me_a_type.
 step s1_discard_temp: 
     DISCARD TEMP;
 
@@ -82,6 +84,8 @@ exec
     
 (1 row)
 
+s1: NOTICE:  function "uses_a_temp_type" will be effectively temporary
+DETAIL:  It depends on temporary type just_give_me_a_type.
 step s1_exit: 
     SELECT pg_terminate_backend(pg_backend_pid());
 
diff --git a/src/test/regress/expected/create_function_sql.out b/src/test/regress/expected/create_function_sql.out
index 73c6730d459..42524230d2b 100644
--- a/src/test/regress/expected/create_function_sql.out
+++ b/src/test/regress/expected/create_function_sql.out
@@ -455,6 +455,16 @@ DROP TABLE functest3 CASCADE;
 NOTICE:  drop cascades to 2 other objects
 DETAIL:  drop cascades to view functestv3
 drop cascades to function functest_s_14()
+-- Check reporting of temporary-object dependencies within SQL-standard body
+-- (tests elsewhere already cover dependencies on arg and result types)
+CREATE TEMP SEQUENCE mytempseq;
+CREATE FUNCTION functest_tempseq() RETURNS int
+    RETURN nextval('mytempseq');
+NOTICE:  function "functest_tempseq" will be effectively temporary
+DETAIL:  It depends on temporary sequence mytempseq.
+-- This discards mytempseq and therefore functest_tempseq().  If it fails to,
+-- the function will appear in the information_schema tests below.
+DISCARD TEMP;
 -- information_schema tests
 CREATE FUNCTION functest_IS_1(a int, b int default 1, c text default 'foo')
     RETURNS int
diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index 30241e22da2..5cc94011e97 100644
--- a/src/test/regress/expected/rangefuncs.out
+++ b/src/test/regress/expected/rangefuncs.out
@@ -2178,6 +2178,8 @@ alter table users drop column todrop;
 create or replace function get_first_user() returns users as
 $$ SELECT * FROM users ORDER BY userid LIMIT 1; $$
 language sql stable;
+NOTICE:  function "get_first_user" will be effectively temporary
+DETAIL:  It depends on temporary type users.
 SELECT get_first_user();
   get_first_user   
 -------------------
@@ -2193,6 +2195,8 @@ SELECT * FROM get_first_user();
 create or replace function get_users() returns setof users as
 $$ SELECT * FROM users ORDER BY userid; $$
 language sql stable;
+NOTICE:  function "get_users" will be effectively temporary
+DETAIL:  It depends on temporary type users.
 SELECT get_users();
       get_users      
 ---------------------
diff --git a/src/test/regress/expected/returning.out b/src/test/regress/expected/returning.out
index d02c2ceab53..cfaaf015bb3 100644
--- a/src/test/regress/expected/returning.out
+++ b/src/test/regress/expected/returning.out
@@ -306,6 +306,8 @@ SELECT * FROM foo;
 -- Check use of a whole-row variable for an inlined set-returning function
 CREATE FUNCTION foo_f() RETURNS SETOF foo AS
   $$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE;
+NOTICE:  function "foo_f" will be effectively temporary
+DETAIL:  It depends on temporary type foo.
 UPDATE foo SET f2 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = foo.f1
   RETURNING foo_f;
       foo_f      
@@ -930,6 +932,8 @@ BEGIN ATOMIC
               (SELECT count(*) FROM foo WHERE foo = o),
               (SELECT count(*) FROM foo WHERE foo = n);
 END;
+NOTICE:  function "foo_update" will be effectively temporary
+DETAIL:  It depends on temporary table foo.
 \sf foo_update
 CREATE OR REPLACE FUNCTION public.foo_update()
  RETURNS void
diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out
index 677ad2ab9ad..956bc2d02fc 100644
--- a/src/test/regress/expected/rowtypes.out
+++ b/src/test/regress/expected/rowtypes.out
@@ -907,6 +907,8 @@ create temp table compos (f1 int, f2 text);
 create function fcompos1(v compos) returns void as $$
 insert into compos values (v);  -- fail
 $$ language sql;
+NOTICE:  function "fcompos1" will be effectively temporary
+DETAIL:  It depends on temporary type compos.
 ERROR:  column "f1" is of type integer but expression is of type compos
 LINE 2: insert into compos values (v);  -- fail
                                    ^
@@ -914,12 +916,18 @@ HINT:  You will need to rewrite or cast the expression.
 create function fcompos1(v compos) returns void as $$
 insert into compos values (v.*);
 $$ language sql;
+NOTICE:  function "fcompos1" will be effectively temporary
+DETAIL:  It depends on temporary type compos.
 create function fcompos2(v compos) returns void as $$
 select fcompos1(v);
 $$ language sql;
+NOTICE:  function "fcompos2" will be effectively temporary
+DETAIL:  It depends on temporary type compos.
 create function fcompos3(v compos) returns void as $$
 select fcompos1(fcompos3.v.*);
 $$ language sql;
+NOTICE:  function "fcompos3" will be effectively temporary
+DETAIL:  It depends on temporary type compos.
 select fcompos1(row(1,'one'));
  fcompos1 
 ----------
@@ -1012,6 +1020,8 @@ select last(f) from fullname f;
 
 create function longname(fullname) returns text language sql
 as $$select $1.first || ' ' || $1.last$$;
+NOTICE:  function "longname" will be effectively temporary
+DETAIL:  It depends on temporary type fullname.
 select f.longname from fullname f;
  longname 
 ----------
diff --git a/src/test/regress/sql/create_function_sql.sql b/src/test/regress/sql/create_function_sql.sql
index 3d5f2a92093..4543273f93a 100644
--- a/src/test/regress/sql/create_function_sql.sql
+++ b/src/test/regress/sql/create_function_sql.sql
@@ -241,6 +241,17 @@ SELECT functest_S_14();
 
 DROP TABLE functest3 CASCADE;
 
+-- Check reporting of temporary-object dependencies within SQL-standard body
+-- (tests elsewhere already cover dependencies on arg and result types)
+CREATE TEMP SEQUENCE mytempseq;
+
+CREATE FUNCTION functest_tempseq() RETURNS int
+    RETURN nextval('mytempseq');
+
+-- This discards mytempseq and therefore functest_tempseq().  If it fails to,
+-- the function will appear in the information_schema tests below.
+DISCARD TEMP;
+
 
 -- information_schema tests