[CF 6074] v5 - Disallow BEGIN ATOMIC SQL functions depending on temp relations

This branch was automatically generated by a robot using patches from an
email thread registered at:

https://commitfest.postgresql.org/patch/6074

The branch will be overwritten each time a new patch version is posted to
the thread, and also periodically to check for bitrot caused by changes
on the master branch.

Patch(es): https://www.postgresql.org/message-id/[email protected]
Author(s): Jim Jones

Author: Commitfest Bot

src/backend/catalog/dependency.c

@@ -44,6 +44,7 @@
 #include "catalog/pg_language.h"
 #include "catalog/pg_largeobject.h"
 #include "catalog/pg_namespace.h"
+#include "catalog/namespace.h"
 #include "catalog/pg_opclass.h"
 #include "catalog/pg_operator.h"
 #include "catalog/pg_opfamily.h"
@@ -1553,10 +1554,41 @@ void
 recordDependencyOnExpr(const ObjectAddress *depender,
 					   Node *expr, List *rtable,
 					   DependencyType behavior)
+{
+	ObjectAddresses *addrs;
+
+	addrs = new_object_addresses();
+
+	/* Collect all dependencies from the expression */
+	collectDependenciesFromExpr(addrs, expr, rtable);
+
+	/* And record 'em */
+	recordMultipleDependencies(depender,
+							   addrs->refs, addrs->numrefs,
+							   behavior);
+
+	free_object_addresses(addrs);
+}
+
+/*
+ * collectDependenciesFromExpr - 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 examine, filter, or modify the collected dependencies before recording them.
+ *
+ * This is particularly useful when dependency recording needs to be conditional
+ * or when dependencies from multiple sources need to be merged before recording.
+ */
+void
+collectDependenciesFromExpr(ObjectAddresses *addrs,
+							Node *expr, List *rtable)
 {
 	find_expr_references_context context;
 
-	context.addrs = new_object_addresses();
+	context.addrs = addrs;
 
 	/* Set up interpretation for Vars at varlevelsup = 0 */
 	context.rtables = list_make1(rtable);
@@ -1565,14 +1597,7 @@ recordDependencyOnExpr(const ObjectAddress *depender,
 	find_expr_references_walker(expr, &context);
 
 	/* Remove any duplicates */
-	eliminate_duplicate_dependencies(context.addrs);
-
-	/* And record 'em */
-	recordMultipleDependencies(depender,
-							   context.addrs->refs, context.addrs->numrefs,
-							   behavior);
-
-	free_object_addresses(context.addrs);
+	eliminate_duplicate_dependencies(addrs);
 }
 
 /*
@@ -1599,10 +1624,12 @@ recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
 								DependencyType self_behavior,
 								bool reverse_self)
 {
+	ObjectAddresses *addrs;
 	find_expr_references_context context;
 	RangeTblEntry rte = {0};
 
-	context.addrs = new_object_addresses();
+	addrs = new_object_addresses();
+	context.addrs = addrs;
 
 	/* We gin up a rather bogus rangetable list to handle Vars */
 	rte.type = T_RangeTblEntry;
@@ -1617,11 +1644,11 @@ recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
 	find_expr_references_walker(expr, &context);
 
 	/* Remove any duplicates */
-	eliminate_duplicate_dependencies(context.addrs);
+	eliminate_duplicate_dependencies(addrs);
 
 	/* Separate self-dependencies if necessary */
 	if ((behavior != self_behavior || reverse_self) &&
-		context.addrs->numrefs > 0)
+		addrs->numrefs > 0)
 	{
 		ObjectAddresses *self_addrs;
 		ObjectAddress *outobj;
@@ -1630,11 +1657,11 @@ recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
 
 		self_addrs = new_object_addresses();
 
-		outobj = context.addrs->refs;
+		outobj = addrs->refs;
 		outrefs = 0;
-		for (oldref = 0; oldref < context.addrs->numrefs; oldref++)
+		for (oldref = 0; oldref < addrs->numrefs; oldref++)
 		{
-			ObjectAddress *thisobj = context.addrs->refs + oldref;
+			ObjectAddress *thisobj = addrs->refs + oldref;
 
 			if (thisobj->classId == RelationRelationId &&
 				thisobj->objectId == relId)
@@ -1644,13 +1671,13 @@ recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
 			}
 			else
 			{
-				/* Keep it in context.addrs */
+				/* Keep it in addrs */
 				*outobj = *thisobj;
 				outobj++;
 				outrefs++;
 			}
 		}
