From e8490e07265a9a83c192dc4b3cb31c6d1509b130 Mon Sep 17 00:00:00 2001 From: Alexander Pyhalov Date: Thu, 27 Mar 2025 09:02:47 +0300 Subject: [PATCH 3/3] 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 ++- src/test/regress/expected/plancache.out | 28 +++++++ src/test/regress/sql/plancache.sql | 27 +++++++ 4 files changed, 99 insertions(+), 43 deletions(-) diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index 7d12ca126e6..548b1c0c2f3 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" @@ -980,6 +981,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 @@ -1139,32 +1141,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 { @@ -1175,25 +1162,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 @@ -1203,7 +1180,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 @@ -1215,13 +1195,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(); diff --git a/src/test/regress/expected/plancache.out b/src/test/regress/expected/plancache.out index 4e59188196c..815618cedb6 100644 --- a/src/test/regress/expected/plancache.out +++ b/src/test/regress/expected/plancache.out @@ -398,3 +398,31 @@ select name, generic_plans, custom_plans from pg_prepared_statements (1 row) drop table test_mode; +-- Here SQL function return type is adjusted to match target. +-- Plan is saved, but dropped immediately on the first cached plan +-- usage. When plan is revalidated, return type should match expected one. +create table t1 (id int, nspname name, tname name); +create table t2 (id int, nspname text, tname text); +CREATE OR REPLACE FUNCTION get_values(id int) + RETURNS TABLE(id int, nspname text, tname text) + LANGUAGE sql +AS $function$ + DISCARD PLANS; + SELECT + id, nspname, tname + FROM t1 + WHERE id = $1; +$function$; +set plan_cache_mode = force_generic_plan; +INSERT INTO t1 VALUES (1, 'nspname', 'tname'); +INSERT INTO t2 SELECT * FROM get_values(1); +INSERT INTO t2 SELECT * FROM get_values(1); +reset plan_cache_mode; +SELECT * FROM t2; + id | nspname | tname +----+---------+------- + 1 | nspname | tname + 1 | nspname | tname +(2 rows) + +drop table t1, t2; diff --git a/src/test/regress/sql/plancache.sql b/src/test/regress/sql/plancache.sql index 4b2f11dcc64..20e93ccee18 100644 --- a/src/test/regress/sql/plancache.sql +++ b/src/test/regress/sql/plancache.sql @@ -223,3 +223,30 @@ select name, generic_plans, custom_plans from pg_prepared_statements where name = 'test_mode_pp'; drop table test_mode; + +-- Here SQL function return type is adjusted to match target. +-- Plan is saved, but dropped immediately on the first cached plan +-- usage. When plan is revalidated, return type should match expected one. +create table t1 (id int, nspname name, tname name); +create table t2 (id int, nspname text, tname text); + +CREATE OR REPLACE FUNCTION get_values(id int) + RETURNS TABLE(id int, nspname text, tname text) + LANGUAGE sql +AS $function$ + DISCARD PLANS; + SELECT + id, nspname, tname + FROM t1 + WHERE id = $1; +$function$; + +set plan_cache_mode = force_generic_plan; +INSERT INTO t1 VALUES (1, 'nspname', 'tname'); + +INSERT INTO t2 SELECT * FROM get_values(1); +INSERT INTO t2 SELECT * FROM get_values(1); +reset plan_cache_mode; +SELECT * FROM t2; + +drop table t1, t2; -- 2.43.0