Skip to content

Commit 572c40b

Browse files
tglsfdctglsfdc
committed
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
1 parent 81966c5 commit 572c40b

File tree

11 files changed

+176
-19
lines changed

11 files changed

+176
-19
lines changed

‎src/backend/catalog/dependency.c‎

Lines changed: 88 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "catalog/dependency.h"
2323
#include "catalog/heap.h"
2424
#include "catalog/index.h"
25+
#include "catalog/namespace.h"
2526
#include "catalog/objectaccess.h"
2627
#include "catalog/pg_am.h"
2728
#include "catalog/pg_amop.h"
@@ -1554,25 +1555,57 @@ recordDependencyOnExpr(const ObjectAddress *depender,
15541555
Node *expr, List *rtable,
15551556
DependencyType behavior)
15561557
{
1557-
find_expr_references_context context;
1558+
ObjectAddresses *addrs;
15581559

1559-
context.addrs = new_object_addresses();
1560+
addrs = new_object_addresses();
15601561

1561-
/* Set up interpretation for Vars at varlevelsup = 0 */
1562-
context.rtables = list_make1(rtable);
1562+
/* Collect all dependencies from the expression */
1563+
collectDependenciesOfExpr(addrs, expr, rtable);
15631564

1564-
/* Scan the expression tree for referenceable objects */
1565-
find_expr_references_walker(expr, &context);
1566-
1567-
/* Remove any duplicates */
1568-
eliminate_duplicate_dependencies(context.addrs);
1565+
/* Remove duplicates */
1566+
eliminate_duplicate_dependencies(addrs);
15691567

15701568
/* And record 'em */
15711569
recordMultipleDependencies(depender,
1572-
context.addrs->refs, context.addrs->numrefs,
1570+
addrs->refs, addrs->numrefs,
15731571
behavior);
15741572

1575-
free_object_addresses(context.addrs);
1573+
free_object_addresses(addrs);
1574+
}
1575+
1576+
/*
1577+
* collectDependenciesOfExpr - collect expression dependencies
1578+
*
1579+
* This function analyzes an expression or query in node-tree form to
1580+
* find all the objects it refers to (tables, columns, operators,
1581+
* functions, etc.) and adds them to the provided ObjectAddresses
1582+
* structure. Unlike recordDependencyOnExpr, this function does not
1583+
* immediately record the dependencies, allowing the caller to add to,
1584+
* filter, or modify the collected dependencies before recording them.
1585+
*
1586+
* rtable is the rangetable to be used to interpret Vars with varlevelsup=0.
1587+
* It can be NIL if no such variables are expected.
1588+
*
1589+
* Note: the returned list may well contain duplicates. The caller should
1590+
* de-duplicate before recording the dependencies. Within this file, callers
1591+
* must call eliminate_duplicate_dependencies(). External callers typically
1592+
* go through record_object_address_dependencies() which will see to that.
1593+
* This choice allows collecting dependencies from multiple sources without
1594+
* redundant de-duplication work.
1595+
*/
1596+
void
1597+
collectDependenciesOfExpr(ObjectAddresses *addrs,
1598+
Node *expr, List *rtable)
1599+
{
1600+
find_expr_references_context context;
1601+
1602+
context.addrs = addrs;
1603+
1604+
/* Set up interpretation for Vars at varlevelsup = 0 */
1605+
context.rtables = list_make1(rtable);
1606+
1607+
/* Scan the expression tree for referenceable objects */
1608+
find_expr_references_walker(expr, &context);
15761609
}
15771610