-		context.addrs->numrefs = outrefs;
+		addrs->numrefs = outrefs;
 
 		/* Record the self-dependencies with the appropriate direction */
 		if (!reverse_self)
@@ -1675,10 +1702,10 @@ recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
 
 	/* Record the external dependencies */
 	recordMultipleDependencies(depender,
-							   context.addrs->refs, context.addrs->numrefs,
+							   addrs->refs, addrs->numrefs,
 							   behavior);
 
-	free_object_addresses(context.addrs);
+	free_object_addresses(addrs);
 }
 
 /*
@@ -2462,6 +2489,87 @@ eliminate_duplicate_dependencies(ObjectAddresses *addrs)
 	addrs->numrefs = newrefs;
 }
 
+/*
+ * filter_temp_objects - detect and reject temporary objects in an ObjectAddresses array
+ *
+ * This function checks if any dependencies on temporary objects (objects in
+ * temporary namespaces) exist in the given ObjectAddresses array. If temp objects
+ * are found, it raises an error to prevent them from being used in SQL functions
+ * with BEGIN ATOMIC bodies, as such dependencies would be inappropriate for
+ * permanent function definitions.
+ *
+ * Currently checks for temporary tables, views, types, and functions by examining
+ * their containing namespaces. The function raises an error with a descriptive
+ * message if any temporary object dependency is detected.
+ */
+void filter_temp_objects(ObjectAddresses *addrs)
+{
+	int oldref;
+
+	if (addrs->numrefs <= 0)
+		return; /* nothing to do */
+
+	/* Check all dependencies for temp objects */
+	for (oldref = 0; oldref < addrs->numrefs; oldref++)
+	{
+		ObjectAddress *thisobj = addrs->refs + oldref;
+		bool is_temp = false;
+		char *objname = NULL;
+
+		/* Check if this dependency is on a temporary object */
+		if (thisobj->classId == RelationRelationId)
+		{
+			/* For relations, check if they're in a temp namespace */
+			Oid relnamespace = get_rel_namespace(thisobj->objectId);
+			if (OidIsValid(relnamespace) && isAnyTempNamespace(relnamespace))
+			{
+				is_temp = true;
+				objname = get_rel_name(thisobj->objectId);
+			}
+		}
+		else if (thisobj->classId == TypeRelationId)
+		{
+			/* For types, check if they're in a temp namespace */
+			HeapTuple tup;
+			Form_pg_type typform;
+			Oid typnamespace = InvalidOid;
+
+			tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(thisobj->objectId));
+			if (HeapTupleIsValid(tup))
+			{
+				typform = (Form_pg_type)GETSTRUCT(tup);
+				typnamespace = typform->typnamespace;
+				if (OidIsValid(typnamespace) && isAnyTempNamespace(typnamespace))
+				{
+					is_temp = true;
+					objname = NameStr(typform->typname);
+				}
+				ReleaseSysCache(tup);
+			}
+		}
+		else if (thisobj->classId == ProcedureRelationId)
+		{
+			/* For functions, check if they're in a temp namespace */
+			Oid funcnamespace = get_func_namespace(thisobj->objectId);
+			if (OidIsValid(funcnamespace) && isAnyTempNamespace(funcnamespace))
+			{
+				is_temp = true;
+				objname = get_func_name(thisobj->objectId);
+			}
+		}
+
+		/* Raise error if temp object found */
+		if (is_temp)
+		{
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot use temporary object \"%s\" in SQL function with BEGIN ATOMIC",
+							objname ? objname : "unknown"),
+					 errdetail("SQL functions with BEGIN ATOMIC cannot depend on temporary objects.")));
+		}
+	}
+}
+
 /*
  * qsort comparator for ObjectAddress items
  */

