From 19c91b224dbf4342f8305037c95cc90d357c08d9 Mon Sep 17 00:00:00 2001 From: Alexander Pyhalov Date: Wed, 5 Mar 2025 18:51:37 +0300 Subject: [PATCH 4/4] Handle SQL functions which result type is adjuisted Query could be modified between rewrite and plan stages by check_sql_fn_retval(). We move this step earlier, so that cached plans were created with already modified tlist. In this case if later revalidation is considered by RevalidateCachedQuery(), modifications, done by check_sql_fn_retval(), will not be lost. We consider that rewriting query cannot ever changes the targetlist results. Note that test_extensions result has changed as cached query can be revalidated after extension is moved to another schema - function oid in the query still matches the existing one. --- src/backend/executor/functions.c | 76 ++++++++++--------- .../expected/test_extensions.out | 11 ++- 2 files changed, 44 insertions(+), 43 deletions(-) diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index 06efcec4e7c..48db2ef061a 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -25,6 +25,7 @@ #include "miscadmin.h" #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" +#include "parser/analyze.h" #include "parser/parse_coerce.h" #include "parser/parse_collate.h" #include "parser/parse_func.h" @@ -948,6 +949,7 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool *lazyEvalOK) SQLFunctionPlanKey plan_cache_entry_key; bool use_plan_cache; bool plan_cache_entry_valid; + List *query_list; /* * Create memory context that holds all the SQLFunctionCache data. It @@ -1098,32 +1100,17 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool *lazyEvalOK) plansource_list = NIL; - queryTree_list = NIL; + /* Construct a list of analyzed parsetrees. */ + query_list = NIL; if (!isNull) { Node *n; - List *stored_query_list; n = stringToNode(TextDatumGetCString(tmp)); if (IsA(n, List)) - stored_query_list = linitial_node(List, castNode(List, n)); + query_list = linitial_node(List, castNode(List, n)); else - stored_query_list = list_make1(n); - - foreach(lc, stored_query_list) - { - Query *parsetree = lfirst_node(Query, lc); - List *queryTree_sublist; - CachedPlanSource *plansource; - - AcquireRewriteLocks(parsetree, true, false); - - plansource = CreateCachedPlanForQuery(parsetree, fcache->src, CreateCommandTag((Node *) parsetree)); - plansource_list = lappend(plansource_list, plansource); - - queryTree_sublist = pg_rewrite_query(parsetree); - queryTree_list = lappend(queryTree_list, queryTree_sublist); - } + query_list = list_make1(n); } else { @@ -1134,25 +1121,15 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool *lazyEvalOK) foreach(lc, raw_parsetree_list) { RawStmt *parsetree = lfirst_node(RawStmt, lc); - List *queryTree_sublist; - CachedPlanSource *plansource; - - plansource = CreateCachedPlan(parsetree, fcache->src, CreateCommandTag(parsetree->stmt)); - plansource_list = lappend(plansource_list, plansource); - - queryTree_sublist = pg_analyze_and_rewrite_withcb(parsetree, - fcache->src, - (ParserSetupHook) sql_fn_parser_setup, - fcache->pinfo, - NULL); - queryTree_list = lappend(queryTree_list, queryTree_sublist); + Query *query; + + query = parse_analyze_withcb(parsetree, fcache->src, (ParserSetupHook) sql_fn_parser_setup, fcache->pinfo, + NULL); + + query_list = lappend(query_list, query); } } - /* - * Check that there are no statements we don't want to allow. - */ - check_sql_fn_statements(queryTree_list); /* * Check that the function returns the type it claims to. Although in @@ -1162,7 +1139,10 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool *lazyEvalOK) * the function had any polymorphic arguments. Moreover, * check_sql_fn_retval takes care of injecting any required column * type coercions. (But we don't ask it to insert nulls for dropped - * columns; the junkfilter handles that.) + * columns; the junkfilter handles that.) As check_sql_fn_retval() can + * modify queries to match expected return types, we execute it prior + * to creating cached plans, so that if revalidation happens and + * triggers query rewriting, return type would be already correct. * * Note: we set fcache->returnsTuple according to whether we are * returning the whole tuple result or just a single column. In the @@ -1174,13 +1154,35 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool *lazyEvalOK) * do that.) */ - fcache->returnsTuple = check_sql_fn_retval(queryTree_list, + fcache->returnsTuple = check_sql_fn_retval(list_make1(query_list), rettype, rettupdesc, procedureStruct->prokind, false, &resulttlist); + queryTree_list = NIL; + + foreach(lc, query_list) + { + Query *parsetree = lfirst_node(Query, lc); + List *queryTree_sublist; + CachedPlanSource *plansource; + + AcquireRewriteLocks(parsetree, true, false); + + plansource = CreateCachedPlanForQuery(parsetree, fcache->src, CreateCommandTag((Node *) parsetree)); + plansource_list = lappend(plansource_list, plansource); + + queryTree_sublist = pg_rewrite_query(parsetree); + queryTree_list = lappend(queryTree_list, queryTree_sublist); + } + + /* + * Check that there are no statements we don't want to allow. + */ + check_sql_fn_statements(queryTree_list); + /* * Queries could be rewritten by check_sql_fn_retval(). Now when they * have their final form, we can complete plan cache entry creation. diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out index 72bae1bf254..ea3d4ca61d8 100644 --- a/src/test/modules/test_extensions/expected/test_extensions.out +++ b/src/test/modules/test_extensions/expected/test_extensions.out @@ -646,12 +646,11 @@ SELECT dep_req3(); (1 row) SELECT dep_req3b(); -- fails -ERROR: function public.dep_req2() does not exist -LINE 1: SELECT public.dep_req2() || ' req3b' - ^ -HINT: No function matches the given name and argument types. You might need to add explicit type casts. -QUERY: SELECT public.dep_req2() || ' req3b' -CONTEXT: SQL function "dep_req3b" statement 1 + dep_req3b +----------------- + req1 req2 req3b +(1 row) + DROP EXTENSION test_ext_req_schema3; ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok SELECT test_s_dep2.dep_req1(); -- 2.43.0