From 7427db23bb4de73dddfdbbf9d6ed902079894aa1 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 29 Mar 2025 21:57:50 -0400 Subject: [PATCH v11 6/7] Change SQL-language functions to use the plan cache. In the historical implementation of SQL functions (when they don't get inlined), we built plans for the contained queries at first call within an outer query, and then re-used those plans for the duration of the outer query, and then forgot everything. This was not ideal, not least because the plans could not be customized to specific values of the function's parameters. Our plancache infrastructure seems mature enough to be used here. That will solve both the problem with not being able to build custom plans and the problem with not being able to share work across successive outer queries. Moreover, this reimplementation will react to events that should cause a replan at the next entry to the SQL function. This is illustrated in the change in the rowsecurity test, where we now detect an RLS context change that was previously ignored. (I also added a test in create_function_sql that exercises ShutdownSQLFunction(), after noting from coverage results that that wasn't getting reached.) Author: Alexander Pyhalov Co-authored-by: Tom Lane Discussion: https://postgr.es/m/8216639.NyiUUSuA9g@aivenlaptop --- src/backend/executor/functions.c | 1082 +++++++++++------ .../expected/test_extensions.out | 2 +- .../regress/expected/create_function_sql.out | 17 +- src/test/regress/expected/rowsecurity.out | 16 +- src/test/regress/sql/create_function_sql.sql | 9 + src/tools/pgindent/typedefs.list | 2 + 6 files changed, 738 insertions(+), 390 deletions(-) diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index 5b06df84335..b5a9ecea637 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -31,6 +31,7 @@ #include "tcop/utility.h" #include "utils/builtins.h" #include "utils/datum.h" +#include "utils/funccache.h" #include "utils/lsyscache.h" #include "utils/memutils.h" #include "utils/snapmgr.h" @@ -50,7 +51,7 @@ typedef struct /* * We have an execution_state record for each query in a function. Each - * record contains a plantree for its query. If the query is currently in + * record references a plantree for its query. If the query is currently in * F_EXEC_RUN state then there's a QueryDesc too. * * The "next" fields chain together all the execution_state records generated @@ -74,24 +75,43 @@ typedef struct execution_state /* - * An SQLFunctionCache record is built during the first call, - * and linked to from the fn_extra field of the FmgrInfo struct. + * Data associated with a SQL-language function is kept in three main + * data structures: * - * Note that currently this has only the lifespan of the calling query. - * Someday we should rewrite this code to use plancache.c to save parse/plan - * results for longer than that. + * 1. SQLFunctionHashEntry is a long-lived (potentially session-lifespan) + * struct that holds all the info we need out of the function's pg_proc row. + * In addition it holds pointers to CachedPlanSource(s) that manage creation + * of plans for the query(s) within the function. A SQLFunctionHashEntry is + * potentially shared across multiple concurrent executions of the function, + * so it must contain no execution-specific state; but its use_count must + * reflect the number of SQLFunctionLink structs pointing at it. + * If the function's pg_proc row is updated, we throw away and regenerate + * the SQLFunctionHashEntry and subsidiary data. (Also note that if the + * function is polymorphic or used as a trigger, there is a separate + * SQLFunctionHashEntry for each usage, so that we need consider only one + * set of relevant data types.) The struct itself is in memory managed by + * funccache.c, and its subsidiary data is kept in hcontext ("hash context"). * - * Physically, though, the data has the lifespan of the FmgrInfo that's used - * to call the function, and there are cases (particularly with indexes) - * where the FmgrInfo might survive across transactions. We cannot assume - * that the parse/plan trees are good for longer than the (sub)transaction in - * which parsing was done, so we must mark the record with the LXID/subxid of - * its creation time, and regenerate everything if that's obsolete. To avoid - * memory leakage when we do have to regenerate things, all the data is kept - * in a sub-context of the FmgrInfo's fn_mcxt. + * 2. SQLFunctionCache lasts for the duration of a single execution of + * the SQL function. (In "lazyEval" mode, this might span multiple calls of + * fmgr_sql.) It holds a reference to the CachedPlan for the current query, + * and other data that is execution-specific. The SQLFunctionCache itself + * as well as its subsidiary data are kept in fcontext ("function context"), + * which we free at completion. In non-returnsSet mode, this is just a child + * of the call-time context. In returnsSet mode, it is made a child of the + * FmgrInfo's fn_mcxt so that it will survive between fmgr_sql calls. + * + * 3. SQLFunctionLink is a tiny struct that just holds pointers to + * the SQLFunctionHashEntry and the current SQLFunctionCache (if any). + * It is pointed to by the fn_extra field of the FmgrInfo struct, and is + * always allocated in the FmgrInfo's fn_mcxt. Its purpose is to reduce + * the cost of repeat lookups of the SQLFunctionHashEntry. */ -typedef struct + +typedef struct SQLFunctionHashEntry { + CachedFunction cfunc; /* fields managed by funccache.c */ + char *fname; /* function name (for error msgs) */ char *src; /* function body text (for error msgs) */ @@ -102,8 +122,25 @@ typedef struct bool typbyval; /* true if return type is pass by value */ bool returnsSet; /* true if returning multiple rows */ bool returnsTuple; /* true if returning whole tuple result */ - bool shutdown_reg; /* true if registered shutdown callback */ bool readonly_func; /* true to run in "read only" mode */ + char prokind; /* prokind from pg_proc row */ + + TupleDesc rettupdesc; /* result tuple descriptor */ + + List *plansource_list; /* CachedPlanSources for fn's queries */ + + /* if positive, this is the index of the query we're parsing */ + int error_query_index; + + MemoryContext hcontext; /* memory context holding all above */ +} SQLFunctionHashEntry; + +typedef struct SQLFunctionCache +{ + SQLFunctionHashEntry *func; /* associated SQLFunctionHashEntry */ + + bool lazyEvalOK; /* true if lazyEval is safe */ + bool shutdown_reg; /* true if registered shutdown callback */ bool lazyEval; /* true if using lazyEval for result query */ ParamListInfo paramLI; /* Param list representing current args */ @@ -112,23 +149,40 @@ typedef struct JunkFilter *junkFilter; /* will be NULL if function returns VOID */ + /* if positive, this is the index of the query we're executing */ + int error_query_index; + /* - * func_state is a List of execution_state records, each of which is the - * first for its original parsetree, with any additional records chained - * to it via the "next" fields. This sublist structure is needed to keep - * track of where the original query boundaries are. + * While executing a particular query within the function, cplan is the + * CachedPlan we've obtained for that query, and eslist is a list of + * execution_state records for the individual plans within the CachedPlan. + * next_query_index is the 0-based index of the next CachedPlanSource to + * get a CachedPlan from. */ - List *func_state; + CachedPlan *cplan; /* Plan for current query, if any */ + ResourceOwner cowner; /* CachedPlan is registered with this owner */ + execution_state *eslist; /* execution_state records */ + int next_query_index; /* index of next CachedPlanSource to run */ MemoryContext fcontext; /* memory context holding this struct and all * subsidiary data */ - - LocalTransactionId lxid; /* lxid in which cache was made */ - SubTransactionId subxid; /* subxid in which cache was made */ } SQLFunctionCache; typedef SQLFunctionCache *SQLFunctionCachePtr; +/* Struct pointed to by FmgrInfo.fn_extra for a SQL function */ +typedef struct SQLFunctionLink +{ + /* Permanent pointer to associated SQLFunctionHashEntry */ + SQLFunctionHashEntry *func; + + /* Transient pointer to SQLFunctionCache, used only if returnsSet */ + SQLFunctionCache *fcache; + + /* Callback to release our use-count on the SQLFunctionHashEntry */ + MemoryContextCallback mcb; +} SQLFunctionLink; + /* non-export function prototypes */ static Node *sql_fn_param_ref(ParseState *pstate, ParamRef *pref); @@ -138,10 +192,10 @@ static Node *sql_fn_make_param(SQLFunctionParseInfoPtr pinfo, int paramno, int location); static Node *sql_fn_resolve_param_name(SQLFunctionParseInfoPtr pinfo, const char *paramname, int location); -static List *init_execution_state(List *queryTree_list, - SQLFunctionCachePtr fcache, - bool lazyEvalOK); -static void init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK); +static bool init_execution_state(SQLFunctionCachePtr fcache); +static void sql_postrewrite_callback(List *querytree_list, void *arg); +static SQLFunctionCache *init_sql_fcache(FunctionCallInfo fcinfo, + bool lazyEvalOK); static void postquel_start(execution_state *es, SQLFunctionCachePtr fcache); static bool postquel_getnext(execution_state *es, SQLFunctionCachePtr fcache); static void postquel_end(execution_state *es); @@ -151,8 +205,10 @@ static Datum postquel_get_single_result(TupleTableSlot *slot, FunctionCallInfo fcinfo, SQLFunctionCachePtr fcache, MemoryContext resultcontext); +static void sql_compile_error_callback(void *arg); static void sql_exec_error_callback(void *arg); static void ShutdownSQLFunction(Datum arg); +static void RemoveSQLFunctionLink(void *arg); static void check_sql_fn_statement(List *queryTreeList); static bool check_sql_stmt_retval(List *queryTreeList, Oid rettype, TupleDesc rettupdesc, @@ -460,99 +516,172 @@ sql_fn_resolve_param_name(SQLFunctionParseInfoPtr pinfo, } /* - * Set up the per-query execution_state records for a SQL function. + * Set up the per-query execution_state records for the next query within + * the SQL function. * - * The input is a List of Lists of parsed and rewritten, but not planned, - * querytrees. The sublist structure denotes the original query boundaries. + * Returns true if successful, false if there are no more queries. */ -static List * -init_execution_state(List *queryTree_list, - SQLFunctionCachePtr fcache, - bool lazyEvalOK) +static bool +init_execution_state(SQLFunctionCachePtr fcache) { - List *eslist = NIL; + CachedPlanSource *plansource; + execution_state *preves = NULL; execution_state *lasttages = NULL; - ListCell *lc1; + ListCell *lc; - foreach(lc1, queryTree_list) + /* + * Clean up after previous query, if there was one. Note that we just + * leak the old execution_state records until end of function execution; + * there aren't likely to be enough of them to matter. + */ + if (fcache->cplan) { - List *qtlist = lfirst_node(List, lc1); - execution_state *firstes = NULL; - execution_state *preves = NULL; - ListCell *lc2; + ReleaseCachedPlan(fcache->cplan, fcache->cowner); + fcache->cplan = NULL; + } + fcache->eslist = NULL; - foreach(lc2, qtlist) - { - Query *queryTree = lfirst_node(Query, lc2); - PlannedStmt *stmt; - execution_state *newes; + /* + * Get the next CachedPlanSource, or stop if there are no more. + */ + if (fcache->next_query_index >= list_length(fcache->func->plansource_list)) + return false; + plansource = (CachedPlanSource *) list_nth(fcache->func->plansource_list, + fcache->next_query_index); + fcache->next_query_index++; - /* Plan the query if needed */ - if (queryTree->commandType == CMD_UTILITY) - { - /* Utility commands require no planning. */ - stmt = makeNode(PlannedStmt); - stmt->commandType = CMD_UTILITY; - stmt->canSetTag = queryTree->canSetTag; - stmt->utilityStmt = queryTree->utilityStmt; - stmt->stmt_location = queryTree->stmt_location; - stmt->stmt_len = queryTree->stmt_len; - stmt->queryId = queryTree->queryId; - } - else - stmt = pg_plan_query(queryTree, - fcache->src, - CURSOR_OPT_PARALLEL_OK, - NULL); + /* Count source queries for sql_exec_error_callback */ + fcache->error_query_index++; - /* - * Precheck all commands for validity in a function. This should - * generally match the restrictions spi.c applies. - */ - if (stmt->commandType == CMD_UTILITY) - { - if (IsA(stmt->utilityStmt, CopyStmt) && - ((CopyStmt *) stmt->utilityStmt)->filename == NULL) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot COPY to/from client in an SQL function"))); + /* + * Generate plans for the query or queries within this CachedPlanSource. + * Register the CachedPlan with the current resource owner. (Saving + * cowner here is mostly paranoia, but this way we needn't assume that + * CurrentResourceOwner will be the same when ShutdownSQLFunction runs.) + */ + fcache->cowner = CurrentResourceOwner; + fcache->cplan = GetCachedPlan(plansource, + fcache->paramLI, + fcache->cowner, + NULL); - if (IsA(stmt->utilityStmt, TransactionStmt)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - /* translator: %s is a SQL statement name */ - errmsg("%s is not allowed in an SQL function", - CreateCommandName(stmt->utilityStmt)))); - } + /* + * Build execution_state list to match the number of contained plans. + */ + foreach(lc, fcache->cplan->stmt_list) + { + PlannedStmt *stmt = lfirst_node(PlannedStmt, lc); + execution_state *newes; + + /* + * Precheck all commands for validity in a function. This should + * generally match the restrictions spi.c applies. + */ + if (stmt->commandType == CMD_UTILITY) + { + if (IsA(stmt->utilityStmt, CopyStmt) && + ((CopyStmt *) stmt->utilityStmt)->filename == NULL) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot COPY to/from client in an SQL function"))); - if (fcache->readonly_func && !CommandIsReadOnly(stmt)) + if (IsA(stmt->utilityStmt, TransactionStmt)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), /* translator: %s is a SQL statement name */ - errmsg("%s is not allowed in a non-volatile function", - CreateCommandName((Node *) stmt)))); + errmsg("%s is not allowed in an SQL function", + CreateCommandName(stmt->utilityStmt)))); + } - /* OK, build the execution_state for this query */ - newes = (execution_state *) palloc(sizeof(execution_state)); - if (preves) - preves->next = newes; - else - firstes = newes; + if (fcache->func->readonly_func && !CommandIsReadOnly(stmt)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + /* translator: %s is a SQL statement name */ + errmsg("%s is not allowed in a non-volatile function", + CreateCommandName((Node *) stmt)))); + + /* OK, build the execution_state for this query */ + newes = (execution_state *) palloc(sizeof(execution_state)); + if (preves) + preves->next = newes; + else + fcache->eslist = newes; - newes->next = NULL; - newes->status = F_EXEC_START; - newes->setsResult = false; /* might change below */ - newes->lazyEval = false; /* might change below */ - newes->stmt = stmt; - newes->qd = NULL; + newes->next = NULL; + newes->status = F_EXEC_START; + newes->setsResult = false; /* might change below */ + newes->lazyEval = false; /* might change below */ + newes->stmt = stmt; + newes->qd = NULL; - if (queryTree->canSetTag) - lasttages = newes; + if (stmt->canSetTag) + lasttages = newes; - preves = newes; - } + preves = newes; + } + + /* + * If this isn't the last CachedPlanSource, we're done here. Otherwise, + * we need to prepare information about how to return the results. + */ + if (fcache->next_query_index < list_length(fcache->func->plansource_list)) + return true; + + /* + * Construct a JunkFilter we can use to coerce the returned rowtype to the + * desired form, unless the result type is VOID, in which case there's + * nothing to coerce to. (XXX Frequently, the JunkFilter isn't doing + * anything very interesting, but much of this module expects it to be + * there anyway.) + */ + if (fcache->func->rettype != VOIDOID) + { + TupleTableSlot *slot = MakeSingleTupleTableSlot(NULL, + &TTSOpsMinimalTuple); + List *resulttlist; + + /* + * Re-fetch the (possibly modified) output tlist of the final + * statement. By this point, we should have thrown an error if there + * is not one. + */ + resulttlist = get_sql_fn_result_tlist(plansource->query_list); - eslist = lappend(eslist, firstes); + /* + * We need to make a copy to ensure that it doesn't disappear + * underneath us due to plancache invalidation. + */ + resulttlist = copyObject(resulttlist); + + /* + * If the result is composite, *and* we are returning the whole tuple + * result, we need to insert nulls for any dropped columns. In the + * single-column-result case, there might be dropped columns within + * the composite column value, but it's not our problem here. There + * should be no resjunk entries in resulttlist, so in the second case + * the JunkFilter is certainly a no-op. + */ + if (fcache->func->rettupdesc && fcache->func->returnsTuple) + fcache->junkFilter = ExecInitJunkFilterConversion(resulttlist, + fcache->func->rettupdesc, + slot); + else + fcache->junkFilter = ExecInitJunkFilter(resulttlist, slot); + } + + if (fcache->func->returnsTuple) + { + /* Make sure output rowtype is properly blessed */ + BlessTupleDesc(fcache->junkFilter->jf_resultSlot->tts_tupleDescriptor); + } + else if (fcache->func->returnsSet && type_is_rowtype(fcache->func->rettype)) + { + /* + * Returning rowtype as if it were scalar --- materialize won't work. + * Right now it's sufficient to override any caller preference for + * materialize mode, but this might need more work in future. + */ + fcache->lazyEvalOK = true; } /* @@ -572,68 +701,69 @@ init_execution_state(List *queryTree_list, if (lasttages && fcache->junkFilter) { lasttages->setsResult = true; - if (lazyEvalOK && + if (fcache->lazyEvalOK && lasttages->stmt->commandType == CMD_SELECT && !lasttages->stmt->hasModifyingCTE) fcache->lazyEval = lasttages->lazyEval = true; } - return eslist; + return true; } /* - * Initialize the SQLFunctionCache for a SQL function + * Fill a new SQLFunctionHashEntry. + * + * The passed-in "cfunc" struct is expected to be zeroes, except + * for the CachedFunction fields, which we don't touch here. + * + * We expect to be called in a short-lived memory context (typically a + * query's per-tuple context). Data that is to be part of the hash entry + * must be copied into the hcontext, or put into a CachedPlanSource. */ static void -init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK) +sql_compile_callback(FunctionCallInfo fcinfo, + HeapTuple procedureTuple, + const CachedFunctionHashKey *hashkey, + CachedFunction *cfunc, + bool forValidator) { - FmgrInfo *finfo = fcinfo->flinfo; - Oid foid = finfo->fn_oid; - MemoryContext fcontext; - MemoryContext oldcontext; + SQLFunctionHashEntry *func = (SQLFunctionHashEntry *) cfunc; + Form_pg_proc procedureStruct = (Form_pg_proc) GETSTRUCT(procedureTuple); + ErrorContextCallback comperrcontext; + MemoryContext hcontext; + MemoryContext oldcontext = CurrentMemoryContext; Oid rettype; TupleDesc rettupdesc; - HeapTuple procedureTuple; - Form_pg_proc procedureStruct; - SQLFunctionCachePtr fcache; - List *queryTree_list; - ListCell *lc; Datum tmp; bool isNull; + List *queryTree_list; + List *plansource_list; + ListCell *qlc; + ListCell *plc; /* - * Create memory context that holds all the SQLFunctionCache data. It - * must be a child of whatever context holds the FmgrInfo. - */ - fcontext = AllocSetContextCreate(finfo->fn_mcxt, - "SQL function", - ALLOCSET_DEFAULT_SIZES); - - oldcontext = MemoryContextSwitchTo(fcontext); - - /* - * Create the struct proper, link it to fcontext and fn_extra. Once this - * is done, we'll be able to recover the memory after failure, even if the - * FmgrInfo is long-lived. + * Setup error traceback support for ereport() during compile */ - fcache = (SQLFunctionCachePtr) palloc0(sizeof(SQLFunctionCache)); - fcache->fcontext = fcontext; - finfo->fn_extra = fcache; + comperrcontext.callback = sql_compile_error_callback; + comperrcontext.arg = func; + comperrcontext.previous = error_context_stack; + error_context_stack = &comperrcontext; /* - * get the procedure tuple corresponding to the given function Oid + * Create the hash entry's memory context. For now it's a child of the + * caller's context, so that it will go away if we fail partway through. */ - procedureTuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(foid)); - if (!HeapTupleIsValid(procedureTuple)) - elog(ERROR, "cache lookup failed for function %u", foid); - procedureStruct = (Form_pg_proc) GETSTRUCT(procedureTuple); + hcontext = AllocSetContextCreate(CurrentMemoryContext, + "SQL function", + ALLOCSET_SMALL_SIZES); /* * copy function name immediately for use by error reporting callback, and * for use as memory context identifier */ - fcache->fname = pstrdup(NameStr(procedureStruct->proname)); - MemoryContextSetIdentifier(fcontext, fcache->fname); + func->fname = MemoryContextStrdup(hcontext, + NameStr(procedureStruct->proname)); + MemoryContextSetIdentifier(hcontext, func->fname); /* * Resolve any polymorphism, obtaining the actual result type, and the @@ -641,32 +771,44 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK) */ (void) get_call_result_type(fcinfo, &rettype, &rettupdesc); - fcache->rettype = rettype; + func->rettype = rettype; + if (rettupdesc) + { + MemoryContextSwitchTo(hcontext); + func->rettupdesc = CreateTupleDescCopy(rettupdesc); + MemoryContextSwitchTo(oldcontext); + } /* Fetch the typlen and byval info for the result type */ - get_typlenbyval(rettype, &fcache->typlen, &fcache->typbyval); + get_typlenbyval(rettype, &func->typlen, &func->typbyval); /* Remember whether we're returning setof something */ - fcache->returnsSet = procedureStruct->proretset; + func->returnsSet = procedureStruct->proretset; /* Remember if function is STABLE/IMMUTABLE */ - fcache->readonly_func = + func->readonly_func = (procedureStruct->provolatile != PROVOLATILE_VOLATILE); + /* Remember routine kind */ + func->prokind = procedureStruct->prokind; + /* * We need the actual argument types to pass to the parser. Also make * sure that parameter symbols are considered to have the function's * resolved input collation. */ - fcache->pinfo = prepare_sql_fn_parse_info(procedureTuple, - finfo->fn_expr, - collation); + MemoryContextSwitchTo(hcontext); + func->pinfo = prepare_sql_fn_parse_info(procedureTuple, + fcinfo->flinfo->fn_expr, + PG_GET_COLLATION()); + MemoryContextSwitchTo(oldcontext); /* * And of course we need the function body text. */ tmp = SysCacheGetAttrNotNull(PROCOID, procedureTuple, Anum_pg_proc_prosrc); - fcache->src = TextDatumGetCString(tmp); + func->src = MemoryContextStrdup(hcontext, + TextDatumGetCString(tmp)); /* If we have prosqlbody, pay attention to that not prosrc. */ tmp = SysCacheGetAttr(PROCOID, @@ -675,19 +817,20 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK) &isNull); /* - * Parse and rewrite the queries in the function text. Use sublists to - * keep track of the original query boundaries. - * - * Note: since parsing and planning is done in fcontext, we will generate - * a lot of cruft that lives as long as the fcache does. This is annoying - * but we'll not worry about it until the module is rewritten to use - * plancache.c. + * Now we must parse and rewrite the function's queries, and create + * CachedPlanSources. Note that we apply CreateCachedPlan[ForQuery] + * immediately so that it captures the original state of the parsetrees, + * but we don't do CompleteCachedPlan until after fixing up the final + * query's targetlist. */ queryTree_list = NIL; + plansource_list = NIL; if (!isNull) { + /* Source queries are already parse-analyzed */ Node *n; List *stored_query_list; + ListCell *lc; n = stringToNode(TextDatumGetCString(tmp)); if (IsA(n, List)) @@ -698,8 +841,17 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK) foreach(lc, stored_query_list) { Query *parsetree = lfirst_node(Query, lc); + CachedPlanSource *plansource; List *queryTree_sublist; + /* Count source queries for sql_compile_error_callback */ + func->error_query_index++; + + plansource = CreateCachedPlanForQuery(parsetree, + func->src, + CreateCommandTag((Node *) parsetree)); + plansource_list = lappend(plansource_list, plansource); + AcquireRewriteLocks(parsetree, true, false); queryTree_sublist = pg_rewrite_query(parsetree); queryTree_list = lappend(queryTree_list, queryTree_sublist); @@ -707,24 +859,38 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK) } else { + /* Source queries are raw parsetrees */ List *raw_parsetree_list; + ListCell *lc; - raw_parsetree_list = pg_parse_query(fcache->src); + raw_parsetree_list = pg_parse_query(func->src); foreach(lc, raw_parsetree_list) { RawStmt *parsetree = lfirst_node(RawStmt, lc); + CachedPlanSource *plansource; List *queryTree_sublist; + /* Count source queries for sql_compile_error_callback */ + func->error_query_index++; + + plansource = CreateCachedPlan(parsetree, + func->src, + CreateCommandTag(parsetree->stmt)); + plansource_list = lappend(plansource_list, plansource); + queryTree_sublist = pg_analyze_and_rewrite_withcb(parsetree, - fcache->src, + func->src, (ParserSetupHook) sql_fn_parser_setup, - fcache->pinfo, + func->pinfo, NULL); queryTree_list = lappend(queryTree_list, queryTree_sublist); } } + /* Failures below here are reported as "during startup" */ + func->error_query_index = 0; + /* * Check that there are no statements we don't want to allow. */ @@ -740,7 +906,7 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK) * ask it to insert nulls for dropped columns; the junkfilter handles * that.) * - * Note: we set fcache->returnsTuple according to whether we are returning + * Note: we set func->returnsTuple according to whether we are returning * the whole tuple result or just a single column. In the latter case we * clear returnsTuple because we need not act different from the scalar * result case, even if it's a rowtype column. (However, we have to force @@ -748,76 +914,244 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK) * the rowtype column into multiple columns, since we have no way to * notify the caller that it should do that.) */ - fcache->returnsTuple = check_sql_fn_retval(queryTree_list, - rettype, - rettupdesc, - procedureStruct->prokind, - false); + func->returnsTuple = check_sql_fn_retval(queryTree_list, + rettype, + rettupdesc, + procedureStruct->prokind, + false); /* - * Construct a JunkFilter we can use to coerce the returned rowtype to the - * desired form, unless the result type is VOID, in which case there's - * nothing to coerce to. (XXX Frequently, the JunkFilter isn't doing - * anything very interesting, but much of this module expects it to be - * there anyway.) + * Now that check_sql_fn_retval has done its thing, we can complete plan + * cache entry creation. */ - if (rettype != VOIDOID) + forboth(qlc, queryTree_list, plc, plansource_list) { - TupleTableSlot *slot = MakeSingleTupleTableSlot(NULL, - &TTSOpsMinimalTuple); - List *resulttlist; + List *queryTree_sublist = lfirst(qlc); + CachedPlanSource *plansource = lfirst(plc); + bool islast; + + /* Finish filling in the CachedPlanSource */ + CompleteCachedPlan(plansource, + queryTree_sublist, + NULL, + NULL, + 0, + (ParserSetupHook) sql_fn_parser_setup, + func->pinfo, + CURSOR_OPT_PARALLEL_OK | CURSOR_OPT_NO_SCROLL, + false); /* - * Re-fetch the (possibly modified) output tlist of the final - * statement. By this point, we should have thrown an error if there - * is not one. + * Install post-rewrite hook. Its arg is the hash entry if this is + * the last statement, else NULL. */ - resulttlist = get_sql_fn_result_tlist(llast_node(List, queryTree_list)); + islast = (lnext(queryTree_list, qlc) == NULL); + SetPostRewriteHook(plansource, + sql_postrewrite_callback, + islast ? func : NULL); + } - /* - * If the result is composite, *and* we are returning the whole tuple - * result, we need to insert nulls for any dropped columns. In the - * single-column-result case, there might be dropped columns within - * the composite column value, but it's not our problem here. There - * should be no resjunk entries in resulttlist, so in the second case - * the JunkFilter is certainly a no-op. - */ - if (rettupdesc && fcache->returnsTuple) - fcache->junkFilter = ExecInitJunkFilterConversion(resulttlist, - rettupdesc, - slot); - else - fcache->junkFilter = ExecInitJunkFilter(resulttlist, slot); + /* + * While the CachedPlanSources can take care of themselves, our List + * pointing to them had better be in the hcontext. + */ + MemoryContextSwitchTo(hcontext); + plansource_list = list_copy(plansource_list); + MemoryContextSwitchTo(oldcontext); + + /* + * We have now completed building the hash entry, so reparent stuff under + * CacheMemoryContext to make all the subsidiary data long-lived. + * Importantly, this part can't fail partway through. + */ + foreach(plc, plansource_list) + { + CachedPlanSource *plansource = lfirst(plc); + + SaveCachedPlan(plansource); + } + MemoryContextSetParent(hcontext, CacheMemoryContext); + + /* And finally, arm sql_delete_callback to delete the stuff again */ + func->plansource_list = plansource_list; + func->hcontext = hcontext; + + error_context_stack = comperrcontext.previous; +} + +/* Deletion callback used by funccache.c */ +static void +sql_delete_callback(CachedFunction *cfunc) +{ + SQLFunctionHashEntry *func = (SQLFunctionHashEntry *) cfunc; + ListCell *lc; + + /* Release the CachedPlanSources */ + foreach(lc, func->plansource_list) + { + CachedPlanSource *plansource = (CachedPlanSource *) lfirst(lc); + + DropCachedPlan(plansource); } + func->plansource_list = NIL; + + /* + * If we have an hcontext, free it, thereby getting rid of all subsidiary + * data. + */ + if (func->hcontext) + MemoryContextDelete(func->hcontext); + func->hcontext = NULL; +} + +/* Post-rewrite callback used by plancache.c */ +static void +sql_postrewrite_callback(List *querytree_list, void *arg) +{ + /* + * Check that there are no statements we don't want to allow. (Presently, + * there's no real point in this because the result can't change from what + * we saw originally. But it's cheap and maybe someday it will matter.) + */ + check_sql_fn_statement(querytree_list); - if (fcache->returnsTuple) + /* + * If this is the last query, we must re-do what check_sql_fn_retval did + * to its targetlist. Also check that returnsTuple didn't change (it + * probably cannot, but be cautious). + */ + if (arg != NULL) { - /* Make sure output rowtype is properly blessed */ - BlessTupleDesc(fcache->junkFilter->jf_resultSlot->tts_tupleDescriptor); + SQLFunctionHashEntry *func = (SQLFunctionHashEntry *) arg; + bool returnsTuple; + + returnsTuple = check_sql_stmt_retval(querytree_list, + func->rettype, + func->rettupdesc, + func->prokind, + false); + if (returnsTuple != func->returnsTuple) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cached plan must not change result type"))); + } +} + +/* + * Initialize the SQLFunctionCache for a SQL function + */ +static SQLFunctionCache * +init_sql_fcache(FunctionCallInfo fcinfo, bool lazyEvalOK) +{ + FmgrInfo *finfo = fcinfo->flinfo; + SQLFunctionHashEntry *func; + SQLFunctionCache *fcache; + SQLFunctionLink *flink; + MemoryContext pcontext; + MemoryContext fcontext; + MemoryContext oldcontext; + + /* + * If this is the first execution for this FmgrInfo, set up a link struct + * (initially containing null pointers). The link must live as long as + * the FmgrInfo, so it goes in fn_mcxt. Also set up a memory context + * callback that will be invoked when fn_mcxt is deleted. + */ + flink = finfo->fn_extra; + if (flink == NULL) + { + flink = (SQLFunctionLink *) + MemoryContextAllocZero(finfo->fn_mcxt, sizeof(SQLFunctionLink)); + flink->mcb.func = RemoveSQLFunctionLink; + flink->mcb.arg = flink; + MemoryContextRegisterResetCallback(finfo->fn_mcxt, &flink->mcb); + finfo->fn_extra = flink; } - else if (fcache->returnsSet && type_is_rowtype(fcache->rettype)) + + /* + * If we are resuming execution of a set-returning function, just keep + * using the same cache. We do not ask funccache.c to re-validate the + * SQLFunctionHashEntry: we want to run to completion using the function's + * initial definition. + */ + if (flink->fcache != NULL) { - /* - * Returning rowtype as if it were scalar --- materialize won't work. - * Right now it's sufficient to override any caller preference for - * materialize mode, but to add more smarts in init_execution_state - * about this, we'd probably need a three-way flag instead of bool. - */ - lazyEvalOK = true; + Assert(flink->fcache->func == flink->func); + return flink->fcache; + } + + /* + * Look up, or re-validate, the long-lived hash entry. Make the hash key + * depend on the result of get_call_result_type() when that's composite, + * so that we can safely assume that we'll build a new hash entry if the + * composite rowtype changes. + */ + func = (SQLFunctionHashEntry *) + cached_function_compile(fcinfo, + (CachedFunction *) flink->func, + sql_compile_callback, + sql_delete_callback, + sizeof(SQLFunctionHashEntry), + true, + false); + + /* + * Install the hash pointer in the SQLFunctionLink, and increment its use + * count to reflect that. If cached_function_compile gave us back a + * different hash entry than we were using before, we must decrement that + * one's use count. + */ + if (func != flink->func) + { + if (flink->func != NULL) + { + Assert(flink->func->cfunc.use_count > 0); + flink->func->cfunc.use_count--; + } + flink->func = func; + func->cfunc.use_count++; } - /* Finally, plan the queries */ - fcache->func_state = init_execution_state(queryTree_list, - fcache, - lazyEvalOK); + /* + * Create memory context that holds all the SQLFunctionCache data. If we + * return a set, we must keep this in whatever context holds the FmgrInfo + * (anything shorter-lived risks leaving a dangling pointer in flink). But + * in a non-SRF we'll delete it before returning, and there's no need for + * it to outlive the caller's context. + */ + pcontext = func->returnsSet ? finfo->fn_mcxt : CurrentMemoryContext; + fcontext = AllocSetContextCreate(pcontext, + "SQL function execution", + ALLOCSET_DEFAULT_SIZES); + + oldcontext = MemoryContextSwitchTo(fcontext); - /* Mark fcache with time of creation to show it's valid */ - fcache->lxid = MyProc->vxid.lxid; - fcache->subxid = GetCurrentSubTransactionId(); + /* + * Create the struct proper, link it to func and fcontext. + */ + fcache = (SQLFunctionCache *) palloc0(sizeof(SQLFunctionCache)); + fcache->func = func; + fcache->fcontext = fcontext; + fcache->lazyEvalOK = lazyEvalOK; - ReleaseSysCache(procedureTuple); + /* + * If we return a set, we must link the fcache into fn_extra so that we + * can find it again during future calls. But in a non-SRF there is no + * need to link it into fn_extra at all. Not doing so removes the risk of + * having a dangling pointer in a long-lived FmgrInfo. + */ + if (func->returnsSet) + flink->fcache = fcache; + + /* + * We're beginning a new execution of the function, so convert params to + * appropriate format. + */ + postquel_sub_params(fcache, fcinfo); MemoryContextSwitchTo(oldcontext); + + return fcache; } /* Start up execution of one execution_state node */ @@ -852,7 +1186,7 @@ postquel_start(execution_state *es, SQLFunctionCachePtr fcache) es->qd = CreateQueryDesc(es->stmt, NULL, - fcache->src, + fcache->func->src, GetActiveSnapshot(), InvalidSnapshot, dest, @@ -893,7 +1227,7 @@ postquel_getnext(execution_state *es, SQLFunctionCachePtr fcache) if (es->qd->operation == CMD_UTILITY) { ProcessUtility(es->qd->plannedstmt, - fcache->src, + fcache->func->src, true, /* protect function cache's parsetree */ PROCESS_UTILITY_QUERY, es->qd->params, @@ -949,7 +1283,7 @@ postquel_sub_params(SQLFunctionCachePtr fcache, if (nargs > 0) { ParamListInfo paramLI; - Oid *argtypes = fcache->pinfo->argtypes; + Oid *argtypes = fcache->func->pinfo->argtypes; if (fcache->paramLI == NULL) { @@ -982,7 +1316,8 @@ postquel_sub_params(SQLFunctionCachePtr fcache, prm->value = MakeExpandedObjectReadOnly(fcinfo->args[i].value, prm->isnull, get_typlen(argtypes[i])); - prm->pflags = 0; + /* Allow the value to be substituted into custom plans */ + prm->pflags = PARAM_FLAG_CONST; prm->ptype = argtypes[i]; } } @@ -1012,7 +1347,7 @@ postquel_get_single_result(TupleTableSlot *slot, */ oldcontext = MemoryContextSwitchTo(resultcontext); - if (fcache->returnsTuple) + if (fcache->func->returnsTuple) { /* We must return the whole tuple as a Datum. */ fcinfo->isnull = false; @@ -1027,7 +1362,7 @@ postquel_get_single_result(TupleTableSlot *slot, value = slot_getattr(slot, 1, &(fcinfo->isnull)); if (!fcinfo->isnull) - value = datumCopy(value, fcache->typbyval, fcache->typlen); + value = datumCopy(value, fcache->func->typbyval, fcache->func->typlen); } MemoryContextSwitchTo(oldcontext); @@ -1042,25 +1377,16 @@ Datum fmgr_sql(PG_FUNCTION_ARGS) { SQLFunctionCachePtr fcache; + SQLFunctionLink *flink; ErrorContextCallback sqlerrcontext; + MemoryContext tscontext; MemoryContext oldcontext; bool randomAccess; bool lazyEvalOK; - bool is_first; bool pushed_snapshot; execution_state *es; TupleTableSlot *slot; Datum result; - List *eslist; - ListCell *eslc; - - /* - * Setup error traceback support for ereport() - */ - sqlerrcontext.callback = sql_exec_error_callback; - sqlerrcontext.arg = fcinfo->flinfo; - sqlerrcontext.previous = error_context_stack; - error_context_stack = &sqlerrcontext; /* Check call context */ if (fcinfo->flinfo->fn_retset) @@ -1081,80 +1407,63 @@ fmgr_sql(PG_FUNCTION_ARGS) errmsg("set-valued function called in context that cannot accept a set"))); randomAccess = rsi->allowedModes & SFRM_Materialize_Random; lazyEvalOK = !(rsi->allowedModes & SFRM_Materialize_Preferred); + /* tuplestore must have query lifespan */ + tscontext = rsi->econtext->ecxt_per_query_memory; } else { randomAccess = false; lazyEvalOK = true; + /* tuplestore needn't outlive caller context */ + tscontext = CurrentMemoryContext; } /* - * Initialize fcache (build plans) if first time through; or re-initialize - * if the cache is stale. + * Initialize fcache if starting a fresh execution. */ - fcache = (SQLFunctionCachePtr) fcinfo->flinfo->fn_extra; + fcache = init_sql_fcache(fcinfo, lazyEvalOK); + /* init_sql_fcache also ensures we have a SQLFunctionLink */ + flink = fcinfo->flinfo->fn_extra; - if (fcache != NULL) - { - if (fcache->lxid != MyProc->vxid.lxid || - !SubTransactionIsActive(fcache->subxid)) - { - /* It's stale; unlink and delete */ - fcinfo->flinfo->fn_extra = NULL; - MemoryContextDelete(fcache->fcontext); - fcache = NULL; - } - } + /* + * Now we can set up error traceback support for ereport() + */ + sqlerrcontext.callback = sql_exec_error_callback; + sqlerrcontext.arg = fcache; + sqlerrcontext.previous = error_context_stack; + error_context_stack = &sqlerrcontext; - if (fcache == NULL) - { - init_sql_fcache(fcinfo, PG_GET_COLLATION(), lazyEvalOK); - fcache = (SQLFunctionCachePtr) fcinfo->flinfo->fn_extra; - } + /* + * Build tuplestore to hold results, if we don't have one already. Make + * sure it's in a suitable context. + */ + oldcontext = MemoryContextSwitchTo(tscontext); + + if (!fcache->tstore) + fcache->tstore = tuplestore_begin_heap(randomAccess, false, work_mem); /* - * Switch to context in which the fcache lives. This ensures that our - * tuplestore etc will have sufficient lifetime. The sub-executor is + * Switch to context in which the fcache lives. The sub-executor is * responsible for deleting per-tuple information. (XXX in the case of a - * long-lived FmgrInfo, this policy represents more memory leakage, but - * it's not entirely clear where to keep stuff instead.) + * long-lived FmgrInfo, this policy potentially causes memory leakage, but + * it's not very clear where we could keep stuff instead. Fortunately, + * there are few if any cases where set-returning functions are invoked + * via FmgrInfos that would outlive the calling query.) */ - oldcontext = MemoryContextSwitchTo(fcache->fcontext); + MemoryContextSwitchTo(fcache->fcontext); /* - * Find first unfinished query in function, and note whether it's the - * first query. + * Find first unfinished execution_state. If none, advance to the next + * query in function. */ - eslist = fcache->func_state; - es = NULL; - is_first = true; - foreach(eslc, eslist) + do { - es = (execution_state *) lfirst(eslc); - + es = fcache->eslist; while (es && es->status == F_EXEC_DONE) - { - is_first = false; es = es->next; - } - if (es) break; - } - - /* - * Convert params to appropriate format if starting a fresh execution. (If - * continuing execution, we can re-use prior params.) - */ - if (is_first && es && es->status == F_EXEC_START) - postquel_sub_params(fcache, fcinfo); - - /* - * Build tuplestore to hold results, if we don't have one already. Note - * it's in the query-lifespan context. - */ - if (!fcache->tstore) - fcache->tstore = tuplestore_begin_heap(randomAccess, false, work_mem); + } while (init_execution_state(fcache)); /* * Execute each command in the function one after another until we either @@ -1187,7 +1496,7 @@ fmgr_sql(PG_FUNCTION_ARGS) * visible. Take a new snapshot if we don't have one yet, * otherwise just bump the command ID in the existing snapshot. */ - if (!fcache->readonly_func) + if (!fcache->func->readonly_func) { CommandCounterIncrement(); if (!pushed_snapshot) @@ -1201,7 +1510,7 @@ fmgr_sql(PG_FUNCTION_ARGS) postquel_start(es, fcache); } - else if (!fcache->readonly_func && !pushed_snapshot) + else if (!fcache->func->readonly_func && !pushed_snapshot) { /* Re-establish active snapshot when re-entering function */ PushActiveSnapshot(es->qd->snapshot); @@ -1218,7 +1527,7 @@ fmgr_sql(PG_FUNCTION_ARGS) * set, we can shut it down anyway because it must be a SELECT and we * don't care about fetching any more result rows. */ - if (completed || !fcache->returnsSet) + if (completed || !fcache->func->returnsSet) postquel_end(es); /* @@ -1234,17 +1543,11 @@ fmgr_sql(PG_FUNCTION_ARGS) break; /* - * Advance to next execution_state, which might be in the next list. + * Advance to next execution_state, and perhaps next query. */ es = es->next; while (!es) { - eslc = lnext(eslist, eslc); - if (!eslc) - break; /* end of function */ - - es = (execution_state *) lfirst(eslc); - /* * Flush the current snapshot so that we will take a new one for * the new query list. This ensures that new snaps are taken at @@ -1256,13 +1559,18 @@ fmgr_sql(PG_FUNCTION_ARGS) PopActiveSnapshot(); pushed_snapshot = false; } + + if (!init_execution_state(fcache)) + break; /* end of function */ + + es = fcache->eslist; } } /* * The tuplestore now contains whatever row(s) we are supposed to return. */ - if (fcache->returnsSet) + if (fcache->func->returnsSet) { ReturnSetInfo *rsi = (ReturnSetInfo *) fcinfo->resultinfo; @@ -1298,7 +1606,7 @@ fmgr_sql(PG_FUNCTION_ARGS) { RegisterExprContextCallback(rsi->econtext, ShutdownSQLFunction, - PointerGetDatum(fcache)); + PointerGetDatum(flink)); fcache->shutdown_reg = true; } } @@ -1322,7 +1630,7 @@ fmgr_sql(PG_FUNCTION_ARGS) { UnregisterExprContextCallback(rsi->econtext, ShutdownSQLFunction, - PointerGetDatum(fcache)); + PointerGetDatum(flink)); fcache->shutdown_reg = false; } } @@ -1338,7 +1646,12 @@ fmgr_sql(PG_FUNCTION_ARGS) fcache->tstore = NULL; /* must copy desc because execSRF.c will free it */ if (fcache->junkFilter) + { + /* setDesc must be allocated in suitable context */ + MemoryContextSwitchTo(tscontext); rsi->setDesc = CreateTupleDescCopy(fcache->junkFilter->jf_cleanTupType); + MemoryContextSwitchTo(fcache->fcontext); + } fcinfo->isnull = true; result = (Datum) 0; @@ -1348,7 +1661,7 @@ fmgr_sql(PG_FUNCTION_ARGS) { UnregisterExprContextCallback(rsi->econtext, ShutdownSQLFunction, - PointerGetDatum(fcache)); + PointerGetDatum(flink)); fcache->shutdown_reg = false; } } @@ -1374,7 +1687,7 @@ fmgr_sql(PG_FUNCTION_ARGS) else { /* Should only get here for VOID functions and procedures */ - Assert(fcache->rettype == VOIDOID); + Assert(fcache->func->rettype == VOIDOID); fcinfo->isnull = true; result = (Datum) 0; } @@ -1387,154 +1700,171 @@ fmgr_sql(PG_FUNCTION_ARGS) if (pushed_snapshot) PopActiveSnapshot(); + MemoryContextSwitchTo(oldcontext); + /* - * If we've gone through every command in the function, we are done. Reset - * the execution states to start over again on next call. + * If we've gone through every command in the function, we are done. + * Release the cache to start over again on next call. */ if (es == NULL) { - foreach(eslc, fcache->func_state) - { - es = (execution_state *) lfirst(eslc); - while (es) - { - es->status = F_EXEC_START; - es = es->next; - } - } + if (fcache->tstore) + tuplestore_end(fcache->tstore); + Assert(fcache->cplan == NULL); + flink->fcache = NULL; + MemoryContextDelete(fcache->fcontext); } error_context_stack = sqlerrcontext.previous; - MemoryContextSwitchTo(oldcontext); - return result; } /* - * error context callback to let us supply a call-stack traceback + * error context callback to let us supply a traceback during compile */ static void -sql_exec_error_callback(void *arg) +sql_compile_error_callback(void *arg) { - FmgrInfo *flinfo = (FmgrInfo *) arg; - SQLFunctionCachePtr fcache = (SQLFunctionCachePtr) flinfo->fn_extra; + SQLFunctionHashEntry *func = (SQLFunctionHashEntry *) arg; int syntaxerrposition; /* - * We can do nothing useful if init_sql_fcache() didn't get as far as - * saving the function name + * We can do nothing useful if sql_compile_callback() didn't get as far as + * copying the function name */ - if (fcache == NULL || fcache->fname == NULL) + if (func->fname == NULL) return; /* * If there is a syntax error position, convert to internal syntax error */ syntaxerrposition = geterrposition(); - if (syntaxerrposition > 0 && fcache->src != NULL) + if (syntaxerrposition > 0 && func->src != NULL) { errposition(0); internalerrposition(syntaxerrposition); - internalerrquery(fcache->src); + internalerrquery(func->src); } /* - * Try to determine where in the function we failed. If there is a query - * with non-null QueryDesc, finger it. (We check this rather than looking - * for F_EXEC_RUN state, so that errors during ExecutorStart or - * ExecutorEnd are blamed on the appropriate query; see postquel_start and - * postquel_end.) + * If we failed while parsing an identifiable query within the function, + * report that. Otherwise say it was "during startup". */ - if (fcache->func_state) - { - execution_state *es; - int query_num; - ListCell *lc; - - es = NULL; - query_num = 1; - foreach(lc, fcache->func_state) - { - es = (execution_state *) lfirst(lc); - while (es) - { - if (es->qd) - { - errcontext("SQL function \"%s\" statement %d", - fcache->fname, query_num); - break; - } - es = es->next; - } - if (es) - break; - query_num++; - } - if (es == NULL) - { - /* - * couldn't identify a running query; might be function entry, - * function exit, or between queries. - */ - errcontext("SQL function \"%s\"", fcache->fname); - } - } + if (func->error_query_index > 0) + errcontext("SQL function \"%s\" statement %d", + func->fname, func->error_query_index); else + errcontext("SQL function \"%s\" during startup", func->fname); +} + +/* + * error context callback to let us supply a call-stack traceback at runtime + */ +static void +sql_exec_error_callback(void *arg) +{ + SQLFunctionCachePtr fcache = (SQLFunctionCachePtr) arg; + int syntaxerrposition; + + /* + * If there is a syntax error position, convert to internal syntax error + */ + syntaxerrposition = geterrposition(); + if (syntaxerrposition > 0 && fcache->func->src != NULL) { - /* - * Assume we failed during init_sql_fcache(). (It's possible that the - * function actually has an empty body, but in that case we may as - * well report all errors as being "during startup".) - */ - errcontext("SQL function \"%s\" during startup", fcache->fname); + errposition(0); + internalerrposition(syntaxerrposition); + internalerrquery(fcache->func->src); } + + /* + * If we failed while executing an identifiable query within the function, + * report that. Otherwise say it was "during startup". + */ + if (fcache->error_query_index > 0) + errcontext("SQL function \"%s\" statement %d", + fcache->func->fname, fcache->error_query_index); + else + errcontext("SQL function \"%s\" during startup", fcache->func->fname); } /* - * callback function in case a function-returning-set needs to be shut down - * before it has been run to completion + * ExprContext callback function + * + * We register this in the active ExprContext while a set-returning SQL + * function is running, in case the function needs to be shut down before it + * has been run to completion. Note that this will not be called during an + * error abort, but we don't need it because transaction abort will take care + * of releasing executor resources. */ static void ShutdownSQLFunction(Datum arg) { - SQLFunctionCachePtr fcache = (SQLFunctionCachePtr) DatumGetPointer(arg); - execution_state *es; - ListCell *lc; + SQLFunctionLink *flink = (SQLFunctionLink *) DatumGetPointer(arg); + SQLFunctionCachePtr fcache = flink->fcache; - foreach(lc, fcache->func_state) + if (fcache != NULL) { - es = (execution_state *) lfirst(lc); + execution_state *es; + + /* Make sure we don't somehow try to do this twice */ + flink->fcache = NULL; + + es = fcache->eslist; while (es) { /* Shut down anything still running */ if (es->status == F_EXEC_RUN) { /* Re-establish active snapshot for any called functions */ - if (!fcache->readonly_func) + if (!fcache->func->readonly_func) PushActiveSnapshot(es->qd->snapshot); postquel_end(es); - if (!fcache->readonly_func) + if (!fcache->func->readonly_func) PopActiveSnapshot(); } - - /* Reset states to START in case we're called again */ - es->status = F_EXEC_START; es = es->next; } - } - /* Release tuplestore if we have one */ - if (fcache->tstore) - tuplestore_end(fcache->tstore); - fcache->tstore = NULL; + /* Release tuplestore if we have one */ + if (fcache->tstore) + tuplestore_end(fcache->tstore); + /* Release CachedPlan if we have one */ + if (fcache->cplan) + ReleaseCachedPlan(fcache->cplan, fcache->cowner); + + /* Release the cache */ + MemoryContextDelete(fcache->fcontext); + } /* execUtils will deregister the callback... */ - fcache->shutdown_reg = false; +} + +/* + * MemoryContext callback function + * + * We register this in the memory context that contains a SQLFunctionLink + * struct. When the memory context is reset or deleted, we release the + * reference count (if any) that the link holds on the long-lived hash entry. + * Note that this will happen even during error aborts. + */ +static void +RemoveSQLFunctionLink(void *arg) +{ + SQLFunctionLink *flink = (SQLFunctionLink *) arg; + + if (flink->func != NULL) + { + Assert(flink->func->cfunc.use_count > 0); + flink->func->cfunc.use_count--; + /* This should be unnecessary, but let's just be sure: */ + flink->func = NULL; + } } /* diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out index d5388a1fecf..72bae1bf254 100644 --- a/src/test/modules/test_extensions/expected/test_extensions.out +++ b/src/test/modules/test_extensions/expected/test_extensions.out @@ -651,7 +651,7 @@ 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" during startup +CONTEXT: SQL function "dep_req3b" statement 1 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/create_function_sql.out b/src/test/regress/expected/create_function_sql.out index 50aca5940ff..70ed5742b65 100644 --- a/src/test/regress/expected/create_function_sql.out +++ b/src/test/regress/expected/create_function_sql.out @@ -563,6 +563,20 @@ CREATE OR REPLACE PROCEDURE functest1(a int) LANGUAGE SQL AS 'SELECT $1'; ERROR: cannot change routine kind DETAIL: "functest1" is a function. DROP FUNCTION functest1(a int); +-- early shutdown of set-returning functions +CREATE FUNCTION functest_srf0() RETURNS SETOF int +LANGUAGE SQL +AS $$ SELECT i FROM generate_series(1, 100) i $$; +SELECT functest_srf0() LIMIT 5; + functest_srf0 +--------------- + 1 + 2 + 3 + 4 + 5 +(5 rows) + -- inlining of set-returning functions CREATE TABLE functest3 (a int); INSERT INTO functest3 VALUES (1), (2), (3); @@ -708,7 +722,7 @@ CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL ERROR: only one AS item needed for language "sql" -- Cleanup DROP SCHEMA temp_func_test CASCADE; -NOTICE: drop cascades to 30 other objects +NOTICE: drop cascades to 31 other objects DETAIL: drop cascades to function functest_a_1(text,date) drop cascades to function functest_a_2(text[]) drop cascades to function functest_a_3() @@ -732,6 +746,7 @@ drop cascades to function functest_s_10(text,date) drop cascades to function functest_s_13() drop cascades to function functest_s_15(integer) drop cascades to function functest_b_2(bigint) +drop cascades to function functest_srf0() drop cascades to function functest_sri1() drop cascades to function voidtest1(integer) drop cascades to function voidtest2(integer,integer) diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 8f2c8319172..1c4e37d2249 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -4723,12 +4723,8 @@ select rls_f(c) from test_t order by rls_f; -- should lead to RLS error during query rewrite set role regress_rls_alice; select rls_f(c) from test_t order by rls_f; - rls_f -------- - boffa - -(2 rows) - +ERROR: query would be affected by row-level security policy for table "rls_t" +CONTEXT: SQL function "rls_f" statement 1 reset role; set plan_cache_mode to force_generic_plan; -- Table owner bypasses RLS, although cached plan will be invalidated @@ -4743,12 +4739,8 @@ select rls_f(c) from test_t order by rls_f; -- should lead to plan invalidation and RLS error during query rewrite set role regress_rls_alice; select rls_f(c) from test_t order by rls_f; - rls_f -------- - boffa - -(2 rows) - +ERROR: query would be affected by row-level security policy for table "rls_t" +CONTEXT: SQL function "rls_f" statement 1 reset role; reset plan_cache_mode; reset rls_test.blah; diff --git a/src/test/regress/sql/create_function_sql.sql b/src/test/regress/sql/create_function_sql.sql index 89e9af3a499..1dd3c4a4e5f 100644 --- a/src/test/regress/sql/create_function_sql.sql +++ b/src/test/regress/sql/create_function_sql.sql @@ -328,6 +328,15 @@ CREATE OR REPLACE PROCEDURE functest1(a int) LANGUAGE SQL AS 'SELECT $1'; DROP FUNCTION functest1(a int); +-- early shutdown of set-returning functions + +CREATE FUNCTION functest_srf0() RETURNS SETOF int +LANGUAGE SQL +AS $$ SELECT i FROM generate_series(1, 100) i $$; + +SELECT functest_srf0() LIMIT 5; + + -- inlining of set-returning functions CREATE TABLE functest3 (a int); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 144c4e9662c..2bbcb43055e 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2613,6 +2613,8 @@ SPPageDesc SQLDropObject SQLFunctionCache SQLFunctionCachePtr +SQLFunctionHashEntry +SQLFunctionLink SQLFunctionParseInfo SQLFunctionParseInfoPtr SQLValueFunction -- 2.43.5