src/backend/catalog/pg_proc.c

@@ -24,6 +24,7 @@
 #include "catalog/pg_language.h"
 #include "catalog/pg_namespace.h"
 #include "catalog/pg_proc.h"
+#include "catalog/namespace.h"
 #include "catalog/pg_transform.h"
 #include "catalog/pg_type.h"
 #include "executor/functions.h"
@@ -663,7 +664,33 @@ ProcedureCreate(const char *procedureName,
 
 	/* dependency on SQL routine body */
 	if (languageObjectId == SQLlanguageId && prosqlbody)
-		recordDependencyOnExpr(&myself, prosqlbody, NIL, DEPENDENCY_NORMAL);
+	{
+		ObjectAddresses *body_addrs;
+
+		/*
+		 * For SQL functions with BEGIN ATOMIC, we use a collect-then-filter-then-record
+		 * approach to handle temp object dependencies appropriately.
+		 */
+		body_addrs = new_object_addresses();
+		collectDependenciesFromExpr(body_addrs, prosqlbody, NIL);
+
+		/*
+		 * Check for temp objects that are referenced in the function body.
+		 * For SQL functions with BEGIN ATOMIC bodies, we need to prevent
+		 * dependencies on temporary objects since such functions should be
+		 * permanently definable and not depend on session-specific temp objects.
+		 * This will raise an error if any temp objects are found. If the function
+		 * itself is being created in a temporary schema, then it's OK for it to
+		 * reference temp objects.
+		 */
+		if (!IsBootstrapProcessingMode() && !IsBinaryUpgrade &&
+			!isAnyTempNamespace(procNamespace))
+			filter_temp_objects(body_addrs);
+
+		/* Record the filtered dependencies */
+		record_object_address_dependencies(&myself, body_addrs, DEPENDENCY_NORMAL);
+		free_object_addresses(body_addrs);
+	}
 
 	/* dependency on parameter default expressions */
 	if (parameterDefaults)

src/include/catalog/dependency.h

@@ -114,12 +114,17 @@ extern void recordDependencyOnExpr(const ObjectAddress *depender,
 								   Node *expr, List *rtable,
 								   DependencyType behavior);
 
+extern void collectDependenciesFromExpr(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 void filter_temp_objects(ObjectAddresses *addrs);
+
 extern ObjectAddresses *new_object_addresses(void);
 
 extern void add_exact_object_address(const ObjectAddress *object,

src/test/regress/expected/create_function_sql.out

@@ -297,6 +297,86 @@ CREATE FUNCTION functest_S_xx(x anyarray) RETURNS anyelement
     LANGUAGE SQL
     RETURN x[1];
 ERROR:  SQL function with unquoted function body cannot have polymorphic arguments
+CREATE TEMPORARY TABLE temp_table AS SELECT 1 AS val;
+CREATE TEMPORARY VIEW temp_view AS SELECT 42 AS val;
+CREATE TYPE pg_temp.temp_type AS (x int, y text);
+CREATE TEMPORARY SEQUENCE temp_seq;
+CREATE DOMAIN pg_temp.temp_domain AS int CHECK (VALUE > 0);
+CREATE FUNCTION pg_temp.temp_func() RETURNS int LANGUAGE sql
+BEGIN ATOMIC;
+  SELECT 42;
+END;
+-- these should fail: BEGIN ATOMIC SQL-functions cannot depend on temporary tables
+CREATE FUNCTION functest_temp_dep() RETURNS int LANGUAGE sql
+BEGIN ATOMIC;
+  SELECT val FROM temp_table;
+END;
+ERROR:  cannot use temporary object "temp_table" in SQL function with BEGIN ATOMIC
+DETAIL:  SQL functions with BEGIN ATOMIC cannot depend on temporary objects.
+CREATE FUNCTION functest_temp_dep_subquery() RETURNS int LANGUAGE sql
+BEGIN ATOMIC;
+  SELECT (SELECT COUNT(*) FROM temp_table);
+END;
+ERROR:  cannot use temporary object "temp_table" in SQL function with BEGIN ATOMIC
+DETAIL:  SQL functions with BEGIN ATOMIC cannot depend on temporary objects.
+CREATE FUNCTION functest_temp_dep_join() RETURNS int LANGUAGE sql
+BEGIN ATOMIC;
+  SELECT t1.val FROM temp_table t1
+  JOIN temp_view t2 ON t1.val = t2.val;
+END;
+ERROR:  cannot use temporary object "temp_view" in SQL function with BEGIN ATOMIC
+DETAIL:  SQL functions with BEGIN ATOMIC cannot depend on temporary objects.
+CREATE FUNCTION functest_temp_indirect_dep() RETURNS int LANGUAGE sql
+BEGIN ATOMIC;
+  SELECT * FROM pg_class WHERE oid = 'temp_table'::regclass;
+END;
+ERROR:  cannot use temporary object "temp_table" in SQL function with BEGIN ATOMIC
+DETAIL:  SQL functions with BEGIN ATOMIC cannot depend on temporary objects.
+-- this should work: the function is created in a temp schema
+CREATE FUNCTION pg_temp.functest_temp_dep() RETURNS int LANGUAGE sql
+BEGIN ATOMIC;
+  SELECT val FROM temp_table;
+END;
+-- this should fail: BEGIN ATOMIC SQL-functions cannot depend on temporary functions
+CREATE FUNCTION functest_temp_func_dep() RETURNS int LANGUAGE sql
+BEGIN ATOMIC;
+  SELECT pg_temp.temp_func();
+END;
+ERROR:  cannot use temporary object "temp_func" in SQL function with BEGIN ATOMIC
+DETAIL:  SQL functions with BEGIN ATOMIC cannot depend on temporary objects.
+-- this should work: temp function calling temp function (both in temp schema)
+CREATE FUNCTION pg_temp.functest_temp_to_temp() RETURNS int LANGUAGE sql
+BEGIN ATOMIC;
+  SELECT pg_temp.temp_func();
+END;
+-- this should fail: BEGIN ATOMIC SQL-functions cannot depend on temporary views
+CREATE FUNCTION functest_temp_view() RETURNS int LANGUAGE sql
+BEGIN ATOMIC;
+  SELECT val FROM temp_view;
+END;
+ERROR:  cannot use temporary object "temp_view" in SQL function with BEGIN ATOMIC
+DETAIL:  SQL functions with BEGIN ATOMIC cannot depend on temporary objects.
+-- this should fail: BEGIN ATOMIC SQL-functions cannot depend on temporary types
+CREATE FUNCTION functest_temp_type() RETURNS int LANGUAGE sql
+BEGIN ATOMIC;
+  SELECT (ROW(1,'test')::pg_temp.temp_type).x;
+END;
+ERROR:  cannot use temporary object "temp_type" in SQL function with BEGIN ATOMIC
+DETAIL:  SQL functions with BEGIN ATOMIC cannot depend on temporary objects.
+-- this should fail: BEGIN ATOMIC SQL-functions cannot depend on temporary sequences
+CREATE FUNCTION functest_temp_sequence() RETURNS int LANGUAGE sql
+BEGIN ATOMIC;
+  SELECT nextval('temp_seq');
+END;
+ERROR:  cannot use temporary object "temp_seq" in SQL function with BEGIN ATOMIC
+DETAIL:  SQL functions with BEGIN ATOMIC cannot depend on temporary objects.
+-- this should fail: BEGIN ATOMIC SQL-functions cannot depend on temporary domains
+CREATE FUNCTION functest_temp_domain() RETURNS int LANGUAGE sql
+BEGIN ATOMIC;
+  SELECT 5::pg_temp.temp_domain;
+END;
+ERROR:  cannot use temporary object "temp_domain" in SQL function with BEGIN ATOMIC
+DETAIL:  SQL functions with BEGIN ATOMIC cannot depend on temporary objects.
 -- check reporting of parse-analysis errors
 CREATE FUNCTION functest_S_xx(x date) RETURNS boolean
     LANGUAGE SQL