aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2023-09-25 14:41:57 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2023-09-25 14:42:17 -0400
commitdc8d72c1c213d65a3f1500111d3c773aabce419f (patch)
tree4167229fd4cbe1726fbbf8606838fe6398c9f741
parenta2c2fbf740fcfdf75b76d589b8e85c1cdf725a46 (diff)
downloadpostgresql-dc8d72c1c213d65a3f1500111d3c773aabce419f.tar.gz
postgresql-dc8d72c1c213d65a3f1500111d3c773aabce419f.zip
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 ee895a655, 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
-rw-r--r--src/backend/optimizer/plan/setrefs.c23
-rw-r--r--src/pl/plpgsql/src/expected/plpgsql_call.out47
-rw-r--r--src/pl/plpgsql/src/sql/plpgsql_call.sql38
3 files changed, 106 insertions, 2 deletions
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 5700bfb5cd2..79622008854 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -3565,8 +3565,27 @@ extract_query_dependencies_walker(Node *node, PlannerInfo *context)
if (query->commandType == CMD_UTILITY)
{
/*
- * Ignore utility statements, except those (such as EXPLAIN) that
- * contain a parsed-but-not-planned query.
+ * This logic must handle any utility command for which parse
+ * analysis was nontrivial (cf. stmt_requires_parse_analysis).
+ *
+ * Notably, CALL requires its own processing.
+ */
+ if (IsA(query->utilityStmt, CallStmt))
+ {
+ CallStmt *callstmt = (CallStmt *) query->utilityStmt;
+
+ /* We need not examine funccall, just the transformed exprs */
+ (void) extract_query_dependencies_walker((Node *) callstmt->funcexpr,
+ context);
+ (void) extract_query_dependencies_walker((Node *) callstmt->outargs,
+ context);
+ return false;
+ }
+
+ /*
+ * Ignore other utility statements, except those (such as EXPLAIN)
+ * that contain a parsed-but-not-planned query. For those, we
+ * just need to transfer our attention to the contained query.
*/
query = UtilityContainsQuery(query->utilityStmt);
if (query == NULL)
diff --git a/src/pl/plpgsql/src/expected/plpgsql_call.out b/src/pl/plpgsql/src/expected/plpgsql_call.out
index 7ab23c6a21f..ab16416c1e2 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_call.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_call.out
@@ -483,3 +483,50 @@ BEGIN
END;
$$;
NOTICE: <NULL>
+-- check that we detect change of dependencies in CALL
+-- atomic and non-atomic call sites used to do this differently, so check both
+CREATE PROCEDURE inner_p (f1 int)
+AS $$
+BEGIN
+ RAISE NOTICE 'inner_p(%)', f1;
+END
+$$ LANGUAGE plpgsql;
+CREATE FUNCTION f(int) RETURNS int AS $$ SELECT $1 + 1 $$ LANGUAGE sql;
+CREATE PROCEDURE outer_p (f1 int)
+AS $$
+BEGIN
+ RAISE NOTICE 'outer_p(%)', f1;
+ CALL inner_p(f(f1));
+END
+$$ LANGUAGE plpgsql;
+CREATE FUNCTION outer_f (f1 int) RETURNS void
+AS $$
+BEGIN
+ RAISE NOTICE 'outer_f(%)', f1;
+ CALL inner_p(f(f1));
+END
+$$ LANGUAGE plpgsql;
+CALL outer_p(42);
+NOTICE: outer_p(42)
+NOTICE: inner_p(43)
+SELECT outer_f(42);
+NOTICE: outer_f(42)
+NOTICE: inner_p(43)
+ outer_f
+---------
+
+(1 row)
+
+DROP FUNCTION f(int);
+CREATE FUNCTION f(int) RETURNS int AS $$ SELECT $1 + 2 $$ LANGUAGE sql;
+CALL outer_p(42);
+NOTICE: outer_p(42)
+NOTICE: inner_p(44)
+SELECT outer_f(42);
+NOTICE: outer_f(42)
+NOTICE: inner_p(44)
+ outer_f
+---------
+
+(1 row)
+
diff --git a/src/pl/plpgsql/src/sql/plpgsql_call.sql b/src/pl/plpgsql/src/sql/plpgsql_call.sql
index 14bbffa0b2e..8efc8e823ac 100644
--- a/src/pl/plpgsql/src/sql/plpgsql_call.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_call.sql
@@ -454,3 +454,41 @@ BEGIN
RAISE NOTICE '%', v_Text;
END;
$$;
+
+
+-- check that we detect change of dependencies in CALL
+-- atomic and non-atomic call sites used to do this differently, so check both
+
+CREATE PROCEDURE inner_p (f1 int)
+AS $$
+BEGIN
+ RAISE NOTICE 'inner_p(%)', f1;
+END
+$$ LANGUAGE plpgsql;
+
+CREATE FUNCTION f(int) RETURNS int AS $$ SELECT $1 + 1 $$ LANGUAGE sql;
+
+CREATE PROCEDURE outer_p (f1 int)
+AS $$
+BEGIN
+ RAISE NOTICE 'outer_p(%)', f1;
+ CALL inner_p(f(f1));
+END
+$$ LANGUAGE plpgsql;
+
+CREATE FUNCTION outer_f (f1 int) RETURNS void
+AS $$
+BEGIN
+ RAISE NOTICE 'outer_f(%)', f1;
+ CALL inner_p(f(f1));
+END
+$$ LANGUAGE plpgsql;
+
+CALL outer_p(42);
+SELECT outer_f(42);
+
+DROP FUNCTION f(int);
+CREATE FUNCTION f(int) RETURNS int AS $$ SELECT $1 + 2 $$ LANGUAGE sql;
+
+CALL outer_p(42);
+SELECT outer_f(42);