15781611
/*
@@ -2402,6 +2435,50 @@ process_function_rte_ref(RangeTblEntry *rte, AttrNumber attnum,
24022435
attnum, rte->eref->aliasname)));
24032436
}
24042437

2438+
/*
2439+
* find_temp_object - search an array of dependency references for temp objects
2440+
*
2441+
* Scan an ObjectAddresses array for references to temporary objects (objects
2442+
* in temporary namespaces), ignoring those in our own temp namespace if
2443+
* local_temp_okay is true. If one is found, return true after storing its
2444+
* address in *foundobj.
2445+
*
2446+
* Current callers only use this to deliver helpful notices, so reporting
2447+
* one such object seems sufficient. We return the first one, which should
2448+
* be a stable result for a given query since it depends only on the order
2449+
* in which this module searches query trees. (However, it's important to
2450+
* call this before de-duplicating the objects, else OID order would affect
2451+
* the result.)
2452+
*/
2453+
bool
2454+
find_temp_object(const ObjectAddresses *addrs, bool local_temp_okay,
2455+
ObjectAddress *foundobj)
2456+
{
2457+
for (int i = 0; i < addrs->numrefs; i++)
2458+
{
2459+
const ObjectAddress *thisobj = addrs->refs + i;
2460+
Oid objnamespace;
2461+
2462+
/*
2463+
* Use get_object_namespace() to see if this object belongs to a
2464+
* schema. If not, we can skip it.
2465+
*/
2466+
objnamespace = get_object_namespace(thisobj);
2467+
2468+
/*
2469+
* If the object is in a temporary namespace, complain, except if
2470+
* local_temp_okay and it's our own temp namespace.
2471+
*/
2472+
if (OidIsValid(objnamespace) && isAnyTempNamespace(objnamespace) &&
2473+
!(local_temp_okay && isTempNamespace(objnamespace)))
2474+
{
2475+
*foundobj = *thisobj;
2476+
return true;
2477+
}
2478+
}
2479+
return false;
2480+
}
2481+
24052482
/*
24062483
* Given an array of dependency references, eliminate any duplicates.
24072484
*/

‎src/backend/catalog/pg_proc.c‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "catalog/catalog.h"
2121
#include "catalog/dependency.h"
2222
#include "catalog/indexing.h"
23+
#include "catalog/namespace.h"
2324
#include "catalog/objectaccess.h"
2425
#include "catalog/pg_language.h"
2526
#include "catalog/pg_namespace.h"
@@ -141,7 +142,8 @@ ProcedureCreate(const char *procedureName,
141142
TupleDesc tupDesc;
142143
bool is_update;
143144
ObjectAddress myself,
144-
referenced;
145+
referenced,
146+
temp_object;
145147
char *detailmsg;
146148
int i;
147149
ObjectAddresses *addrs;
@@ -658,17 +660,40 @@ ProcedureCreate(const char *procedureName,
658660
add_exact_object_address(&referenced, addrs);
659661
}
660662

661-
record_object_address_dependencies(&myself, addrs, DEPENDENCY_NORMAL);
662-
free_object_addresses(addrs);
663-
664-
/* dependency on SQL routine body */
663+
/* dependencies appearing in new-style SQL routine body */
665664
if (languageObjectId == SQLlanguageId && prosqlbody)
666-
recordDependencyOnExpr(&myself, prosqlbody, NIL, DEPENDENCY_NORMAL);
665+
collectDependenciesOfExpr(addrs, prosqlbody, NIL);
667666

668667
/* dependency on parameter default expressions */
669668
if (parameterDefaults)
670-
recordDependencyOnExpr(&myself, (Node *) parameterDefaults,
671-
NIL, DEPENDENCY_NORMAL);
669+
collectDependenciesOfExpr(addrs, (Node *) parameterDefaults, NIL);
670+
671+
/*
672+
* Now that we have all the normal dependencies, thumb through them and
673+
* warn if any are to temporary objects. This informs the user if their
674+
* supposedly non-temp function will silently go away at session exit, due
675+
* to a dependency on a temp object. However, do not complain when a
676+
* function created in our own pg_temp namespace refers to other objects
677+
* in that namespace, since then they'll have similar lifespans anyway.
678+
*/
679+
if (find_temp_object(addrs, isTempNamespace(procNamespace), &temp_object))
680+
ereport(NOTICE,
681+
(errmsg("function \"%s\" will be effectively temporary",
682+
procedureName),
683+
errdetail("It depends on temporary %s.",
684+
getObjectDescription(&temp_object, false))));
685+
686+
/*
687+
* Now record all normal dependencies at once. This will also remove any
688+
* duplicates in the list. (Role and extension dependencies are handled
689+
* separately below. Role dependencies would have to be separate anyway
690+
* since they are shared dependencies. An extension dependency could be
691+
* folded into the addrs list, but pg_depend.c doesn't make that easy, and
692+
* it won't duplicate anything we've collected so far anyway.)
693+
*/
694+
record_object_address_dependencies(&myself, addrs, DEPENDENCY_NORMAL);
695+
696+
free_object_addresses(addrs);
672697

673698
/* dependency on owner */
674699
if (!is_update)

‎src/backend/commands/functioncmds.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,8 @@ compute_return_type(TypeName *returnType, Oid languageOid,
153153
address = TypeShellMake(typname, namespaceId, GetUserId());
154154
rettype = address.objectId;
155155
Assert(OidIsValid(rettype));
156+
/* Ensure the new shell type is visible to ProcedureCreate */
157+
CommandCounterIncrement();
156158
}
157159

158160
aclresult = object_aclcheck(TypeRelationId, rettype, GetUserId(), ACL_USAGE);

‎src/backend/commands/typecmds.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1742,6 +1742,9 @@ DefineRange(ParseState *pstate, CreateRangeStmt *stmt)
17421742
false, /* Type NOT NULL */
17431743
InvalidOid); /* typcollation */
17441744

