Skip to content

Commit fdc7cf7

Browse files
committed
Collect dependency information for parsed CallStmts.
Parse analysis of a CallStmt will inject mutable information, for instance the OID of the called procedure, so that subsequent DDL may create a need to re-parse the CALL. We failed to detect this for CALLs in plpgsql routines, because no dependency information was collected when putting a CallStmt into the plan cache. That could lead to misbehavior or strange errors such as "cache lookup failed". Before commit ee895a6, the issue would only manifest for CALLs appearing in atomic contexts, because we re-planned non-atomic CALLs every time through anyway. It is now apparent that extract_query_dependencies() probably needs a special case for every utility statement type for which stmt_requires_parse_analysis() returns true. I wanted to add something like Assert(!stmt_requires_parse_analysis(...)) when falling out of extract_query_dependencies_walker without doing anything, but there are API issues as well as a more fundamental point: stmt_requires_parse_analysis is supposed to be applied to raw parser output, so it'd be cheating to assume it will give the correct answer for post-parse-analysis trees. I contented myself with adding a comment. Per bug #18131 from Christian Stork. Back-patch to all supported branches. Discussion: https://postgr.es/m/18131-576854e79c5cd264@postgresql.org
1 parent 0fb91ed commit fdc7cf7

File tree

3 files changed

+104
-2
lines changed

3 files changed

+104
-2
lines changed

src/backend/optimizer/plan/setrefs.c

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2649,8 +2649,25 @@ extract_query_dependencies_walker(Node *node, PlannerInfo *context)
26492649
if (query->commandType == CMD_UTILITY)
26502650
{
26512651
/*
2652-
* Ignore utility statements, except those (such as EXPLAIN) that
2653-
* contain a parsed-but-not-planned query.
2652+
* This logic must handle any utility command for which parse
2653+
* analysis was nontrivial (cf. stmt_requires_parse_analysis).
2654+
*
2655+
* Notably, CALL requires its own processing.
2656+
*/
2657+
if (IsA(query->utilityStmt, CallStmt))
2658+
{
2659+
CallStmt *callstmt = (CallStmt *) query->utilityStmt;
2660+
2661+
/* We need not examine funccall, just the transformed exprs */
2662+
(void) extract_query_dependencies_walker((Node *) callstmt->funcexpr,
2663+
context);
2664+
return false;
2665+
}
2666+
2667+
/*
2668+
* Ignore other utility statements, except those (such as EXPLAIN)
2669+
* that contain a parsed-but-not-planned query. For those, we
2670+
* just need to transfer our attention to the contained query.
26542671
*/
26552672
query = UtilityContainsQuery(query->utilityStmt);
26562673
if (query == NULL)

src/pl/plpgsql/src/expected/plpgsql_call.out

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,3 +376,50 @@ BEGIN
376376
END;
377377
$$;
378378
NOTICE: <NULL>
379+
-- check that we detect change of dependencies in CALL
380+
-- atomic and non-atomic call sites do this differently, so check both
381+
CREATE PROCEDURE inner_p (f1 int)
382+
AS $$
383+
BEGIN
384+
RAISE NOTICE 'inner_p(%)', f1;
385+
END
386+
$$ LANGUAGE plpgsql;
387+
CREATE FUNCTION f(int) RETURNS int AS $$ SELECT $1 + 1 $$ LANGUAGE sql;
388+
CREATE PROCEDURE outer_p (f1 int)
389+
AS $$
390+
BEGIN
391+
RAISE NOTICE 'outer_p(%)', f1;
392+
CALL inner_p(f(f1));
393+
END
394+
$$ LANGUAGE plpgsql;
395+
CREATE FUNCTION outer_f (f1 int) RETURNS void
396+
AS $$
397+
BEGIN
398+
RAISE NOTICE 'outer_f(%)', f1;
399+
CALL inner_p(f(f1));
400+
END
401+
$$ LANGUAGE plpgsql;
402+
CALL outer_p(42);
403+
NOTICE: outer_p(42)
404+
NOTICE: inner_p(43)
405+
SELECT outer_f(42);
406+
NOTICE: outer_f(42)
407+
NOTICE: inner_p(43)
408+
outer_f
409+
---------
410+
411+
(1 row)
412+
413+
DROP FUNCTION f(int);
414+
CREATE FUNCTION f(int) RETURNS int AS $$ SELECT $1 + 2 $$ LANGUAGE sql;
415+
CALL outer_p(42);
416+
NOTICE: outer_p(42)
417+
NOTICE: inner_p(44)
418+
SELECT outer_f(42);
419+
NOTICE: outer_f(42)
420+
NOTICE: inner_p(44)
421+
outer_f
422+
---------
423+
424+
(1 row)
425+

src/pl/plpgsql/src/sql/plpgsql_call.sql

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,3 +357,41 @@ BEGIN
357357
RAISE NOTICE '%', v_Text;
358358
END;
359359
$$;
360+
361+
362+
-- check that we detect change of dependencies in CALL
363+
-- atomic and non-atomic call sites do this differently, so check both
364+
365+
CREATE PROCEDURE inner_p (f1 int)
366+
AS $$
367+
BEGIN
368+
RAISE NOTICE 'inner_p(%)', f1;
369+
END
370+
$$ LANGUAGE plpgsql;
371+
372+
CREATE FUNCTION f(int) RETURNS int AS $$ SELECT $1 + 1 $$ LANGUAGE sql;
373+
374+
CREATE PROCEDURE outer_p (f1 int)
375+
AS $$
376+
BEGIN
377+
RAISE NOTICE 'outer_p(%)', f1;
378+
CALL inner_p(f(f1));
379+
END
380+
$$ LANGUAGE plpgsql;
381+
382+
CREATE FUNCTION outer_f (f1 int) RETURNS void
383+
AS $$
384+
BEGIN
385+
RAISE NOTICE 'outer_f(%)', f1;
386+
CALL inner_p(f(f1));
387+
END
388+
$$ LANGUAGE plpgsql;
389+
390+
CALL outer_p(42);
391+
SELECT outer_f(42);
392+
393+
DROP FUNCTION f(int);
394+
CREATE FUNCTION f(int) RETURNS int AS $$ SELECT $1 + 2 $$ LANGUAGE sql;
395+
396+
CALL outer_p(42);
397+
SELECT outer_f(42);

0 commit comments

Comments
 (0)