From 7a86471c0b9c5d2e1493c37fbb1faa156e6a2dc8 Mon Sep 17 00:00:00 2001 From: Jim Jones Date: Sat, 22 Nov 2025 22:43:14 +0100 Subject: [PATCH v8 2/2] Disallow dependencies from non-temporary functions to 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 did, it would silently go away at end of session, due to the implied DROP ... CASCADE on the session's temporary objects. People have complained that that's surprising, so let's fix it by disallowing the case. Hand-made temp functions are still allowed to reference temp objects, so long as they're in our own session's temp schema. This required modifying several existing regression tests that were depending on exactly the case that's now forbidden. Maybe that's an indication that this is a bad idea? 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 Reviewed-by: Tom Lane --- src/backend/catalog/dependency.c | 42 +++++++++++ src/backend/catalog/pg_proc.c | 43 +++++++++-- src/backend/commands/functioncmds.c | 2 + src/backend/commands/typecmds.c | 3 + src/include/catalog/dependency.h | 4 + .../expected/temp-schema-cleanup.out | 10 --- .../isolation/specs/temp-schema-cleanup.spec | 5 -- .../regress/expected/create_function_sql.out | 73 +++++++++++++++---- src/test/regress/expected/rangefuncs.out | 4 + src/test/regress/expected/returning.out | 17 ++++- src/test/regress/expected/rowtypes.out | 10 +++ src/test/regress/sql/create_function_sql.sql | 39 ++++++++++ src/test/regress/sql/returning.sql | 10 ++- 13 files changed, 222 insertions(+), 40 deletions(-) diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index 61bdf4a577..9cb4c64f3d 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" @@ -2434,6 +2435,47 @@ process_function_rte_ref(RangeTblEntry *rte, AttrNumber attnum, attnum, rte->eref->aliasname))); } +/* + * 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. + * + * There's no need to identify all such objects; the caller will throw an + * error anyway, so identifying the first one seems sufficiently courteous. + * (We'd throw the error here, except we don't know the referencing object.) + */ +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. */ diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index b89b9ccda0..e2f8d61ace 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,42 @@ 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 + * complain if any are to temporary objects. This prevents the scenario + * where a non-temp function silently goes away at session exit due to a + * dependency on a temp object. However, we allow a function created in + * our own pg_temp namespace to refer to other objects in that namespace, + * since then they'd have similar lifespans anyway (and there are field + * uses of that scenario). + */ + if (find_temp_object(addrs, isTempNamespace(procNamespace), &temp_object)) + ereport(NOTICE, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + 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 0335e982b3..59d00638ee 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 5979580139..47d5047fe8 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 2712befc8e..81309b8ce3 100644 --- a/src/include/catalog/dependency.h +++ b/src/include/catalog/dependency.h @@ -123,6 +123,10 @@ extern void recordDependencyOnSingleRelExpr(const ObjectAddress *depender, 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 d10aee53a8..321ad2c70c 100644 --- a/src/test/isolation/expected/temp-schema-cleanup.out +++ b/src/test/isolation/expected/temp-schema-cleanup.out @@ -15,11 +15,6 @@ step s1_create_temp_objects: -- table after. CREATE TEMPORARY TABLE invalidate_catalog_cache(); - -- test non-temp function is dropped when depending on temp table - CREATE TEMPORARY TABLE just_give_me_a_type(id serial primary key); - - CREATE FUNCTION uses_a_temp_type(just_give_me_a_type) RETURNS int LANGUAGE sql AS $$SELECT 1;$$; - exec ---- @@ -72,11 +67,6 @@ step s1_create_temp_objects: -- table after. CREATE TEMPORARY TABLE invalidate_catalog_cache(); - -- test non-temp function is dropped when depending on temp table - CREATE TEMPORARY TABLE just_give_me_a_type(id serial primary key); - - CREATE FUNCTION uses_a_temp_type(just_give_me_a_type) RETURNS int LANGUAGE sql AS $$SELECT 1;$$; - exec ---- diff --git a/src/test/isolation/specs/temp-schema-cleanup.spec b/src/test/isolation/specs/temp-schema-cleanup.spec index 72decba6cb..82416e1c76 100644 --- a/src/test/isolation/specs/temp-schema-cleanup.spec +++ b/src/test/isolation/specs/temp-schema-cleanup.spec @@ -35,11 +35,6 @@ step s1_create_temp_objects { -- that newer objects are deleted before older objects, so create a -- table after. CREATE TEMPORARY TABLE invalidate_catalog_cache(); - - -- test non-temp function is dropped when depending on temp table - CREATE TEMPORARY TABLE just_give_me_a_type(id serial primary key); - - CREATE FUNCTION uses_a_temp_type(just_give_me_a_type) RETURNS int LANGUAGE sql AS $$SELECT 1;$$; } step s1_discard_temp { diff --git a/src/test/regress/expected/create_function_sql.out b/src/test/regress/expected/create_function_sql.out index 73c6730d45..da86701a77 100644 --- a/src/test/regress/expected/create_function_sql.out +++ b/src/test/regress/expected/create_function_sql.out @@ -297,6 +297,45 @@ 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 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; +-- 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 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; +NOTICE: function "functest_temp_view" will be effectively temporary +DETAIL: It depends on temporary view temp_view. +-- 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; +NOTICE: function "functest_temp_sequence" will be effectively temporary +DETAIL: It depends on temporary sequence temp_seq. +-- 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; +NOTICE: function "functest_temp_domain" will be effectively temporary +DETAIL: It depends on temporary type temp_domain. -- check reporting of parse-analysis errors CREATE FUNCTION functest_S_xx(x date) RETURNS boolean LANGUAGE SQL @@ -516,27 +555,30 @@ SELECT r0.routine_name, r1.routine_name SELECT routine_name, sequence_name FROM information_schema.routine_sequence_usage WHERE routine_schema = 'temp_func_test' ORDER BY 1, 2; - routine_name | sequence_name ----------------+--------------- - functest_is_5 | functest1 - functest_is_6 | functest1 -(2 rows) + routine_name | sequence_name +------------------------+--------------- + functest_is_5 | functest1 + functest_is_6 | functest1 + functest_temp_sequence | temp_seq +(3 rows) SELECT routine_name, table_name, column_name FROM information_schema.routine_column_usage WHERE routine_schema = 'temp_func_test' ORDER BY 1, 2; - routine_name | table_name | column_name ----------------+------------+------------- - functest_is_7 | functest2 | a -(1 row) + routine_name | table_name | column_name +--------------------+------------+------------- + functest_is_7 | functest2 | a + functest_temp_view | temp_view | val +(2 rows) SELECT routine_name, table_name FROM information_schema.routine_table_usage WHERE routine_schema = 'temp_func_test' ORDER BY 1, 2; - routine_name | table_name ----------------+------------ - functest_is_7 | functest2 -(1 row) + routine_name | table_name +--------------------+------------ + functest_is_7 | functest2 + functest_temp_view | temp_view +(2 rows) DROP FUNCTION functest_IS_4a CASCADE; NOTICE: drop cascades to function functest_is_4b(integer) @@ -790,7 +832,7 @@ CONTEXT: SQL function "test1" during startup RESET check_function_bodies; -- Cleanup DROP SCHEMA temp_func_test CASCADE; -NOTICE: drop cascades to 38 other objects +NOTICE: drop cascades to 41 other objects DETAIL: drop cascades to function functest_a_1(text,date) drop cascades to function functest_a_2(text[]) drop cascades to function functest_a_3() @@ -812,6 +854,9 @@ drop cascades to function functest_s_3() drop cascades to function functest_s_3a() drop cascades to function functest_s_10(text,date) drop cascades to function functest_s_13() +drop cascades to function functest_temp_view() +drop cascades to function functest_temp_sequence() +drop cascades to function functest_temp_domain() drop cascades to function functest_s_15(integer) drop cascades to function functest_b_2(bigint) drop cascades to function functest_srf0() diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out index 30241e22da..5cc94011e9 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 d02c2ceab5..16012af6d1 100644 --- a/src/test/regress/expected/returning.out +++ b/src/test/regress/expected/returning.out @@ -1,6 +1,10 @@ -- -- Test INSERT/UPDATE/DELETE RETURNING -- +-- This script is full of poorly-chosen object names. +-- Put them in a separate schema to avoid collisions with concurrent scripts. +CREATE SCHEMA returning_test; +SET search_path = returning_test, public; -- Simple cases CREATE TEMP TABLE foo (f1 serial, f2 text, f3 int default 42); INSERT INTO foo (f2,f3) @@ -306,6 +310,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,8 +936,10 @@ 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() +CREATE OR REPLACE FUNCTION returning_test.foo_update() RETURNS void LANGUAGE sql BEGIN ATOMIC @@ -985,4 +993,9 @@ BEGIN ATOMIC FROM foo foo_1 WHERE (foo_1.* = n.*)) AS count; END -DROP FUNCTION foo_update; +-- Clean up +RESET search_path; +DROP SCHEMA returning_test CASCADE; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to function returning_test.joinview_upd_trig_fn() +drop cascades to function returning_test.foo_update() diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out index 677ad2ab9a..956bc2d02f 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 3d5f2a9209..7c6361d606 100644 --- a/src/test/regress/sql/create_function_sql.sql +++ b/src/test/regress/sql/create_function_sql.sql @@ -199,6 +199,45 @@ CREATE FUNCTION functest_S_xx(x anyarray) RETURNS anyelement LANGUAGE SQL RETURN x[1]; +CREATE TEMPORARY TABLE temp_table AS SELECT 1 AS val; +CREATE TEMPORARY VIEW temp_view AS SELECT 42 AS val; +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; + +-- 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 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; + +-- 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; + +-- 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; + -- check reporting of parse-analysis errors CREATE FUNCTION functest_S_xx(x date) RETURNS boolean LANGUAGE SQL diff --git a/src/test/regress/sql/returning.sql b/src/test/regress/sql/returning.sql index cc99cb53f6..f1c85a9731 100644 --- a/src/test/regress/sql/returning.sql +++ b/src/test/regress/sql/returning.sql @@ -2,6 +2,11 @@ -- Test INSERT/UPDATE/DELETE RETURNING -- +-- This script is full of poorly-chosen object names. +-- Put them in a separate schema to avoid collisions with concurrent scripts. +CREATE SCHEMA returning_test; +SET search_path = returning_test, public; + -- Simple cases CREATE TEMP TABLE foo (f1 serial, f2 text, f3 int default 42); @@ -407,4 +412,7 @@ BEGIN ATOMIC END; \sf foo_update -DROP FUNCTION foo_update; + +-- Clean up +RESET search_path; +DROP SCHEMA returning_test CASCADE; -- 2.43.0