1745+
/* Ensure these new types are visible to ProcedureCreate */
1746+
CommandCounterIncrement();
1747+
17451748
/* And create the constructor functions for this range type */
17461749
makeRangeConstructors(typeName, typeNamespace, typoid, rangeSubtype);
17471750
makeMultirangeConstructors(multirangeTypeName, typeNamespace,

‎src/include/catalog/dependency.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,19 @@ extern void recordDependencyOnExpr(const ObjectAddress *depender,
114114
Node *expr, List *rtable,
115115
DependencyType behavior);
116116

117+
extern void collectDependenciesOfExpr(ObjectAddresses *addrs,
118+
Node *expr, List *rtable);
119+
117120
extern void recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
118121
Node *expr, Oid relId,
119122
DependencyType behavior,
120123
DependencyType self_behavior,
121124
bool reverse_self);
122125

126+
extern bool find_temp_object(const ObjectAddresses *addrs,
127+
bool local_temp_okay,
128+
ObjectAddress *foundobj);
129+
123130
extern ObjectAddresses *new_object_addresses(void);
124131

125132
extern void add_exact_object_address(const ObjectAddress *object,

‎src/test/isolation/expected/temp-schema-cleanup.out‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ exec
2525

2626
(1 row)
2727

28+
s1: NOTICE: function "uses_a_temp_type" will be effectively temporary
29+
DETAIL: It depends on temporary type just_give_me_a_type.
2830
step s1_discard_temp:
2931
DISCARD TEMP;
3032

@@ -82,6 +84,8 @@ exec
8284

8385
(1 row)
8486

87+
s1: NOTICE: function "uses_a_temp_type" will be effectively temporary
88+
DETAIL: It depends on temporary type just_give_me_a_type.
8589
step s1_exit:
8690
SELECT pg_terminate_backend(pg_backend_pid());
8791

‎src/test/regress/expected/create_function_sql.out‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,16 @@ DROP TABLE functest3 CASCADE;
455455
NOTICE: drop cascades to 2 other objects
456456
DETAIL: drop cascades to view functestv3
457457
drop cascades to function functest_s_14()
458+
-- Check reporting of temporary-object dependencies within SQL-standard body
459+
-- (tests elsewhere already cover dependencies on arg and result types)
460+
CREATE TEMP SEQUENCE mytempseq;
461+
CREATE FUNCTION functest_tempseq() RETURNS int
462+
RETURN nextval('mytempseq');
463+
NOTICE: function "functest_tempseq" will be effectively temporary
464+
DETAIL: It depends on temporary sequence mytempseq.
465+
-- This discards mytempseq and therefore functest_tempseq(). If it fails to,
466+
-- the function will appear in the information_schema tests below.
467+
DISCARD TEMP;
458468
-- information_schema tests
459469
CREATE FUNCTION functest_IS_1(a int, b int default 1, c text default 'foo')
460470
RETURNS int

‎src/test/regress/expected/rangefuncs.out‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2178,6 +2178,8 @@ alter table users drop column todrop;
21782178
create or replace function get_first_user() returns users as
21792179
$$ SELECT * FROM users ORDER BY userid LIMIT 1; $$
21802180
language sql stable;
2181+
NOTICE: function "get_first_user" will be effectively temporary
2182+
DETAIL: It depends on temporary type users.
21812183
SELECT get_first_user();
21822184
get_first_user
21832185
-------------------
@@ -2193,6 +2195,8 @@ SELECT * FROM get_first_user();
21932195
create or replace function get_users() returns setof users as
21942196
$$ SELECT * FROM users ORDER BY userid; $$
21952197
language sql stable;
2198+
NOTICE: function "get_users" will be effectively temporary
2199+
DETAIL: It depends on temporary type users.
21962200
SELECT get_users();
21972201
get_users
21982202
---------------------

‎src/test/regress/expected/returning.out‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,8 @@ SELECT * FROM foo;
306306
-- Check use of a whole-row variable for an inlined set-returning function
307307
CREATE FUNCTION foo_f() RETURNS SETOF foo AS
308308
$$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE;
309+
NOTICE: function "foo_f" will be effectively temporary
310+
DETAIL: It depends on temporary type foo.
309311
UPDATE foo SET f2 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = foo.f1
310312
RETURNING foo_f;
311313
foo_f
@@ -930,6 +932,8 @@ BEGIN ATOMIC
930932
(SELECT count(*) FROM foo WHERE foo = o),
931933
(SELECT count(*) FROM foo WHERE foo = n);
932934
END;
935+
NOTICE: function "foo_update" will be effectively temporary
936+
DETAIL: It depends on temporary table foo.
933937
\sf foo_update
934938
CREATE OR REPLACE FUNCTION public.foo_update()
935939
RETURNS void

‎src/test/regress/expected/rowtypes.out‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -907,19 +907,27 @@ create temp table compos (f1 int, f2 text);
907907
create function fcompos1(v compos) returns void as $$
908908
insert into compos values (v); -- fail
909909
$$ language sql;
910+
NOTICE: function "fcompos1" will be effectively temporary
911+
DETAIL: It depends on temporary type compos.
910912
ERROR: column "f1" is of type integer but expression is of type compos
911913
LINE 2: insert into compos values (v); -- fail
912914
^
913915
HINT: You will need to rewrite or cast the expression.
914916
create function fcompos1(v compos) returns void as $$
915917
insert into compos values (v.*);
916918
$$ language sql;
919+
NOTICE: function "fcompos1" will be effectively temporary
920+
DETAIL: It depends on temporary type compos.
917921
create function fcompos2(v compos) returns void as $$
918922
select fcompos1(v);
919923
$$ language sql;
924+
NOTICE: function "fcompos2" will be effectively temporary
925+
DETAIL: It depends on temporary type compos.
920926
create function fcompos3(v compos) returns void as $$
921927
select fcompos1(fcompos3.v.*);
922928
$$ language sql;
929+
NOTICE: function "fcompos3" will be effectively temporary
930+
DETAIL: It depends on temporary type compos.
923931
select fcompos1(row(1,'one'));
924932
fcompos1
925933
----------
@@ -1012,6 +1020,8 @@ select last(f) from fullname f;
10121020

10131021
create function longname(fullname) returns text language sql
10141022
as $$select $1.first || ' ' || $1.last$$;
1023+
NOTICE: function "longname" will be effectively temporary
1024+
DETAIL: It depends on temporary type fullname.
10151025
select f.longname from fullname f;
10161026
longname
10171027
----------

0 commit comments

Comments
 (0)