Re: SQLFunctionCache and generic plans - Mailing list pgsql-hackers
| From | Tom Lane |
|---|---|
| Subject | Re: SQLFunctionCache and generic plans |
| Date | |
| Msg-id | 3344719.1741985546@sss.pgh.pa.us Whole thread Raw |
| In response to | Re: SQLFunctionCache and generic plans (Alexander Pyhalov <a.pyhalov@postgrespro.ru>) |
| Responses |
Re: SQLFunctionCache and generic plans
|
| List | pgsql-hackers |
I spent some time today going through the actual code in this patch.
I realized that there's no longer any point in 0001: the later patches
don't move or repeatedly-call that bit of code, so it can be left as-is.
What I think we could stand to split out, though, is the changes in
the plancache support. The new 0001 attached is just the plancache
and analyze.c changes. That could be committed separately, although
of course there's little point in pushing it till we're happy with
the rest.
In general, this patch series is paying far too little attention to
updating existing comments that it obsoletes or adding new ones
explaining what's going on. For example, the introductory comment
for struct SQLFunctionCache still says
* 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.
and I wonder how much of the para after that is still accurate either.
The new structs aren't adequately documented either IMO. We now have
about three different structs that have something to do with caches
by their names, but the reader is left to guess how they fit together.
Another example is that the header comment for init_execution_state
still describes an argument list it hasn't got anymore.
I tried to clean up the comment situation in the plancache in 0001,
but I've not done much of anything to functions.c.
I'm fairly confused why 0002 and 0003 are separate patches, and the
commit messages for them do nothing to clarify that. It seems like
you're expecting reviewers to review a very transitory state of
affairs in 0002, and it's not clear why. Maybe the code is fine
and you just need to explain the change sequence a bit more
in the commit messages. 0002 could stand to explain the point
of the new test cases, too, especially since one of them seems to
be demonstrating the fixing of a pre-existing bug.
Something is very wrong in 0004: it should not be breaking that
test case in test_extensions. It seems to me we should already
have the necessary infrastructure for that, in that the plan
ought to have a PlanInvalItem referencing public.dep_req2(),
and the ALTER SET SCHEMA that gets done on that function should
result in an invalidation. So it looks to me like that patch
has somehow rearranged things so we miss an invalidation.
I've not tried to figure out why. I'm also sad that 0004
doesn't appear to include any test cases showing it doing
something right: without that, why do it at all?
regards, tom lane
From f975519041cbd278005f1d4035d4fe53a80cb665 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 14 Mar 2025 15:54:56 -0400
Subject: [PATCH v8 1/4] Support cached plans that work from a parse-analyzed
Query.
Up to now, plancache.c dealt only with raw parse trees as the
starting point for a cached plan. However, we'd like to use
this infrastructure for SQL functions, and in the case of a
new-style SQL function we'll only have the stored querytree,
which corresponds to an analyzed-but-not-rewritten Query.
Fortunately, we can make plancache.c handle that scenario
with only minor modifications; the biggest change is in
RevalidateCachedQuery() where we will need to apply only
pg_rewrite_query not pg_analyze_and_rewrite.
This patch just installs the infrastructure; there's no
caller as yet.
Author: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/8216639.NyiUUSuA9g@aivenlaptop
---
src/backend/parser/analyze.c | 39 +++++++
src/backend/utils/cache/plancache.c | 158 +++++++++++++++++++++-------
src/include/parser/analyze.h | 1 +
src/include/utils/plancache.h | 23 +++-
4 files changed, 179 insertions(+), 42 deletions(-)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 76f58b3aca3..1f4d6adda52 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -591,6 +591,45 @@ analyze_requires_snapshot(RawStmt *parseTree)
return stmt_requires_parse_analysis(parseTree);
}
+/*
+ * query_requires_rewrite_plan()
+ * Returns true if rewriting or planning is non-trivial for this Query.
+ *
+ * This is much like stmt_requires_parse_analysis(), but applies one step
+ * further down the pipeline.
+ *
+ * We do not provide an equivalent of analyze_requires_snapshot(): callers
+ * can assume that any rewriting or planning activity needs a snapshot.
+ */
+bool
+query_requires_rewrite_plan(Query *query)
+{
+ bool result;
+
+ if (query->commandType != CMD_UTILITY)
+ {
+ /* All optimizable statements require rewriting/planning */
+ result = true;
+ }
+ else
+ {
+ /* This list should match stmt_requires_parse_analysis() */
+ switch (nodeTag(query->utilityStmt))
+ {
+ case T_DeclareCursorStmt:
+ case T_ExplainStmt:
+ case T_CreateTableAsStmt:
+ case T_CallStmt:
+ result = true;
+ break;
+ default:
+ result = false;
+ break;
+ }
+ }
+ return result;
+}
+
/*
* transformDeleteStmt -
* transforms a Delete Statement
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 6c2979d5c82..5983927a4c2 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -14,7 +14,7 @@
* Cache invalidation is driven off sinval events. Any CachedPlanSource
* that matches the event is marked invalid, as is its generic CachedPlan
* if it has one. When (and if) the next demand for a cached plan occurs,
- * parse analysis and rewrite is repeated to build a new valid query tree,
+ * parse analysis and/or rewrite is repeated to build a new valid query tree,
* and then planning is performed as normal. We also force re-analysis and
* re-planning if the active search_path is different from the previous time
* or, if RLS is involved, if the user changes or the RLS environment changes.
@@ -63,6 +63,7 @@
#include "nodes/nodeFuncs.h"
#include "optimizer/optimizer.h"
#include "parser/analyze.h"
+#include "rewrite/rewriteHandler.h"
#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/utility.h"
@@ -74,18 +75,6 @@
#include "utils/syscache.h"
-/*
- * We must skip "overhead" operations that involve database access when the
- * cached plan's subject statement is a transaction control command or one
- * that requires a snapshot not to be set yet (such as SET or LOCK). More
- * generally, statements that do not require parse analysis/rewrite/plan
- * activity never need to be revalidated, so we can treat them all like that.
- * For the convenience of postgres.c, treat empty statements that way too.
- */
-#define StmtPlanRequiresRevalidation(plansource) \
- ((plansource)->raw_parse_tree != NULL && \
- stmt_requires_parse_analysis((plansource)->raw_parse_tree))
-
/*
* This is the head of the backend's list of "saved" CachedPlanSources (i.e.,
* those that are in long-lived storage and are examined for sinval events).
@@ -100,6 +89,8 @@ static dlist_head saved_plan_list = DLIST_STATIC_INIT(saved_plan_list);
static dlist_head cached_expression_list = DLIST_STATIC_INIT(cached_expression_list);
static void ReleaseGenericPlan(CachedPlanSource *plansource);
+static bool StmtPlanRequiresRevalidation(CachedPlanSource *plansource);
+static bool BuildingPlanRequiresSnapshot(CachedPlanSource *plansource);
static List *RevalidateCachedQuery(CachedPlanSource *plansource,
QueryEnvironment *queryEnv,
bool release_generic);
@@ -166,7 +157,7 @@ InitPlanCache(void)
}
/*
- * CreateCachedPlan: initially create a plan cache entry.
+ * CreateCachedPlan: initially create a plan cache entry for a raw parse tree.
*
* Creation of a cached plan is divided into two steps, CreateCachedPlan and
* CompleteCachedPlan. CreateCachedPlan should be called after running the
@@ -220,6 +211,7 @@ CreateCachedPlan(RawStmt *raw_parse_tree,
plansource = (CachedPlanSource *) palloc0(sizeof(CachedPlanSource));
plansource->magic = CACHEDPLANSOURCE_MAGIC;
plansource->raw_parse_tree = copyObject(raw_parse_tree);
+ plansource->analyzed_parse_tree = NULL;
plansource->query_string = pstrdup(query_string);
MemoryContextSetIdentifier(source_context, plansource->query_string);
plansource->commandTag = commandTag;
@@ -255,6 +247,34 @@ CreateCachedPlan(RawStmt *raw_parse_tree,
return plansource;
}
+/*
+ * CreateCachedPlanForQuery: initially create a plan cache entry for a Query.
+ *
+ * This is used in the same way as CreateCachedPlan, except that the source
+ * query has already been through parse analysis, and the plancache will never
+ * try to re-do that step.
+ *
+ * Currently this is used only for new-style SQL functions, where we have a
+ * Query from the function's prosqlbody, but no source text. The query_string
+ * is typically empty, but is required anyway.
+ */
+CachedPlanSource *
+CreateCachedPlanForQuery(Query *analyzed_parse_tree,
+ const char *query_string,
+ CommandTag commandTag)
+{
+ CachedPlanSource *plansource;
+ MemoryContext oldcxt;
+
+ /* Rather than duplicating CreateCachedPlan, just do this: */
+ plansource = CreateCachedPlan(NULL, query_string, commandTag);
+ oldcxt = MemoryContextSwitchTo(plansource->context);
+ plansource->analyzed_parse_tree = copyObject(analyzed_parse_tree);
+ MemoryContextSwitchTo(oldcxt);
+
+ return plansource;
+}
+
/*
* CreateOneShotCachedPlan: initially create a one-shot plan cache entry.
*
@@ -289,6 +309,7 @@ CreateOneShotCachedPlan(RawStmt *raw_parse_tree,
plansource = (CachedPlanSource *) palloc0(sizeof(CachedPlanSource));
plansource->magic = CACHEDPLANSOURCE_MAGIC;
plansource->raw_parse_tree = raw_parse_tree;
+ plansource->analyzed_parse_tree = NULL;
plansource->query_string = query_string;
plansource->commandTag = commandTag;
plansource->param_types = NULL;
@@ -566,6 +587,42 @@ ReleaseGenericPlan(CachedPlanSource *plansource)
}
}
+/*
+ * We must skip "overhead" operations that involve database access when the
+ * cached plan's subject statement is a transaction control command or one
+ * that requires a snapshot not to be set yet (such as SET or LOCK). More
+ * generally, statements that do not require parse analysis/rewrite/plan
+ * activity never need to be revalidated, so we can treat them all like that.
+ * For the convenience of postgres.c, treat empty statements that way too.
+ */
+static bool
+StmtPlanRequiresRevalidation(CachedPlanSource *plansource)
+{
+ if (plansource->raw_parse_tree != NULL)
+ return stmt_requires_parse_analysis(plansource->raw_parse_tree);
+ else if (plansource->analyzed_parse_tree != NULL)
+ return query_requires_rewrite_plan(plansource->analyzed_parse_tree);
+ /* empty query never needs revalidation */
+ return false;
+}
+
+/*
+ * Determine if creating a plan for this CachedPlanSource requires a snapshot.
+ * In fact this function matches StmtPlanRequiresRevalidation(), but we want
+ * to preserve the distinction between stmt_requires_parse_analysis() and
+ * analyze_requires_snapshot().
+ */
+static bool
+BuildingPlanRequiresSnapshot(CachedPlanSource *plansource)
+{
+ if (plansource->raw_parse_tree != NULL)
+ return analyze_requires_snapshot(plansource->raw_parse_tree);
+ else if (plansource->analyzed_parse_tree != NULL)
+ return query_requires_rewrite_plan(plansource->analyzed_parse_tree);
+ /* empty query never needs a snapshot */
+ return false;
+}
+
/*
* RevalidateCachedQuery: ensure validity of analyzed-and-rewritten query tree.
*
@@ -592,7 +649,6 @@ RevalidateCachedQuery(CachedPlanSource *plansource,
bool release_generic)
{
bool snapshot_set;
- RawStmt *rawtree;
List *tlist; /* transient query-tree list */
List *qlist; /* permanent query-tree list */
TupleDesc resultDesc;
@@ -615,7 +671,10 @@ RevalidateCachedQuery(CachedPlanSource *plansource,
/*
* If the query is currently valid, we should have a saved search_path ---
* check to see if that matches the current environment. If not, we want
- * to force replan.
+ * to force replan. (We could almost ignore this consideration when
+ * working from an analyzed parse tree; but there are scenarios where
+ * planning can have search_path-dependent results, for example if it
+ * inlines an old-style SQL function.)
*/
if (plansource->is_valid)
{
@@ -662,9 +721,9 @@ RevalidateCachedQuery(CachedPlanSource *plansource,
}
/*
- * Discard the no-longer-useful query tree. (Note: we don't want to do
- * this any earlier, else we'd not have been able to release locks
- * correctly in the race condition case.)
+ * Discard the no-longer-useful rewritten query tree. (Note: we don't
+ * want to do this any earlier, else we'd not have been able to release
+ * locks correctly in the race condition case.)
*/
plansource->is_valid = false;
plansource->query_list = NIL;
@@ -711,25 +770,48 @@ RevalidateCachedQuery(CachedPlanSource *plansource,
}
/*
- * Run parse analysis and rule rewriting. The parser tends to scribble on
- * its input, so we must copy the raw parse tree to prevent corruption of
- * the cache.
+ * Run parse analysis (if needed) and rule rewriting.
*/
- rawtree = copyObject(plansource->raw_parse_tree);
- if (rawtree == NULL)
- tlist = NIL;
- else if (plansource->parserSetup != NULL)
- tlist = pg_analyze_and_rewrite_withcb(rawtree,
- plansource->query_string,
- plansource->parserSetup,
- plansource->parserSetupArg,
- queryEnv);
+ if (plansource->raw_parse_tree != NULL)
+ {
+ /* Source is raw parse tree */
+ RawStmt *rawtree;
+
+ /*
+ * The parser tends to scribble on its input, so we must copy the raw
+ * parse tree to prevent corruption of the cache.
+ */
+ rawtree = copyObject(plansource->raw_parse_tree);
+ if (plansource->parserSetup != NULL)
+ tlist = pg_analyze_and_rewrite_withcb(rawtree,
+ plansource->query_string,
+ plansource->parserSetup,
+ plansource->parserSetupArg,
+ queryEnv);
+ else
+ tlist = pg_analyze_and_rewrite_fixedparams(rawtree,
+ plansource->query_string,
+ plansource->param_types,
+ plansource->num_params,
+ queryEnv);
+ }
+ else if (plansource->analyzed_parse_tree != NULL)
+ {
+ /* Source is pre-analyzed query, so we only need to rewrite */
+ Query *analyzed_tree;
+
+ /* The rewriter scribbles on its input, too, so copy */
+ analyzed_tree = copyObject(plansource->analyzed_parse_tree);
+ /* Acquire locks needed before rewriting ... */
+ AcquireRewriteLocks(analyzed_tree, true, false);
+ /* ... and do it */
+ tlist = pg_rewrite_query(analyzed_tree);
+ }
else
- tlist = pg_analyze_and_rewrite_fixedparams(rawtree,
- plansource->query_string,
- plansource->param_types,
- plansource->num_params,
- queryEnv);
+ {
+ /* Empty query, nothing to do */
+ tlist = NIL;
+ }
/* Release snapshot if we got one */
if (snapshot_set)
@@ -963,8 +1045,7 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
*/
snapshot_set = false;
if (!ActiveSnapshotSet() &&
- plansource->raw_parse_tree &&
- analyze_requires_snapshot(plansource->raw_parse_tree))
+ BuildingPlanRequiresSnapshot(plansource))
{
PushActiveSnapshot(GetTransactionSnapshot());
snapshot_set = true;
@@ -1703,6 +1784,7 @@ CopyCachedPlan(CachedPlanSource *plansource)
newsource = (CachedPlanSource *) palloc0(sizeof(CachedPlanSource));
newsource->magic = CACHEDPLANSOURCE_MAGIC;
newsource->raw_parse_tree = copyObject(plansource->raw_parse_tree);
+ newsource->analyzed_parse_tree = copyObject(plansource->analyzed_parse_tree);
newsource->query_string = pstrdup(plansource->query_string);
MemoryContextSetIdentifier(source_context, newsource->query_string);
newsource->commandTag = plansource->commandTag;
diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h
index f1bd18c49f2..f29ed03b476 100644
--- a/src/include/parser/analyze.h
+++ b/src/include/parser/analyze.h
@@ -52,6 +52,7 @@ extern Query *transformStmt(ParseState *pstate, Node *parseTree);
extern bool stmt_requires_parse_analysis(RawStmt *parseTree);
extern bool analyze_requires_snapshot(RawStmt *parseTree);
+extern bool query_requires_rewrite_plan(Query *query);
extern const char *LCS_asString(LockClauseStrength strength);
extern void CheckSelectLocking(Query *qry, LockClauseStrength strength);
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index 199cc323a28..5930fcb50f0 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -25,7 +25,8 @@
#include "utils/resowner.h"
-/* Forward declaration, to avoid including parsenodes.h here */
+/* Forward declarations, to avoid including parsenodes.h here */
+struct Query;
struct RawStmt;
/* possible values for plan_cache_mode */
@@ -45,12 +46,22 @@ extern PGDLLIMPORT int plan_cache_mode;
/*
* CachedPlanSource (which might better have been called CachedQuery)
- * represents a SQL query that we expect to use multiple times. It stores
- * the query source text, the raw parse tree, and the analyzed-and-rewritten
+ * represents a SQL query that we expect to use multiple times. It stores the
+ * query source text, the source parse tree, and the analyzed-and-rewritten
* query tree, as well as adjunct data. Cache invalidation can happen as a
* result of DDL affecting objects used by the query. In that case we discard
* the analyzed-and-rewritten query tree, and rebuild it when next needed.
*
+ * There are two ways in which the source query can be represented: either
+ * as a raw parse tree, or as an analyzed-but-not-rewritten parse tree.
+ * In the latter case we expect that cache invalidation need not affect
+ * the parse-analysis results, only the rewriting and planning steps.
+ * Only one of raw_parse_tree and analyzed_parse_tree can be non-NULL.
+ * (If both are NULL, the CachedPlanSource represents an empty query.)
+ * Note that query_string is typically just an empty string when the
+ * source query is an analyzed parse tree; also, param_types, num_params,
+ * parserSetup, and parserSetupArg will not be used.
+ *
* An actual execution plan, represented by CachedPlan, is derived from the
* CachedPlanSource when we need to execute the query. The plan could be
* either generic (usable with any set of plan parameters) or custom (for a
@@ -78,7 +89,7 @@ extern PGDLLIMPORT int plan_cache_mode;
* though it may be useful if the CachedPlan can be discarded early.)
*
* A CachedPlanSource has two associated memory contexts: one that holds the
- * struct itself, the query source text and the raw parse tree, and another
+ * struct itself, the query source text and the source parse tree, and another
* context that holds the rewritten query tree and associated data. This
* allows the query tree to be discarded easily when it is invalidated.
*
@@ -94,6 +105,7 @@ typedef struct CachedPlanSource
{
int magic; /* should equal CACHEDPLANSOURCE_MAGIC */
struct RawStmt *raw_parse_tree; /* output of raw_parser(), or NULL */
+ struct Query *analyzed_parse_tree; /* analyzed parse tree, or NULL */
const char *query_string; /* source text of query */
CommandTag commandTag; /* command tag for query */
Oid *param_types; /* array of parameter type OIDs, or NULL */
@@ -196,6 +208,9 @@ extern void ReleaseAllPlanCacheRefsInOwner(ResourceOwner owner);
extern CachedPlanSource *CreateCachedPlan(struct RawStmt *raw_parse_tree,
const char *query_string,
CommandTag commandTag);
+extern CachedPlanSource *CreateCachedPlanForQuery(struct Query *analyzed_parse_tree,
+ const char *query_string,
+ CommandTag commandTag);
extern CachedPlanSource *CreateOneShotCachedPlan(struct RawStmt *raw_parse_tree,
const char *query_string,
CommandTag commandTag);
--
2.43.5
From eb9d28bd28ad8741cacb6edca83d30b6da3d6589 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 14 Mar 2025 16:05:25 -0400
Subject: [PATCH v8 2/4] Use custom plan machinery for SQL function
---
src/backend/executor/functions.c | 219 +++++++++++++++++-----
src/test/regress/expected/rowsecurity.out | 51 +++++
src/test/regress/expected/rules.out | 35 ++++
src/test/regress/sql/rowsecurity.sql | 41 ++++
src/test/regress/sql/rules.sql | 24 +++
5 files changed, 328 insertions(+), 42 deletions(-)
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 6aa8e9c4d8a..ae0425c050a 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -33,6 +33,7 @@
#include "utils/datum.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
+#include "utils/plancache.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
@@ -112,6 +113,12 @@ typedef struct
JunkFilter *junkFilter; /* will be NULL if function returns VOID */
+ /* Cached plans support */
+ List *plansource_list; /* list of plansource */
+ List *cplan_list; /* list of cached plans */
+ int planning_stmt_number; /* the number of statement we are
+ * currently planning */
+
/*
* func_state is a List of execution_state records, each of which is the
* first for its original parsetree, with any additional records chained
@@ -122,6 +129,8 @@ typedef struct
MemoryContext fcontext; /* memory context holding this struct and all
* subsidiary data */
+ MemoryContext planning_context; /* memory context which is used for
+ * planning */
LocalTransactionId lxid; /* lxid in which cache was made */
SubTransactionId subxid; /* subxid in which cache was made */
@@ -138,10 +147,9 @@ 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,
+static List *init_execution_state(SQLFunctionCachePtr fcache,
bool lazyEvalOK);
-static void init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK);
+static void init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, 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);
@@ -461,45 +469,52 @@ sql_fn_resolve_param_name(SQLFunctionParseInfoPtr pinfo,
* querytrees. The sublist structure denotes the original query boundaries.
*/
static List *
-init_execution_state(List *queryTree_list,
- SQLFunctionCachePtr fcache,
+init_execution_state(SQLFunctionCachePtr fcache,
bool lazyEvalOK)
{
List *eslist = NIL;
+ List *cplan_list = NIL;
execution_state *lasttages = NULL;
ListCell *lc1;
+ MemoryContext oldcontext;
+
+ /*
+ * Invalidate func_state prior to resetting - otherwise error callback can
+ * access it
+ */
+ fcache->func_state = NIL;
+ MemoryContextReset(fcache->planning_context);
+
+ oldcontext = MemoryContextSwitchTo(fcache->planning_context);
- foreach(lc1, queryTree_list)
+ foreach(lc1, fcache->plansource_list)
{
- List *qtlist = lfirst_node(List, lc1);
+ CachedPlanSource *plansource = (CachedPlanSource *) lfirst(lc1);
execution_state *firstes = NULL;
execution_state *preves = NULL;
ListCell *lc2;
+ CachedPlan *cplan;
+
+ /* Save statement number for error reporting */
+ fcache->planning_stmt_number = foreach_current_index(lc1) + 1;
+
+ /*
+ * Get plan for the query. If paramLI is set, we can get custom plan
+ */
+ cplan = GetCachedPlan(plansource,
+ fcache->paramLI,
+ plansource->is_saved ? CurrentResourceOwner : NULL,
+ NULL);
- foreach(lc2, qtlist)
+ /* Record cplan in plan list to be released on replanning */
+ cplan_list = lappend(cplan_list, cplan);
+
+ /* For each planned statement create execution state */
+ foreach(lc2, cplan->stmt_list)
{
- Query *queryTree = lfirst_node(Query, lc2);
- PlannedStmt *stmt;
+ PlannedStmt *stmt = lfirst_node(PlannedStmt, lc2);
execution_state *newes;
- /* 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);
-
/*
* Precheck all commands for validity in a function. This should
* generally match the restrictions spi.c applies.
@@ -541,7 +556,7 @@ init_execution_state(List *queryTree_list,
newes->stmt = stmt;
newes->qd = NULL;
- if (queryTree->canSetTag)
+ if (stmt->canSetTag)
lasttages = newes;
preves = newes;
@@ -573,6 +588,11 @@ init_execution_state(List *queryTree_list,
fcache->lazyEval = lasttages->lazyEval = true;
}
+ /* We've finished planning, reset planning statement number */
+ fcache->planning_stmt_number = 0;
+ fcache->cplan_list = cplan_list;
+
+ MemoryContextSwitchTo(oldcontext);
return eslist;
}
@@ -580,7 +600,7 @@ init_execution_state(List *queryTree_list,
* Initialize the SQLFunctionCache for a SQL function
*/
static void
-init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
+init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool *lazyEvalOK)
{
FmgrInfo *finfo = fcinfo->flinfo;
Oid foid = finfo->fn_oid;
@@ -596,6 +616,7 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
ListCell *lc;
Datum tmp;
bool isNull;
+ List *plansource_list;
/*
* Create memory context that holds all the SQLFunctionCache data. It
@@ -614,6 +635,10 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
*/
fcache = (SQLFunctionCachePtr) palloc0(sizeof(SQLFunctionCache));
fcache->fcontext = fcontext;
+ /* Create separate context for planning */
+ fcache->planning_context = AllocSetContextCreate(fcache->fcontext,
+ "SQL language functions planning context",
+ ALLOCSET_SMALL_SIZES);
finfo->fn_extra = fcache;
/*
@@ -680,6 +705,7 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
* plancache.c.
*/
queryTree_list = NIL;
+ plansource_list = NIL;
if (!isNull)
{
Node *n;
@@ -695,8 +721,13 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
{
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);
}
@@ -711,6 +742,10 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
{
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,
@@ -751,6 +786,33 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
false,
&resulttlist);
+ /*
+ * Queries could be rewritten by check_sql_fn_retval(). Now when they have
+ * their final form, we can complete plan cache entry creation.
+ */
+ if (plansource_list != NIL)
+ {
+ ListCell *qlc;
+ ListCell *plc;
+
+ forboth(qlc, queryTree_list, plc, plansource_list)
+ {
+ List *queryTree_sublist = lfirst(qlc);
+ CachedPlanSource *plansource = lfirst(plc);
+
+ /* Finish filling in the CachedPlanSource */
+ CompleteCachedPlan(plansource,
+ queryTree_sublist,
+ NULL,
+ NULL,
+ 0,
+ (ParserSetupHook) sql_fn_parser_setup,
+ fcache->pinfo,
+ CURSOR_OPT_PARALLEL_OK | CURSOR_OPT_NO_SCROLL,
+ 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
@@ -792,13 +854,10 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
* 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;
+ *lazyEvalOK = true;
}
- /* Finally, plan the queries */
- fcache->func_state = init_execution_state(queryTree_list,
- fcache,
- lazyEvalOK);
+ fcache->plansource_list = plansource_list;
/* Mark fcache with time of creation to show it's valid */
fcache->lxid = MyProc->vxid.lxid;
@@ -971,7 +1030,12 @@ postquel_sub_params(SQLFunctionCachePtr fcache,
prm->value = MakeExpandedObjectReadOnly(fcinfo->args[i].value,
prm->isnull,
get_typlen(argtypes[i]));
- prm->pflags = 0;
+
+ /*
+ * PARAM_FLAG_CONST is necessary to build efficient custom plan.
+ */
+ prm->pflags = PARAM_FLAG_CONST;
+
prm->ptype = argtypes[i];
}
}
@@ -1024,6 +1088,33 @@ postquel_get_single_result(TupleTableSlot *slot,
return value;
}
+/*
+ * Release plans. This function is called prior to planning
+ * statements with new parameters. When custom plans are generated
+ * for each function call in a statement, they can consume too much memory, so
+ * release them. Generic plans will survive it as plansource holds
+ * reference to a generic plan.
+ */
+static void
+release_plans(List *cplans)
+{
+ ListCell *lc;
+
+ /*
+ * We support separate plan list, so that we visit each plan here only
+ * once
+ */
+ foreach(lc, cplans)
+ {
+ CachedPlan *cplan = lfirst(lc);
+
+ ReleaseCachedPlan(cplan, cplan->is_saved ? CurrentResourceOwner : NULL);
+ }
+
+ /* Cleanup the list itself */
+ list_free(cplans);
+}
+
/*
* fmgr_sql: function call manager for SQL functions
*/
@@ -1042,6 +1133,7 @@ fmgr_sql(PG_FUNCTION_ARGS)
Datum result;
List *eslist;
ListCell *eslc;
+ bool build_cached_plans = false;
/*
* Setup error traceback support for ereport()
@@ -1097,7 +1189,11 @@ fmgr_sql(PG_FUNCTION_ARGS)
if (fcache == NULL)
{
- init_sql_fcache(fcinfo, PG_GET_COLLATION(), lazyEvalOK);
+ /*
+ * init_sql_fcache() can set lazyEvalOK in additional cases when it
+ * determines that materialize won't work.
+ */
+ init_sql_fcache(fcinfo, PG_GET_COLLATION(), &lazyEvalOK);
fcache = (SQLFunctionCachePtr) fcinfo->flinfo->fn_extra;
}
@@ -1131,12 +1227,37 @@ fmgr_sql(PG_FUNCTION_ARGS)
break;
}
+ /*
+ * We skip actual planning for initial run, so in this case we have to
+ * build cached plans now.
+ */
+ if (fcache->plansource_list != NIL && eslist == NIL)
+ build_cached_plans = true;
+
/*
* 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)
+ if ((is_first && es && es->status == F_EXEC_START) || build_cached_plans)
+ {
postquel_sub_params(fcache, fcinfo);
+ if (fcache->plansource_list)
+ {
+ /* replan the queries */
+ fcache->func_state = init_execution_state(fcache,
+ lazyEvalOK);
+ /* restore execution state and eslist-related variables */
+ eslist = fcache->func_state;
+ /* find the first non-NULL execution state */
+ foreach(eslc, eslist)
+ {
+ es = (execution_state *) lfirst(eslc);
+
+ if (es)
+ break;
+ }
+ }
+ }
/*
* Build tuplestore to hold results, if we don't have one already. Note
@@ -1391,6 +1512,10 @@ fmgr_sql(PG_FUNCTION_ARGS)
es = es->next;
}
}
+
+ /* Release plans when functions stops executing */
+ release_plans(fcache->cplan_list);
+ fcache->cplan_list = NULL;
}
error_context_stack = sqlerrcontext.previous;
@@ -1430,13 +1555,19 @@ sql_exec_error_callback(void *arg)
}
/*
- * 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
+ * Try to determine where in the function we failed. If failure happens
+ * while building plans, look at planning_stmt_number. Else 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 (fcache->func_state)
+ if (fcache->planning_stmt_number)
+ {
+ errcontext("SQL function \"%s\" statement %d",
+ fcache->fname, fcache->planning_stmt_number);
+ }
+ else if (fcache->func_state)
{
execution_state *es;
int query_num;
@@ -1522,6 +1653,10 @@ ShutdownSQLFunction(Datum arg)
tuplestore_end(fcache->tstore);
fcache->tstore = NULL;
+ /* Release plans when functions stops executing */
+ release_plans(fcache->cplan_list);
+ fcache->cplan_list = NULL;
+
/* execUtils will deregister the callback... */
fcache->shutdown_reg = false;
}
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 87929191d06..438eaf69928 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -4695,6 +4695,57 @@ RESET ROLE;
DROP FUNCTION rls_f();
DROP VIEW rls_v;
DROP TABLE rls_t;
+-- RLS changes invalidate cached function plans
+create table rls_t (c text);
+create table test_t (c text);
+insert into rls_t values ('a'), ('b'), ('c'), ('d');
+insert into test_t values ('a'), ('b');
+alter table rls_t enable row level security;
+grant select on rls_t to regress_rls_alice;
+grant select on test_t to regress_rls_alice;
+create policy p1 on rls_t for select to regress_rls_alice using (c = current_setting('rls_test.blah'));
+-- Function changes row_security setting and so invalidates plan
+create or replace function rls_f(text)
+ RETURNS text
+ LANGUAGE sql
+BEGIN ATOMIC
+ select set_config('rls_test.blah', $1, true) || set_config('row_security', 'false', true) || string_agg(c, ',' order
byc) from rls_t;
+END;
+-- Table owner bypasses RLS
+select rls_f(c) from test_t order by rls_f;
+ rls_f
+-------------
+ aoffa,b,c,d
+ boffa,b,c,d
+(2 rows)
+
+set role regress_rls_alice;
+-- For casual user changes in row_security setting lead
+-- to error during query rewrite
+select rls_f(c) from test_t order by rls_f;
+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, but cached plan invalidates
+select rls_f(c) from test_t order by rls_f;
+ rls_f
+-------------
+ aoffa,b,c,d
+ boffa,b,c,d
+(2 rows)
+
+-- For casual user changes in row_security setting lead
+-- to plan invalidation and error during query rewrite
+set role regress_rls_alice;
+select rls_f(c) from test_t order by rls_f;
+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;
+drop function rls_f;
+drop table rls_t, test_t;
--
-- Clean up objects
--
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 62f69ac20b2..b9fe71f391d 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3878,3 +3878,38 @@ DROP TABLE ruletest_t3;
DROP TABLE ruletest_t2;
DROP TABLE ruletest_t1;
DROP USER regress_rule_user1;
+-- Test that SQL functions correctly handle DO NOTHING rule
+CREATE TABLE some_data (i int, data text);
+CREATE TABLE some_data_values (i int, data text);
+CREATE FUNCTION insert_data(i int, data text)
+RETURNS INT
+AS $$
+INSERT INTO some_data VALUES ($1, $2);
+SELECT 1;
+$$ LANGUAGE SQL;
+INSERT INTO some_data_values SELECT i , 'data'|| i FROM generate_series(1, 10) i;
+CREATE RULE some_data_noinsert AS ON INSERT TO some_data DO INSTEAD NOTHING;
+SELECT insert_data(i, data) FROM some_data_values;
+ insert_data
+-------------
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+(10 rows)
+
+SELECT * FROM some_data ORDER BY i;
+ i | data
+---+------
+(0 rows)
+
+DROP RULE some_data_noinsert ON some_data;
+DROP TABLE some_data_values;
+DROP TABLE some_data;
+DROP FUNCTION insert_data(int, text);
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index f61dbbf9581..9fe8f4b059c 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -2307,6 +2307,47 @@ DROP FUNCTION rls_f();
DROP VIEW rls_v;
DROP TABLE rls_t;
+-- RLS changes invalidate cached function plans
+create table rls_t (c text);
+create table test_t (c text);
+
+insert into rls_t values ('a'), ('b'), ('c'), ('d');
+insert into test_t values ('a'), ('b');
+alter table rls_t enable row level security;
+grant select on rls_t to regress_rls_alice;
+grant select on test_t to regress_rls_alice;
+create policy p1 on rls_t for select to regress_rls_alice using (c = current_setting('rls_test.blah'));
+
+-- Function changes row_security setting and so invalidates plan
+create or replace function rls_f(text)
+ RETURNS text
+ LANGUAGE sql
+BEGIN ATOMIC
+ select set_config('rls_test.blah', $1, true) || set_config('row_security', 'false', true) || string_agg(c, ',' order
byc) from rls_t;
+END;
+
+-- Table owner bypasses RLS
+select rls_f(c) from test_t order by rls_f;
+set role regress_rls_alice;
+-- For casual user changes in row_security setting lead
+-- to error during query rewrite
+select rls_f(c) from test_t order by rls_f;
+reset role;
+
+set plan_cache_mode to force_generic_plan;
+-- Table owner bypasses RLS, but cached plan invalidates
+select rls_f(c) from test_t order by rls_f;
+-- For casual user changes in row_security setting lead
+-- to plan invalidation and error during query rewrite
+set role regress_rls_alice;
+select rls_f(c) from test_t order by rls_f;
+reset role;
+reset plan_cache_mode;
+reset rls_test.blah;
+
+drop function rls_f;
+drop table rls_t, test_t;
+
--
-- Clean up objects
--
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index fdd3ff1d161..505449452ee 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1432,3 +1432,27 @@ DROP TABLE ruletest_t2;
DROP TABLE ruletest_t1;
DROP USER regress_rule_user1;
+
+-- Test that SQL functions correctly handle DO NOTHING rule
+CREATE TABLE some_data (i int, data text);
+CREATE TABLE some_data_values (i int, data text);
+
+CREATE FUNCTION insert_data(i int, data text)
+RETURNS INT
+AS $$
+INSERT INTO some_data VALUES ($1, $2);
+SELECT 1;
+$$ LANGUAGE SQL;
+
+INSERT INTO some_data_values SELECT i , 'data'|| i FROM generate_series(1, 10) i;
+
+CREATE RULE some_data_noinsert AS ON INSERT TO some_data DO INSTEAD NOTHING;
+
+SELECT insert_data(i, data) FROM some_data_values;
+
+SELECT * FROM some_data ORDER BY i;
+
+DROP RULE some_data_noinsert ON some_data;
+DROP TABLE some_data_values;
+DROP TABLE some_data;
+DROP FUNCTION insert_data(int, text);
--
2.43.5
From c0dfeca675283aec80768d5cb82bc8dbcff50b13 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 14 Mar 2025 16:14:51 -0400
Subject: [PATCH v8 3/4] Introduce SQL functions plan cache
---
src/backend/executor/functions.c | 657 ++++++++++++++----
.../expected/test_extensions.out | 2 +-
src/tools/pgindent/typedefs.list | 2 +
3 files changed, 526 insertions(+), 135 deletions(-)
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index ae0425c050a..678ca55a026 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -18,6 +18,8 @@
#include "access/xact.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
+#include "commands/trigger.h"
+#include "commands/event_trigger.h"
#include "executor/functions.h"
#include "funcapi.h"
#include "miscadmin.h"
@@ -138,6 +140,46 @@ typedef struct
typedef SQLFunctionCache *SQLFunctionCachePtr;
+/*
+ * Plan cache-related structures
+ */
+typedef struct SQLFunctionPlanKey
+{
+ Oid fn_oid;
+ Oid inputCollation;
+ Oid argtypes[FUNC_MAX_ARGS];
+} SQLFunctionPlanKey;
+
+typedef struct SQLFunctionPlanEntry
+{
+ SQLFunctionPlanKey key;
+
+ /* Fields required to invalidate a cache entry */
+ TransactionId fn_xmin;
+ ItemPointerData fn_tid;
+
+ /*
+ * result_tlist is required to recreate function execution state as well
+ * as to validate a cache entry
+ */
+ List *result_tlist;
+
+ bool returnsTuple; /* True if this function returns tuple */
+ List *plansource_list; /* List of CachedPlanSource for this
+ * function */
+
+ /*
+ * SQLFunctionParseInfoPtr is used as hooks arguments, so should persist
+ * across calls. Fortunately, if it doesn't, this means that argtypes or
+ * collation mismatches and we get new cache entry.
+ */
+ SQLFunctionParseInfoPtr pinfo; /* cached information about arguments */
+
+ MemoryContext entry_ctx; /* memory context for allocated fields of this
+ * entry */
+} SQLFunctionPlanEntry;
+
+static HTAB *sql_plan_cache_htab = NULL;
/* non-export function prototypes */
static Node *sql_fn_param_ref(ParseState *pstate, ParamRef *pref);
@@ -171,6 +213,48 @@ static bool sqlfunction_receive(TupleTableSlot *slot, DestReceiver *self);
static void sqlfunction_shutdown(DestReceiver *self);
static void sqlfunction_destroy(DestReceiver *self);
+/* SQL-functions plan cache-related routines */
+static void compute_plan_entry_key(SQLFunctionPlanKey *hashkey, FunctionCallInfo fcinfo, Form_pg_proc
procedureStruct);
+static SQLFunctionPlanEntry *get_cached_plan_entry(SQLFunctionPlanKey *hashkey);
+static void save_cached_plan_entry(SQLFunctionPlanKey *hashkey, HeapTuple procedureTuple, List *plansource_list, List
*result_tlist,bool returnsTuple, SQLFunctionParseInfoPtr pinfo, MemoryContext alianable_context);
+static void delete_cached_plan_entry(SQLFunctionPlanEntry *entry);
+
+static bool check_sql_fn_retval_matches(List *tlist, Oid rettype, TupleDesc rettupdesc, char prokind);
+static bool target_entry_has_compatible_type(TargetEntry *tle, Oid res_type, int32 res_typmod);
+
+/*
+ * Fill array of arguments with actual function argument types oids
+ */
+static void
+compute_argument_types(Oid *argOidVect, Form_pg_proc procedureStruct, Node *call_expr)
+{
+ int argnum;
+ int nargs;
+
+ nargs = procedureStruct->pronargs;
+ if (nargs > 0)
+ {
+ memcpy(argOidVect,
+ procedureStruct->proargtypes.values,
+ nargs * sizeof(Oid));
+
+ for (argnum = 0; argnum < nargs; argnum++)
+ {
+ Oid argtype = argOidVect[argnum];
+
+ if (IsPolymorphicType(argtype))
+ {
+ argtype = get_call_expr_argtype(call_expr, argnum);
+ if (argtype == InvalidOid)
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("could not determine actual type of argument declared %s",
+ format_type_be(argOidVect[argnum]))));
+ argOidVect[argnum] = argtype;
+ }
+ }
+ }
+}
/*
* Prepare the SQLFunctionParseInfo struct for parsing a SQL function body
@@ -204,31 +288,8 @@ prepare_sql_fn_parse_info(HeapTuple procedureTuple,
pinfo->nargs = nargs = procedureStruct->pronargs;
if (nargs > 0)
{
- Oid *argOidVect;
- int argnum;
-
- argOidVect = (Oid *) palloc(nargs * sizeof(Oid));
- memcpy(argOidVect,
- procedureStruct->proargtypes.values,
- nargs * sizeof(Oid));
-
- for (argnum = 0; argnum < nargs; argnum++)
- {
- Oid argtype = argOidVect[argnum];
-
- if (IsPolymorphicType(argtype))
- {
- argtype = get_call_expr_argtype(call_expr, argnum);
- if (argtype == InvalidOid)
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("could not determine actual type of argument declared %s",
- format_type_be(argOidVect[argnum]))));
- argOidVect[argnum] = argtype;
- }
- }
-
- pinfo->argtypes = argOidVect;
+ pinfo->argtypes = (Oid *) palloc(nargs * sizeof(Oid));
+ compute_argument_types(pinfo->argtypes, procedureStruct, call_expr);
}
/*
@@ -596,6 +657,264 @@ init_execution_state(SQLFunctionCachePtr fcache,
return eslist;
}
+/*
+ * Compute key for searching plan entry in backend cache
+ */
+static void
+compute_plan_entry_key(SQLFunctionPlanKey *hashkey, FunctionCallInfo fcinfo, Form_pg_proc procedureStruct)
+{
+ MemSet(hashkey, 0, sizeof(SQLFunctionPlanKey));
+
+ hashkey->fn_oid = fcinfo->flinfo->fn_oid;
+
+ /* set input collation, if known */
+ hashkey->inputCollation = fcinfo->fncollation;
+
+ if (procedureStruct->pronargs > 0)
+ {
+ /* get the argument types */
+ compute_argument_types(hashkey->argtypes, procedureStruct, fcinfo->flinfo->fn_expr);
+ }
+}
+
+/*
+ * Get cached plan by pre-computed key
+ */
+static SQLFunctionPlanEntry *
+get_cached_plan_entry(SQLFunctionPlanKey *hashkey)
+{
+ SQLFunctionPlanEntry *plan_entry = NULL;
+
+ if (sql_plan_cache_htab)
+ {
+ plan_entry = (SQLFunctionPlanEntry *) hash_search(sql_plan_cache_htab,
+ hashkey,
+ HASH_FIND,
+ NULL);
+ }
+ return plan_entry;
+}
+
+/*
+ * Save function execution plan in cache
+ */
+static void
+save_cached_plan_entry(SQLFunctionPlanKey *hashkey, HeapTuple procedureTuple, List *plansource_list, List
*result_tlist,bool returnsTuple, SQLFunctionParseInfoPtr pinfo, MemoryContext alianable_context)
+{
+ MemoryContext oldcontext;
+ MemoryContext entry_context;
+ SQLFunctionPlanEntry *entry;
+ ListCell *lc;
+ bool found;
+
+ if (sql_plan_cache_htab == NULL)
+ {
+ HASHCTL ctl;
+
+ ctl.keysize = sizeof(SQLFunctionPlanKey);
+ ctl.entrysize = sizeof(SQLFunctionPlanEntry);
+ ctl.hcxt = CacheMemoryContext;
+
+ sql_plan_cache_htab = hash_create("SQL function plan hash",
+ 100 /* arbitrary initial size */ ,
+ &ctl,
+ HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
+ }
+
+ entry = (SQLFunctionPlanEntry *) hash_search(sql_plan_cache_htab,
+ hashkey,
+ HASH_ENTER,
+ &found);
+ if (found)
+ elog(WARNING, "trying to insert a function that already exists");
+
+ /*
+ * Create long-lived memory context that holds entry fields
+ */
+ entry_context = AllocSetContextCreate(CacheMemoryContext,
+ "SQL function plan entry context",
+ ALLOCSET_DEFAULT_SIZES);
+
+ oldcontext = MemoryContextSwitchTo(entry_context);
+
+ /* fill entry */
+ memcpy(&entry->key, hashkey, sizeof(SQLFunctionPlanKey));
+
+ entry->entry_ctx = entry_context;
+
+ /* Some generated data, like pinfo, should be reparented */
+ MemoryContextSetParent(alianable_context, entry->entry_ctx);
+
+ entry->pinfo = pinfo;
+
+ /* Preserve list in long-lived context */
+ if (plansource_list)
+ entry->plansource_list = list_copy(plansource_list);
+ else
+ entry->plansource_list = NULL;
+
+ entry->result_tlist = copyObject(result_tlist);
+
+ entry->returnsTuple = returnsTuple;
+
+ /* Fill fields needed to invalidate cache entry */
+ entry->fn_xmin = HeapTupleHeaderGetRawXmin(procedureTuple->t_data);
+ entry->fn_tid = procedureTuple->t_self;
+
+ /* Save plans */
+ foreach(lc, entry->plansource_list)
+ {
+ CachedPlanSource *plansource = (CachedPlanSource *) lfirst(lc);
+
+ SaveCachedPlan(plansource);
+ }
+ MemoryContextSwitchTo(oldcontext);
+
+}
+
+/*
+ * Remove plan from cache
+ */
+static void
+delete_cached_plan_entry(SQLFunctionPlanEntry *entry)
+{
+ ListCell *lc;
+ bool found;
+
+ /* Release plans */
+ foreach(lc, entry->plansource_list)
+ {
+ CachedPlanSource *plansource = (CachedPlanSource *) lfirst(lc);
+
+ DropCachedPlan(plansource);
+ }
+ MemoryContextDelete(entry->entry_ctx);
+
+ hash_search(sql_plan_cache_htab, &entry->key, HASH_REMOVE, &found);
+ Assert(found);
+}
+
+/*
+ * Determine if TargetEntry is compatible to specified type
+ */
+static bool
+target_entry_has_compatible_type(TargetEntry *tle, Oid res_type, int32 res_typmod)
+{
+ Var *var;
+ Node *cast_result;
+ bool result = true;
+
+ /* Are types equivalent? */
+ var = makeVarFromTargetEntry(1, tle);
+
+ cast_result = coerce_to_target_type(NULL,
+ (Node *) var,
+ var->vartype,
+ res_type, res_typmod,
+ COERCION_ASSIGNMENT,
+ COERCE_IMPLICIT_CAST,
+ -1);
+
+ /*
+ * If conversion is not possible or requires a cast, entry is incompatible
+ * with the type.
+ */
+ if (cast_result == NULL || cast_result != (Node *) var)
+ result = false;
+
+ if (cast_result && cast_result != (Node *) var)
+ pfree(cast_result);
+ pfree(var);
+
+ return result;
+}
+
+/*
+ * Check if result tlist would be changed by check_sql_fn_retval()
+ */
+static bool
+check_sql_fn_retval_matches(List *tlist, Oid rettype, TupleDesc rettupdesc, char prokind)
+{
+ char fn_typtype;
+ int tlistlen;
+
+ /*
+ * Count the non-junk entries in the result targetlist.
+ */
+ tlistlen = ExecCleanTargetListLength(tlist);
+
+ fn_typtype = get_typtype(rettype);
+
+ if (fn_typtype == TYPTYPE_BASE ||
+ fn_typtype == TYPTYPE_DOMAIN ||
+ fn_typtype == TYPTYPE_ENUM ||
+ fn_typtype == TYPTYPE_RANGE ||
+ fn_typtype == TYPTYPE_MULTIRANGE)
+ {
+ TargetEntry *tle;
+
+ /* Something unexpected, invalidate cached plan */
+ if (tlistlen != 1)
+ return false;
+
+ tle = (TargetEntry *) linitial(tlist);
+
+ return target_entry_has_compatible_type(tle, rettype, -1);
+ }
+ else if (fn_typtype == TYPTYPE_COMPOSITE || rettype == RECORDOID)
+ {
+ ListCell *lc;
+ int colindex;
+ int tupnatts;
+
+ if (tlistlen == 1 && prokind != PROKIND_PROCEDURE)
+ {
+ TargetEntry *tle = (TargetEntry *) linitial(tlist);
+
+ return target_entry_has_compatible_type(tle, rettype, -1);
+ }
+
+ /* We consider results comnpatible if there's no tupledesc */
+ if (rettupdesc == NULL)
+ return true;
+
+ /*
+ * Verify that saved targetlist matches the return tuple type.
+ */
+ tupnatts = rettupdesc->natts;
+ colindex = 0;
+ foreach(lc, tlist)
+ {
+ TargetEntry *tle = (TargetEntry *) lfirst(lc);
+ Form_pg_attribute attr;
+
+ /* resjunk columns can simply be ignored */
+ if (tle->resjunk)
+ continue;
+
+ do
+ {
+ colindex++;
+ if (colindex > tupnatts)
+ return false;
+
+ attr = TupleDescAttr(rettupdesc, colindex - 1);
+ } while (attr->attisdropped);
+
+ if (!target_entry_has_compatible_type(tle, attr->atttypid, attr->atttypmod))
+ return false;
+ }
+
+ /* remaining columns in rettupdesc had better all be dropped */
+ for (colindex++; colindex <= tupnatts; colindex++)
+ {
+ if (!TupleDescCompactAttr(rettupdesc, colindex - 1)->attisdropped)
+ return false;
+ }
+ }
+ return true;
+}
+
/*
* Initialize the SQLFunctionCache for a SQL function
*/
@@ -617,6 +936,10 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool *lazyEvalOK)
Datum tmp;
bool isNull;
List *plansource_list;
+ SQLFunctionPlanEntry *cached_plan_entry = NULL;
+ SQLFunctionPlanKey plan_cache_entry_key;
+ bool use_plan_cache;
+ bool plan_cache_entry_valid;
/*
* Create memory context that holds all the SQLFunctionCache data. It
@@ -674,15 +997,6 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool *lazyEvalOK)
fcache->readonly_func =
(procedureStruct->provolatile != PROVOLATILE_VOLATILE);
- /*
- * 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);
-
/*
* And of course we need the function body text.
*/
@@ -695,122 +1009,200 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool *lazyEvalOK)
Anum_pg_proc_prosqlbody,
&isNull);
+
+ use_plan_cache = true;
+ plan_cache_entry_valid = false;
+
/*
- * 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.
+ * If function is trigger, we can see different rowtypes or transition
+ * table names. So don't use cache for such plans.
*/
- queryTree_list = NIL;
- plansource_list = NIL;
- if (!isNull)
- {
- Node *n;
- List *stored_query_list;
+ if (CALLED_AS_TRIGGER(fcinfo) || CALLED_AS_EVENT_TRIGGER(fcinfo))
+ use_plan_cache = false;
- n = stringToNode(TextDatumGetCString(tmp));
- if (IsA(n, List))
- stored_query_list = linitial_node(List, castNode(List, n));
- else
- stored_query_list = list_make1(n);
+ if (use_plan_cache)
+ {
+ compute_plan_entry_key(&plan_cache_entry_key, fcinfo, procedureStruct);
- foreach(lc, stored_query_list)
+ cached_plan_entry = get_cached_plan_entry(&plan_cache_entry_key);
+ if (cached_plan_entry)
{
- 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);
+ if (cached_plan_entry->fn_xmin == HeapTupleHeaderGetRawXmin(procedureTuple->t_data) &&
+ ItemPointerEquals(&cached_plan_entry->fn_tid, &procedureTuple->t_self))
+ {
+ /*
+ * Avoid using plan if returned result type doesn't match the
+ * expected one. check_sql_fn_retval() in this case would
+ * change query to match expected result type. But we've
+ * already planned query, possibly modified to match another
+ * result type. So discard the cached entry and replan.
+ */
+ if (check_sql_fn_retval_matches(cached_plan_entry->result_tlist, rettype, rettupdesc,
procedureStruct->prokind))
+ plan_cache_entry_valid = true;
+ }
+ if (!plan_cache_entry_valid)
+ delete_cached_plan_entry(cached_plan_entry);
}
}
+
+ if (plan_cache_entry_valid)
+ {
+ plansource_list = cached_plan_entry->plansource_list;
+ resulttlist = copyObject(cached_plan_entry->result_tlist);
+ fcache->returnsTuple = cached_plan_entry->returnsTuple;
+ fcache->pinfo = cached_plan_entry->pinfo;
+ }
else
{
- List *raw_parsetree_list;
+ MemoryContext alianable_context = fcontext;
+
+ /* We need to preserve parse info */
+ if (use_plan_cache)
+ {
+ alianable_context = AllocSetContextCreate(CurrentMemoryContext,
+ "SQL function plan entry alianable context",
+ ALLOCSET_DEFAULT_SIZES);
+
+ MemoryContextSwitchTo(alianable_context);
+ }
- raw_parsetree_list = pg_parse_query(fcache->src);
+ /*
+ * 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);
+
+ if (use_plan_cache)
+ MemoryContextSwitchTo(fcontext);
- foreach(lc, raw_parsetree_list)
+ /*
+ * 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.
+ */
+
+ plansource_list = NIL;
+
+ queryTree_list = NIL;
+ if (!isNull)
{
- 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);
+ Node *n;
+ List *stored_query_list;
+
+ n = stringToNode(TextDatumGetCString(tmp));
+ if (IsA(n, List))
+ stored_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);
+ }
}
- }
+ else
+ {
+ List *raw_parsetree_list;
- /*
- * Check that there are no statements we don't want to allow.
- */
- check_sql_fn_statements(queryTree_list);
+ raw_parsetree_list = pg_parse_query(fcache->src);
- /*
- * Check that the function returns the type it claims to. Although in
- * simple cases this was already done when the function was defined, we
- * have to recheck because database objects used in the function's queries
- * might have changed type. We'd have to recheck anyway if 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.)
- *
- * Note: we set fcache->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
- * lazy eval mode in that case; otherwise we'd need extra code to expand
- * 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,
- &resulttlist);
+ 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);
+ }
+ }
- /*
- * Queries could be rewritten by check_sql_fn_retval(). Now when they have
- * their final form, we can complete plan cache entry creation.
- */
- if (plansource_list != NIL)
- {
- ListCell *qlc;
- ListCell *plc;
+ /*
+ * 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
+ * simple cases this was already done when the function was defined,
+ * we have to recheck because database objects used in the function's
+ * queries might have changed type. We'd have to recheck anyway if
+ * 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.)
+ *
+ * Note: we set fcache->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 lazy eval mode in that case; otherwise
+ * we'd need extra code to expand the rowtype column into multiple
+ * columns, since we have no way to notify the caller that it should
+ * do that.)
+ */
- forboth(qlc, queryTree_list, plc, plansource_list)
+ fcache->returnsTuple = check_sql_fn_retval(queryTree_list,
+ rettype,
+ rettupdesc,
+ procedureStruct->prokind,
+ false,
+ &resulttlist);
+
+ /*
+ * Queries could be rewritten by check_sql_fn_retval(). Now when they
+ * have their final form, we can complete plan cache entry creation.
+ */
+ if (plansource_list != NIL)
{
- List *queryTree_sublist = lfirst(qlc);
- CachedPlanSource *plansource = lfirst(plc);
-
- /* Finish filling in the CachedPlanSource */
- CompleteCachedPlan(plansource,
- queryTree_sublist,
- NULL,
- NULL,
- 0,
- (ParserSetupHook) sql_fn_parser_setup,
- fcache->pinfo,
- CURSOR_OPT_PARALLEL_OK | CURSOR_OPT_NO_SCROLL,
- false);
+ ListCell *qlc;
+ ListCell *plc;
+
+ forboth(qlc, queryTree_list, plc, plansource_list)
+ {
+ List *queryTree_sublist = lfirst(qlc);
+ CachedPlanSource *plansource = lfirst(plc);
+
+ /* Finish filling in the CachedPlanSource */
+ CompleteCachedPlan(plansource,
+ queryTree_sublist,
+ NULL,
+ NULL,
+ 0,
+ (ParserSetupHook) sql_fn_parser_setup,
+ fcache->pinfo,
+ CURSOR_OPT_PARALLEL_OK | CURSOR_OPT_NO_SCROLL,
+ false);
+ }
}
+
+ /* If we can possibly use cached plan entry, save it. */
+ if (use_plan_cache)
+ save_cached_plan_entry(&plan_cache_entry_key, procedureTuple, plansource_list, resulttlist,
fcache->returnsTuple,fcache->pinfo, alianable_context);
}
/*
@@ -1110,9 +1502,6 @@ release_plans(List *cplans)
ReleaseCachedPlan(cplan, cplan->is_saved ? CurrentResourceOwner : NULL);
}
-
- /* Cleanup the list itself */
- list_free(cplans);
}
/*
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/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 93339ef3c58..1671101cebb 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2577,6 +2577,8 @@ SQLFunctionCache
SQLFunctionCachePtr
SQLFunctionParseInfo
SQLFunctionParseInfoPtr
+SQLFunctionPlanEntry
+SQLFunctionPlanKey
SQLValueFunction
SQLValueFunctionOp
SSL
--
2.43.5
From 8c6cee3bdae49c6282fd8a667a8bc9312e22df02 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 14 Mar 2025 16:24:51 -0400
Subject: [PATCH v8 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 678ca55a026..f745458d3d0 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"
@@ -940,6 +941,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
@@ -1090,32 +1092,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
{
@@ -1126,25 +1113,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
@@ -1154,7 +1131,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
@@ -1166,13 +1146,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.5
pgsql-hackers by date: