Re: SQLFunctionCache and generic plans - Mailing list pgsql-hackers
| From | Tom Lane |
|---|---|
| Subject | Re: SQLFunctionCache and generic plans |
| Date | |
| Msg-id | 3605427.1743351017@sss.pgh.pa.us Whole thread Raw |
| In response to | Re: SQLFunctionCache and generic plans (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: SQLFunctionCache and generic plans
Re: SQLFunctionCache and generic plans Re: SQLFunctionCache and generic plans |
| List | pgsql-hackers |
I spent some time reading and reworking this code, and have
arrived at a patch set that I'm pretty happy with. I'm not
sure it's quite committable but it's close:
0001: Same as in v8, extend plancache.c to allow caching
starting from a Query.
0002: Add a post-rewrite callback hook in plancache.c. There
is no chance of getting check_sql_fn_retval to work correctly
without that in the raw-parsetree case: we can't apply the
transformation on what goes into the plancache, and we have
to be able to re-apply it if plancache regenerates the plan
from the raw parse trees.
0003: As I mentioned yesterday, I think we should use the same
cache management logic that plpgsql does, and the best way for
that to happen is to share code. So this invents a new module
"funccache" that extracts the logic plpgsql was using. I did
have to add one feature that plpgsql doesn't have, to allow
part of the cache key to be the output rowtype. Otherwise
cases like this one from rangefuncs.sql won't work:
select * from array_to_set(array['one', 'two']) as t(f1 int,f2 text);
select * from array_to_set(array['one', 'two']) as t(f1 numeric(4,2),f2 text);
These have to have separate cached plans because check_sql_fn_retval
will modify the plans differently.
0004: Restructure check_sql_fn_retval so that we can use it in the
callback hook envisioned in 0002. There's an edge-case semantics
change as explained in the commit message; perhaps that will be
controversial?
0005: This extracts the RLS test case you had and commits it
with the old non-failing behavior, just so that we can see that
the new code does it differently. (I didn't adopt your test
from rules.sql, because AFAICS it works the same with or without
this patch set. What was the point of that one again?)
0006: The guts of the patch. I couldn't break this down any
further.
One big difference from what you had is that there is only one path
of control: we always use the plan cache. The hack you had to not
use it for triggers was only needed because you didn't include the
right cache key items to distinguish different trigger usages, but
the code coming from plpgsql has that right.
Also, the memory management is done a bit differently. The
"fcontext" memory context holding the SQLFunctionCache struct is
now discarded at the end of each execution of the SQL function,
which considerably alleviates worries about leaking memory there.
I invented a small "SQLFunctionLink" struct that is what fn_extra
points at, and it survives as long as the FmgrInfo does, so that's
what saves us from redoing hash key computations in most cases.
I also moved some code around -- notably, init_execution_state now
builds plans for only one CachedPlanState at a time, and we don't
try to construct the output JunkFilter until we plan the last
CachedPlanState. Because of this change, there's no longer a List
of execution_state sublists, but only a sublist matching the
current CachedPlan. We track where we are in the function using
an integer counter of the CachedPlanStates instead.
There's more stuff that could be done, but I feel that all of this
could be left for later:
* I really wanted to do what I mentioned upthread and change things
so we don't even parse the later queries until we've executed the
ones before that. However that seems to be a bit of a mess to
make happen, and the patch is large/complex enough already.
* The execution_state sublist business seems quite vestigial now:
we could probably drop it in favor of one set of those fields and
a counter. But that would involve a lot of notational churn,
much of it in code that doesn't need changes otherwise, and in
the end it would not buy much except removing a tiny amount of
transient space usage. Maybe some other day.
* There's some duplication of effort between cache key computation
and the callers, particularly that for SQL functions we end up
doing get_call_result_type() twice during the initial call.
This could probably be fixed with more refactoring, but it's not
really expensive enough to get too excited about.
I redid Pavel's tests from [1], and got these results in
non-assert builds:
master v10 patch
fx: 50077.251 ms 21221.104 ms
fx2: 8578.874 ms 8576.935 ms
fx3: 66331.186 ms 21173.215 ms
fx4: 56233.003 ms 22757.320 ms
fx5: 13248.177 ms 12370.693 ms
fx6: 13103.840 ms 12245.266 ms
We get substantial wins on all of fx, fx3, fx4. fx2 is the
case that gets inlined and never reaches functions.c, so the
lack of change there is expected. What I found odd is that
I saw a small speedup (~6%) on fx5 and fx6; those functions
are in plpgsql so they really shouldn't change either.
The only thing I can think of is that I made the hash key
computation a tiny bit faster by omitting unused argtypes[]
entries. That does avoid hashing several hundred bytes
typically, but it's hard to believe it'd amount to any
visible savings overall.
Anyway, PFA v10.
regards, tom lane
[1] https://www.postgresql.org/message-id/CAFj8pRDWDeF2cC%2BpCjLHJno7KnK5kdtjYN-f933RHS7UneArFw%40mail.gmail.com
From 5d2f5e092ef326f72100d6d47ba1b5cb207e62ba Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 29 Mar 2025 14:50:45 -0400
Subject: [PATCH v10 1/6] 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 0394a86c27cf5e82ce0574f951392428f80fe4b7 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 29 Mar 2025 14:51:37 -0400
Subject: [PATCH v10 2/6] Provide a post-rewrite callback hook in plancache.c.
SQL-language functions sometimes want to modify the targetlist of
the query that returns their result. If they're to use the plan
cache, it needs to be possible to do that over again when a
replan occurs. Invent a callback hook to make that happen.
I chose to provide a separate function SetPostRewriteHook to
install such hooks. An alternative API could be to add two
more arguments to CompleteCachedPlan. I didn't do so because
I felt that few callers will want this, but there's a case that
that way would be cleaner.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/8216639.NyiUUSuA9g@aivenlaptop
---
src/backend/utils/cache/plancache.c | 33 +++++++++++++++++++++++++++++
src/include/utils/plancache.h | 8 +++++++
src/tools/pgindent/typedefs.list | 1 +
3 files changed, 42 insertions(+)
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 5983927a4c2..3b681647060 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -219,6 +219,8 @@ CreateCachedPlan(RawStmt *raw_parse_tree,
plansource->num_params = 0;
plansource->parserSetup = NULL;
plansource->parserSetupArg = NULL;
+ plansource->postRewrite = NULL;
+ plansource->postRewriteArg = NULL;
plansource->cursor_options = 0;
plansource->fixed_result = false;
plansource->resultDesc = NULL;
@@ -316,6 +318,8 @@ CreateOneShotCachedPlan(RawStmt *raw_parse_tree,
plansource->num_params = 0;
plansource->parserSetup = NULL;
plansource->parserSetupArg = NULL;
+ plansource->postRewrite = NULL;
+ plansource->postRewriteArg = NULL;
plansource->cursor_options = 0;
plansource->fixed_result = false;
plansource->resultDesc = NULL;
@@ -485,6 +489,29 @@ CompleteCachedPlan(CachedPlanSource *plansource,
plansource->is_valid = true;
}
+/*
+ * SetPostRewriteHook: set a hook to modify post-rewrite query trees
+ *
+ * Some callers have a need to modify the query trees between rewriting and
+ * planning. In the initial call to CompleteCachedPlan, it's assumed such
+ * work was already done on the querytree_list. However, if we're forced
+ * to replan, it will need to be done over. The caller can set this hook
+ * to provide code to make that happen.
+ *
+ * postRewriteArg is just passed verbatim to the hook. As with parserSetupArg,
+ * it is caller's responsibility that the referenced data remains
+ * valid for as long as the CachedPlanSource exists.
+ */
+void
+SetPostRewriteHook(CachedPlanSource *plansource,
+ PostRewriteHook postRewrite,
+ void *postRewriteArg)
+{
+ Assert(plansource->magic == CACHEDPLANSOURCE_MAGIC);
+ plansource->postRewrite = postRewrite;
+ plansource->postRewriteArg = postRewriteArg;
+}
+
/*
* SaveCachedPlan: save a cached plan permanently
*
@@ -813,6 +840,10 @@ RevalidateCachedQuery(CachedPlanSource *plansource,
tlist = NIL;
}
+ /* Apply post-rewrite callback if there is one */
+ if (plansource->postRewrite != NULL)
+ plansource->postRewrite(tlist, plansource->postRewriteArg);
+
/* Release snapshot if we got one */
if (snapshot_set)
PopActiveSnapshot();
@@ -1800,6 +1831,8 @@ CopyCachedPlan(CachedPlanSource *plansource)
newsource->num_params = plansource->num_params;
newsource->parserSetup = plansource->parserSetup;
newsource->parserSetupArg = plansource->parserSetupArg;
+ newsource->postRewrite = plansource->postRewrite;
+ newsource->postRewriteArg = plansource->postRewriteArg;
newsource->cursor_options = plansource->cursor_options;
newsource->fixed_result = plansource->fixed_result;
if (plansource->resultDesc)
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index 5930fcb50f0..07ec5318db7 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -40,6 +40,9 @@ typedef enum
/* GUC parameter */
extern PGDLLIMPORT int plan_cache_mode;
+/* Optional callback to editorialize on rewritten parse trees */
+typedef void (*PostRewriteHook) (List *querytree_list, void *arg);
+
#define CACHEDPLANSOURCE_MAGIC 195726186
#define CACHEDPLAN_MAGIC 953717834
#define CACHEDEXPR_MAGIC 838275847
@@ -112,6 +115,8 @@ typedef struct CachedPlanSource
int num_params; /* length of param_types array */
ParserSetupHook parserSetup; /* alternative parameter spec method */
void *parserSetupArg;
+ PostRewriteHook postRewrite; /* see SetPostRewriteHook */
+ void *postRewriteArg;
int cursor_options; /* cursor options used for planning */
bool fixed_result; /* disallow change in result tupdesc? */
TupleDesc resultDesc; /* result type; NULL = doesn't return tuples */
@@ -223,6 +228,9 @@ extern void CompleteCachedPlan(CachedPlanSource *plansource,
void *parserSetupArg,
int cursor_options,
bool fixed_result);
+extern void SetPostRewriteHook(CachedPlanSource *plansource,
+ PostRewriteHook postRewrite,
+ void *postRewriteArg);
extern void SaveCachedPlan(CachedPlanSource *plansource);
extern void DropCachedPlan(CachedPlanSource *plansource);
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index b66cecd8799..ff75a508876 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2266,6 +2266,7 @@ PortalHashEnt
PortalStatus
PortalStrategy
PostParseColumnRefHook
+PostRewriteHook
PostgresPollingStatusType
PostingItem
PreParseColumnRefHook
--
2.43.5
From 8911e13329809ce2c44aecd82a460e54ad164d25 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 29 Mar 2025 16:56:23 -0400
Subject: [PATCH v10 3/6] Factor out plpgsql's management of its function
cache.
SQL-language functions need precisely this same functionality to
manage a long-lived cache of functions. Rather than duplicating
or reinventing that code, let's split it out into a new module
funccache.c so that it is available for any language that wants
to use it.
This is mostly an exercise in moving and renaming code, and should
not change any behavior. I have added one feature that plpgsql
doesn't use but SQL functions will need: the cache lookup key
can include the output tuple descriptor when the function
returns composite.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/8216639.NyiUUSuA9g@aivenlaptop
---
src/backend/utils/cache/Makefile | 1 +
src/backend/utils/cache/funccache.c | 612 ++++++++++++++++++++++++++++
src/backend/utils/cache/meson.build | 1 +
src/include/utils/funccache.h | 134 ++++++
src/pl/plpgsql/src/pl_comp.c | 433 ++------------------
src/pl/plpgsql/src/pl_funcs.c | 9 +-
src/pl/plpgsql/src/pl_handler.c | 15 +-
src/pl/plpgsql/src/plpgsql.h | 45 +-
src/tools/pgindent/typedefs.list | 5 +
9 files changed, 811 insertions(+), 444 deletions(-)
create mode 100644 src/backend/utils/cache/funccache.c
create mode 100644 src/include/utils/funccache.h
diff --git a/src/backend/utils/cache/Makefile b/src/backend/utils/cache/Makefile
index 5105018cb79..77b3e1a037b 100644
--- a/src/backend/utils/cache/Makefile
+++ b/src/backend/utils/cache/Makefile
@@ -16,6 +16,7 @@ OBJS = \
attoptcache.o \
catcache.o \
evtcache.o \
+ funccache.o \
inval.o \
lsyscache.o \
partcache.o \
diff --git a/src/backend/utils/cache/funccache.c b/src/backend/utils/cache/funccache.c
new file mode 100644
index 00000000000..203d17f2459
--- /dev/null
+++ b/src/backend/utils/cache/funccache.c
@@ -0,0 +1,612 @@
+/*-------------------------------------------------------------------------
+ *
+ * funccache.c
+ * Function cache management.
+ *
+ * funccache.c manages a cache of function execution data. The cache
+ * is used by SQL-language and PL/pgSQL functions, and could be used by
+ * other function languages. Each cache entry is specific to the execution
+ * of a particular function (identified by OID) with specific input data
+ * types; so a polymorphic function could have many associated cache entries.
+ * Trigger functions similarly have a cache entry per trigger. These rules
+ * allow the cached data to be specific to the particular data types the
+ * function call will be dealing with.
+ *
+ *
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ * src/backend/utils/cache/funccache.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "commands/event_trigger.h"
+#include "commands/trigger.h"
+#include "common/hashfn.h"
+#include "funcapi.h"
+#include "catalog/pg_proc.h"
+#include "utils/funccache.h"
+#include "utils/hsearch.h"
+#include "utils/syscache.h"
+
+
+/*
+ * Hash table for cached functions
+ */
+static HTAB *cfunc_hashtable = NULL;
+
+typedef struct CachedFunctionHashEntry
+{
+ CachedFunctionHashKey key; /* hash key, must be first */
+ CachedFunction *function; /* points to data of language-specific size */
+} CachedFunctionHashEntry;
+
+#define FUNCS_PER_USER 128 /* initial table size */
+
+static uint32 cfunc_hash(const void *key, Size keysize);
+static int cfunc_match(const void *key1, const void *key2, Size keysize);
+
+
+/*
+ * Initialize the hash table on first use.
+ *
+ * The hash table will be in TopMemoryContext regardless of caller's context.
+ */
+static void
+cfunc_hashtable_init(void)
+{
+ HASHCTL ctl;
+
+ /* don't allow double-initialization */
+ Assert(cfunc_hashtable == NULL);
+
+ ctl.keysize = sizeof(CachedFunctionHashKey);
+ ctl.entrysize = sizeof(CachedFunctionHashEntry);
+ ctl.hash = cfunc_hash;
+ ctl.match = cfunc_match;
+ cfunc_hashtable = hash_create("Cached function hash",
+ FUNCS_PER_USER,
+ &ctl,
+ HASH_ELEM | HASH_FUNCTION | HASH_COMPARE);
+}
+
+/*
+ * cfunc_hash: hash function for cfunc hash table
+ *
+ * We need special hash and match functions to deal with the optional
+ * presence of a TupleDesc in the hash keys. As long as we have to do
+ * that, we might as well also be smart about not comparing unused
+ * elements of the argtypes arrays.
+ */
+static uint32
+cfunc_hash(const void *key, Size keysize)
+{
+ const CachedFunctionHashKey *k = (const CachedFunctionHashKey *) key;
+ uint32 h;
+
+ Assert(keysize == sizeof(CachedFunctionHashKey));
+ /* Hash all the fixed fields except callResultType */
+ h = DatumGetUInt32(hash_any((const unsigned char *) k,
+ offsetof(CachedFunctionHashKey, callResultType)));
+ /* Incorporate input argument types */
+ if (k->nargs > 0)
+ h = hash_combine(h,
+ DatumGetUInt32(hash_any((const unsigned char *) k->argtypes,
+ k->nargs * sizeof(Oid))));
+ /* Incorporate callResultType if present */
+ if (k->callResultType)
+ h = hash_combine(h, hashRowType(k->callResultType));
+ return h;
+}
+
+/*
+ * cfunc_match: match function to use with cfunc_hash
+ */
+static int
+cfunc_match(const void *key1, const void *key2, Size keysize)
+{
+ const CachedFunctionHashKey *k1 = (const CachedFunctionHashKey *) key1;
+ const CachedFunctionHashKey *k2 = (const CachedFunctionHashKey *) key2;
+
+ Assert(keysize == sizeof(CachedFunctionHashKey));
+ /* Compare all the fixed fields except callResultType */
+ if (memcmp(k1, k2, offsetof(CachedFunctionHashKey, callResultType)) != 0)
+ return 1; /* not equal */
+ /* Compare input argument types (we just verified that nargs matches) */
+ if (k1->nargs > 0 &&
+ memcmp(k1->argtypes, k2->argtypes, k1->nargs * sizeof(Oid)) != 0)
+ return 1; /* not equal */
+ /* Compare callResultType */
+ if (k1->callResultType)
+ {
+ if (k2->callResultType)
+ {
+ if (!equalRowTypes(k1->callResultType, k2->callResultType))
+ return 1; /* not equal */
+ }
+ else
+ return 1; /* not equal */
+ }
+ else
+ {
+ if (k2->callResultType)
+ return 1; /* not equal */
+ }
+ return 0; /* equal */
+}
+
+/*
+ * Look up the CachedFunction for the given hash key.
+ * Returns NULL if not present.
+ */
+static CachedFunction *
+cfunc_hashtable_lookup(CachedFunctionHashKey *func_key)
+{
+ CachedFunctionHashEntry *hentry;
+
+ if (cfunc_hashtable == NULL)
+ return NULL;
+
+ hentry = (CachedFunctionHashEntry *) hash_search(cfunc_hashtable,
+ func_key,
+ HASH_FIND,
+ NULL);
+ if (hentry)
+ return hentry->function;
+ else
+ return NULL;
+}
+
+/*
+ * Insert a hash table entry.
+ */
+static void
+cfunc_hashtable_insert(CachedFunction *function,
+ CachedFunctionHashKey *func_key)
+{
+ CachedFunctionHashEntry *hentry;
+ bool found;
+
+ if (cfunc_hashtable == NULL)
+ cfunc_hashtable_init();
+
+ hentry = (CachedFunctionHashEntry *) hash_search(cfunc_hashtable,
+ func_key,
+ HASH_ENTER,
+ &found);
+ if (found)
+ elog(WARNING, "trying to insert a function that already exists");
+
+ /*
+ * If there's a callResultType, copy it into TopMemoryContext. If we're
+ * unlucky enough for that to fail, leave the entry with null
+ * callResultType, which will probably never match anything.
+ */
+ if (func_key->callResultType)
+ {
+ MemoryContext oldcontext = MemoryContextSwitchTo(TopMemoryContext);
+
+ hentry->key.callResultType = NULL;
+ hentry->key.callResultType = CreateTupleDescCopy(func_key->callResultType);
+ MemoryContextSwitchTo(oldcontext);
+ }
+
+ hentry->function = function;
+
+ /* Set back-link from function to hashtable key */
+ function->fn_hashkey = &hentry->key;
+}
+
+/*
+ * Delete a hash table entry.
+ */
+static void
+cfunc_hashtable_delete(CachedFunction *function)
+{
+ CachedFunctionHashEntry *hentry;
+ TupleDesc tupdesc;
+
+ /* do nothing if not in table */
+ if (function->fn_hashkey == NULL)
+ return;
+
+ /*
+ * We need to free the callResultType if present, which is slightly tricky
+ * because it has to be valid during the hashtable search. Fortunately,
+ * because we have the hashkey back-link, we can grab that pointer before
+ * deleting the hashtable entry.
+ */
+ tupdesc = function->fn_hashkey->callResultType;
+
+ hentry = (CachedFunctionHashEntry *) hash_search(cfunc_hashtable,
+ function->fn_hashkey,
+ HASH_REMOVE,
+ NULL);
+ if (hentry == NULL)
+ elog(WARNING, "trying to delete function that does not exist");
+
+ /* Remove back link, which no longer points to allocated storage */
+ function->fn_hashkey = NULL;
+
+ /* Release the callResultType if present */
+ if (tupdesc)
+ FreeTupleDesc(tupdesc);
+}
+
+/*
+ * Compute the hashkey for a given function invocation
+ *
+ * The hashkey is returned into the caller-provided storage at *hashkey.
+ * Note however that if a callResultType is incorporated, we've not done
+ * anything about copying that.
+ */
+static void
+compute_function_hashkey(FunctionCallInfo fcinfo,
+ Form_pg_proc procStruct,
+ CachedFunctionHashKey *hashkey,
+ Size cacheEntrySize,
+ bool includeResultType,
+ bool forValidator)
+{
+ /* Make sure pad bytes within fixed part of the struct are zero */
+ memset(hashkey, 0, offsetof(CachedFunctionHashKey, argtypes));
+
+ /* get function OID */
+ hashkey->funcOid = fcinfo->flinfo->fn_oid;
+
+ /* get call context */
+ hashkey->isTrigger = CALLED_AS_TRIGGER(fcinfo);
+ hashkey->isEventTrigger = CALLED_AS_EVENT_TRIGGER(fcinfo);
+
+ /* record cacheEntrySize so multiple languages can share hash table */
+ hashkey->cacheEntrySize = cacheEntrySize;
+
+ /*
+ * If DML trigger, include trigger's OID in the hash, so that each trigger
+ * usage gets a different hash entry, allowing for e.g. different relation
+ * rowtypes or transition table names. In validation mode we do not know
+ * what relation or transition table names are intended to be used, so we
+ * leave trigOid zero; the hash entry built in this case will never be
+ * used for any actual calls.
+ *
+ * We don't currently need to distinguish different event trigger usages
+ * in the same way, since the special parameter variables don't vary in
+ * type in that case.
+ */
+ if (hashkey->isTrigger && !forValidator)
+ {
+ TriggerData *trigdata = (TriggerData *) fcinfo->context;
+
+ hashkey->trigOid = trigdata->tg_trigger->tgoid;
+ }
+
+ /* get input collation, if known */
+ hashkey->inputCollation = fcinfo->fncollation;
+
+ /*
+ * We include only input arguments in the hash key, since output argument
+ * types can be deduced from those, and it would require extra cycles to
+ * include the output arguments. But we have to resolve any polymorphic
+ * argument types to the real types for the call.
+ */
+ if (procStruct->pronargs > 0)
+ {
+ hashkey->nargs = procStruct->pronargs;
+ memcpy(hashkey->argtypes, procStruct->proargtypes.values,
+ procStruct->pronargs * sizeof(Oid));
+ cfunc_resolve_polymorphic_argtypes(procStruct->pronargs,
+ hashkey->argtypes,
+ NULL, /* all args are inputs */
+ fcinfo->flinfo->fn_expr,
+ forValidator,
+ NameStr(procStruct->proname));
+ }
+
+ /*
+ * While regular OUT arguments are sufficiently represented by the
+ * resolved input arguments, a function returning composite has additional
+ * variability: ALTER TABLE/ALTER TYPE could affect what it returns. Also,
+ * a function returning RECORD may depend on a column definition list to
+ * determine its output rowtype. If the caller needs the exact result
+ * type to be part of the hash lookup key, we must run
+ * get_call_result_type() to find that out.
+ */
+ if (includeResultType)
+ {
+ Oid resultTypeId;
+ TupleDesc tupdesc;
+
+ switch (get_call_result_type(fcinfo, &resultTypeId, &tupdesc))
+ {
+ case TYPEFUNC_COMPOSITE:
+ case TYPEFUNC_COMPOSITE_DOMAIN:
+ hashkey->callResultType = tupdesc;
+ break;
+ default:
+ /* scalar result, or indeterminate rowtype */
+ break;
+ }
+ }
+}
+
+/*
+ * This is the same as the standard resolve_polymorphic_argtypes() function,
+ * except that:
+ * 1. We go ahead and report the error if we can't resolve the types.
+ * 2. We treat RECORD-type input arguments (not output arguments) as if
+ * they were polymorphic, replacing their types with the actual input
+ * types if we can determine those. This allows us to create a separate
+ * function cache entry for each named composite type passed to such an
+ * argument.
+ * 3. In validation mode, we have no inputs to look at, so assume that
+ * polymorphic arguments are integer, integer-array or integer-range.
+ */
+void
+cfunc_resolve_polymorphic_argtypes(int numargs,
+ Oid *argtypes, char *argmodes,
+ Node *call_expr, bool forValidator,
+ const char *proname)
+{
+ int i;
+
+ if (!forValidator)
+ {
+ int inargno;
+
+ /* normal case, pass to standard routine */
+ if (!resolve_polymorphic_argtypes(numargs, argtypes, argmodes,
+ call_expr))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("could not determine actual argument "
+ "type for polymorphic function \"%s\"",
+ proname)));
+ /* also, treat RECORD inputs (but not outputs) as polymorphic */
+ inargno = 0;
+ for (i = 0; i < numargs; i++)
+ {
+ char argmode = argmodes ? argmodes[i] : PROARGMODE_IN;
+
+ if (argmode == PROARGMODE_OUT || argmode == PROARGMODE_TABLE)
+ continue;
+ if (argtypes[i] == RECORDOID || argtypes[i] == RECORDARRAYOID)
+ {
+ Oid resolvedtype = get_call_expr_argtype(call_expr,
+ inargno);
+
+ if (OidIsValid(resolvedtype))
+ argtypes[i] = resolvedtype;
+ }
+ inargno++;
+ }
+ }
+ else
+ {
+ /* special validation case (no need to do anything for RECORD) */
+ for (i = 0; i < numargs; i++)
+ {
+ switch (argtypes[i])
+ {
+ case ANYELEMENTOID:
+ case ANYNONARRAYOID:
+ case ANYENUMOID: /* XXX dubious */
+ case ANYCOMPATIBLEOID:
+ case ANYCOMPATIBLENONARRAYOID:
+ argtypes[i] = INT4OID;
+ break;
+ case ANYARRAYOID:
+ case ANYCOMPATIBLEARRAYOID:
+ argtypes[i] = INT4ARRAYOID;
+ break;
+ case ANYRANGEOID:
+ case ANYCOMPATIBLERANGEOID:
+ argtypes[i] = INT4RANGEOID;
+ break;
+ case ANYMULTIRANGEOID:
+ argtypes[i] = INT4MULTIRANGEOID;
+ break;
+ default:
+ break;
+ }
+ }
+ }
+}
+
+/*
+ * delete_function - clean up as much as possible of a stale function cache
+ *
+ * We can't release the CachedFunction struct itself, because of the
+ * possibility that there are fn_extra pointers to it. We can release
+ * the subsidiary storage, but only if there are no active evaluations
+ * in progress. Otherwise we'll just leak that storage. Since the
+ * case would only occur if a pg_proc update is detected during a nested
+ * recursive call on the function, a leak seems acceptable.
+ *
+ * Note that this can be called more than once if there are multiple fn_extra
+ * pointers to the same function cache. Hence be careful not to do things
+ * twice.
+ */
+static void
+delete_function(CachedFunction *func)
+{
+ /* remove function from hash table (might be done already) */
+ cfunc_hashtable_delete(func);
+
+ /* release the function's storage if safe and not done already */
+ if (func->use_count == 0 &&
+ func->dcallback != NULL)
+ {
+ func->dcallback(func);
+ func->dcallback = NULL;
+ }
+}
+
+/*
+ * Compile a cached function, if no existing cache entry is suitable.
+ *
+ * fcinfo is the current call information.
+ *
+ * function should be NULL or the result of a previous call of
+ * cached_function_compile() for the same fcinfo. The caller will
+ * typically save the result in fcinfo->flinfo->fn_extra, or in a
+ * field of a struct pointed to by fn_extra, to re-use in later
+ * calls within the same query.
+ *
+ * ccallback and dcallback are function-language-specific callbacks to
+ * compile and delete a cached function entry. dcallback can be NULL
+ * if there's nothing for it to do.
+ *
+ * cacheEntrySize is the function-language-specific size of the cache entry
+ * (which embeds a CachedFunction struct and typically has many more fields
+ * after that).
+ *
+ * If includeResultType is true and the function returns composite,
+ * include the actual result descriptor in the cache lookup key.
+ *
+ * If forValidator is true, we're only compiling for validation purposes,
+ * and so some checks are skipped.
+ *
+ * Note: it's important for this to fall through quickly if the function
+ * has already been compiled.
+ *
+ * Note: this function leaves the "use_count" field as zero. The caller
+ * is expected to increment the use_count and decrement it when done with
+ * the cache entry.
+ */
+CachedFunction *
+cached_function_compile(FunctionCallInfo fcinfo,
+ CachedFunction *function,
+ CachedFunctionCompileCallback ccallback,
+ CachedFunctionDeleteCallback dcallback,
+ Size cacheEntrySize,
+ bool includeResultType,
+ bool forValidator)
+{
+ Oid funcOid = fcinfo->flinfo->fn_oid;
+ HeapTuple procTup;
+ Form_pg_proc procStruct;
+ CachedFunctionHashKey hashkey;
+ bool function_valid = false;
+ bool hashkey_valid = false;
+
+ /*
+ * Lookup the pg_proc tuple by Oid; we'll need it in any case
+ */
+ procTup = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcOid));
+ if (!HeapTupleIsValid(procTup))
+ elog(ERROR, "cache lookup failed for function %u", funcOid);
+ procStruct = (Form_pg_proc) GETSTRUCT(procTup);
+
+ /*
+ * Do we already have a cache entry for the current FmgrInfo? If not, try
+ * to find one in the hash table.
+ */
+recheck:
+ if (!function)
+ {
+ /* Compute hashkey using function signature and actual arg types */
+ compute_function_hashkey(fcinfo, procStruct, &hashkey,
+ cacheEntrySize, includeResultType,
+ forValidator);
+ hashkey_valid = true;
+
+ /* And do the lookup */
+ function = cfunc_hashtable_lookup(&hashkey);
+ }
+
+ if (function)
+ {
+ /* We have a compiled function, but is it still valid? */
+ if (function->fn_xmin == HeapTupleHeaderGetRawXmin(procTup->t_data) &&
+ ItemPointerEquals(&function->fn_tid, &procTup->t_self))
+ function_valid = true;
+ else
+ {
+ /*
+ * Nope, so remove it from hashtable and try to drop associated
+ * storage (if not done already).
+ */
+ delete_function(function);
+
+ /*
+ * If the function isn't in active use then we can overwrite the
+ * func struct with new data, allowing any other existing fn_extra
+ * pointers to make use of the new definition on their next use.
+ * If it is in use then just leave it alone and make a new one.
+ * (The active invocations will run to completion using the
+ * previous definition, and then the cache entry will just be
+ * leaked; doesn't seem worth adding code to clean it up, given
+ * what a corner case this is.)
+ *
+ * If we found the function struct via fn_extra then it's possible
+ * a replacement has already been made, so go back and recheck the
+ * hashtable.
+ */
+ if (function->use_count != 0)
+ {
+ function = NULL;
+ if (!hashkey_valid)
+ goto recheck;
+ }
+ }
+ }
+
+ /*
+ * If the function wasn't found or was out-of-date, we have to compile it.
+ */
+ if (!function_valid)
+ {
+ /*
+ * Calculate hashkey if we didn't already; we'll need it to store the
+ * completed function.
+ */
+ if (!hashkey_valid)
+ compute_function_hashkey(fcinfo, procStruct, &hashkey,
+ cacheEntrySize, includeResultType,
+ forValidator);
+
+ /*
+ * Create the new function struct, if not done already. The function
+ * structs are never thrown away, so keep them in TopMemoryContext.
+ */
+ Assert(cacheEntrySize >= sizeof(CachedFunction));
+ if (function == NULL)
+ {
+ function = (CachedFunction *)
+ MemoryContextAllocZero(TopMemoryContext, cacheEntrySize);
+ }
+ else
+ {
+ /* re-using a previously existing struct, so clear it out */
+ memset(function, 0, cacheEntrySize);
+ }
+
+ /*
+ * Fill in the CachedFunction part. fn_hashkey and use_count remain
+ * zeroes for now.
+ */
+ function->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
+ function->fn_tid = procTup->t_self;
+ function->dcallback = dcallback;
+
+ /*
+ * Do the hard, language-specific part.
+ */
+ ccallback(fcinfo, procTup, &hashkey, function, forValidator);
+
+ /*
+ * Add the completed struct to the hash table.
+ */
+ cfunc_hashtable_insert(function, &hashkey);
+ }
+
+ ReleaseSysCache(procTup);
+
+ /*
+ * Finally return the compiled function
+ */
+ return function;
+}
diff --git a/src/backend/utils/cache/meson.build b/src/backend/utils/cache/meson.build
index 104b28737d7..a1784dce585 100644
--- a/src/backend/utils/cache/meson.build
+++ b/src/backend/utils/cache/meson.build
@@ -4,6 +4,7 @@ backend_sources += files(
'attoptcache.c',
'catcache.c',
'evtcache.c',
+ 'funccache.c',
'inval.c',
'lsyscache.c',
'partcache.c',
diff --git a/src/include/utils/funccache.h b/src/include/utils/funccache.h
new file mode 100644
index 00000000000..e0112ebfa11
--- /dev/null
+++ b/src/include/utils/funccache.h
@@ -0,0 +1,134 @@
+/*-------------------------------------------------------------------------
+ *
+ * funccache.h
+ * Function cache definitions.
+ *
+ * See funccache.c for comments.
+ *
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/utils/funccache.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef FUNCCACHE_H
+#define FUNCCACHE_H
+
+#include "access/htup_details.h"
+#include "fmgr.h"
+#include "storage/itemptr.h"
+
+struct CachedFunctionHashKey; /* forward references */
+struct CachedFunction;
+
+/*
+ * Callback that cached_function_compile() invokes when it's necessary to
+ * compile a cached function. The callback must fill in *function (except
+ * for the fields of struct CachedFunction), or throw an error if trouble.
+ * fcinfo: current call information
+ * procTup: function's pg_proc row from catcache
+ * hashkey: hash key that will be used for the function
+ * function: pre-zeroed workspace, of size passed to cached_function_compile()
+ * forValidator: passed through from cached_function_compile()
+ */
+typedef void (*CachedFunctionCompileCallback) (FunctionCallInfo fcinfo,
+ HeapTuple procTup,
+ const struct CachedFunctionHashKey *hashkey,
+ struct CachedFunction *function,
+ bool forValidator);
+
+/*
+ * Callback called when discarding a cache entry. Free any free-able
+ * subsidiary data of cfunc, but not the struct CachedFunction itself.
+ */
+typedef void (*CachedFunctionDeleteCallback) (struct CachedFunction *cfunc);
+
+/*
+ * Hash lookup key for functions. This must account for all aspects
+ * of a specific call that might lead to different data types or
+ * collations being used within the function.
+ */
+typedef struct CachedFunctionHashKey
+{
+ Oid funcOid;
+
+ bool isTrigger; /* true if called as a DML trigger */
+ bool isEventTrigger; /* true if called as an event trigger */
+
+ /* be careful that pad bytes in this struct get zeroed! */
+
+ /*
+ * We include the language-specific size of the function's cache entry in
+ * the cache key. This covers the case where CREATE OR REPLACE FUNCTION
+ * is used to change the implementation language, and the new language
+ * also uses funccache.c but needs a different-sized cache entry.
+ */
+ Size cacheEntrySize;
+
+ /*
+ * For a trigger function, the OID of the trigger is part of the hash key
+ * --- we want to compile the trigger function separately for each trigger
+ * it is used with, in case the rowtype or transition table names are
+ * different. Zero if not called as a DML trigger.
+ */
+ Oid trigOid;
+
+ /*
+ * We must include the input collation as part of the hash key too,
+ * because we have to generate different plans (with different Param
+ * collations) for different collation settings.
+ */
+ Oid inputCollation;
+
+ /* Number of arguments (counting input arguments only, ie pronargs) */
+ int nargs;
+
+ /* If you change anything below here, fix hashing code in funccache.c! */
+
+ /*
+ * If relevant, the result descriptor for a function returning composite.
+ */
+ TupleDesc callResultType;
+
+ /*
+ * Input argument types, with any polymorphic types resolved to actual
+ * types. Only the first nargs entries are valid.
+ */
+ Oid argtypes[FUNC_MAX_ARGS];
+} CachedFunctionHashKey;
+
+/*
+ * Representation of a compiled function. This struct contains just the
+ * fields that funccache.c needs to deal with. It will typically be
+ * embedded in a larger struct containing function-language-specific data.
+ */
+typedef struct CachedFunction
+{
+ /* back-link to hashtable entry, or NULL if not in hash table */
+ CachedFunctionHashKey *fn_hashkey;
+ /* xmin and ctid of function's pg_proc row; used to detect invalidation */
+ TransactionId fn_xmin;
+ ItemPointerData fn_tid;
+ /* deletion callback */
+ CachedFunctionDeleteCallback dcallback;
+
+ /* this field changes when the function is used: */
+ uint64 use_count;
+} CachedFunction;
+
+extern CachedFunction *cached_function_compile(FunctionCallInfo fcinfo,
+ CachedFunction *function,
+ CachedFunctionCompileCallback ccallback,
+ CachedFunctionDeleteCallback dcallback,
+ Size cacheEntrySize,
+ bool includeResultType,
+ bool forValidator);
+extern void cfunc_resolve_polymorphic_argtypes(int numargs,
+ Oid *argtypes,
+ char *argmodes,
+ Node *call_expr,
+ bool forValidator,
+ const char *proname);
+
+#endif /* FUNCCACHE_H */
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 6fdba95962d..1a091d0c55f 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -52,20 +52,6 @@ PLpgSQL_function *plpgsql_curr_compile;
/* A context appropriate for short-term allocs during compilation */
MemoryContext plpgsql_compile_tmp_cxt;
-/* ----------
- * Hash table for compiled functions
- * ----------
- */
-static HTAB *plpgsql_HashTable = NULL;
-
-typedef struct plpgsql_hashent
-{
- PLpgSQL_func_hashkey key;
- PLpgSQL_function *function;
-} plpgsql_HashEnt;
-
-#define FUNCS_PER_USER 128 /* initial table size */
-
/* ----------
* Lookup table for EXCEPTION condition names
* ----------
@@ -86,11 +72,11 @@ static const ExceptionLabelMap exception_label_map[] = {
* static prototypes
* ----------
*/
-static PLpgSQL_function *do_compile(FunctionCallInfo fcinfo,
- HeapTuple procTup,
- PLpgSQL_function *function,
- PLpgSQL_func_hashkey *hashkey,
- bool forValidator);
+static void plpgsql_compile_callback(FunctionCallInfo fcinfo,
+ HeapTuple procTup,
+ const CachedFunctionHashKey *hashkey,
+ CachedFunction *cfunc,
+ bool forValidator);
static void plpgsql_compile_error_callback(void *arg);
static void add_parameter_name(PLpgSQL_nsitem_type itemtype, int itemno, const char *name);
static void add_dummy_return(PLpgSQL_function *function);
@@ -105,19 +91,6 @@ static PLpgSQL_type *build_datatype(HeapTuple typeTup, int32 typmod,
Oid collation, TypeName *origtypname);
static void plpgsql_start_datums(void);
static void plpgsql_finish_datums(PLpgSQL_function *function);
-static void compute_function_hashkey(FunctionCallInfo fcinfo,
- Form_pg_proc procStruct,
- PLpgSQL_func_hashkey *hashkey,
- bool forValidator);
-static void plpgsql_resolve_polymorphic_argtypes(int numargs,
- Oid *argtypes, char *argmodes,
- Node *call_expr, bool forValidator,
- const char *proname);
-static PLpgSQL_function *plpgsql_HashTableLookup(PLpgSQL_func_hashkey *func_key);
-static void plpgsql_HashTableInsert(PLpgSQL_function *function,
- PLpgSQL_func_hashkey *func_key);
-static void plpgsql_HashTableDelete(PLpgSQL_function *function);
-static void delete_function(PLpgSQL_function *func);
/* ----------
* plpgsql_compile Make an execution tree for a PL/pgSQL function.
@@ -132,97 +105,24 @@ static void delete_function(PLpgSQL_function *func);
PLpgSQL_function *
plpgsql_compile(FunctionCallInfo fcinfo, bool forValidator)
{
- Oid funcOid = fcinfo->flinfo->fn_oid;
- HeapTuple procTup;
- Form_pg_proc procStruct;
PLpgSQL_function *function;
- PLpgSQL_func_hashkey hashkey;
- bool function_valid = false;
- bool hashkey_valid = false;
-
- /*
- * Lookup the pg_proc tuple by Oid; we'll need it in any case
- */
- procTup = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcOid));
- if (!HeapTupleIsValid(procTup))
- elog(ERROR, "cache lookup failed for function %u", funcOid);
- procStruct = (Form_pg_proc) GETSTRUCT(procTup);
-
- /*
- * See if there's already a cache entry for the current FmgrInfo. If not,
- * try to find one in the hash table.
- */
- function = (PLpgSQL_function *) fcinfo->flinfo->fn_extra;
-
-recheck:
- if (!function)
- {
- /* Compute hashkey using function signature and actual arg types */
- compute_function_hashkey(fcinfo, procStruct, &hashkey, forValidator);
- hashkey_valid = true;
-
- /* And do the lookup */
- function = plpgsql_HashTableLookup(&hashkey);
- }
-
- if (function)
- {
- /* We have a compiled function, but is it still valid? */
- if (function->fn_xmin == HeapTupleHeaderGetRawXmin(procTup->t_data) &&
- ItemPointerEquals(&function->fn_tid, &procTup->t_self))
- function_valid = true;
- else
- {
- /*
- * Nope, so remove it from hashtable and try to drop associated
- * storage (if not done already).
- */
- delete_function(function);
-
- /*
- * If the function isn't in active use then we can overwrite the
- * func struct with new data, allowing any other existing fn_extra
- * pointers to make use of the new definition on their next use.
- * If it is in use then just leave it alone and make a new one.
- * (The active invocations will run to completion using the
- * previous definition, and then the cache entry will just be
- * leaked; doesn't seem worth adding code to clean it up, given
- * what a corner case this is.)
- *
- * If we found the function struct via fn_extra then it's possible
- * a replacement has already been made, so go back and recheck the
- * hashtable.
- */
- if (function->use_count != 0)
- {
- function = NULL;
- if (!hashkey_valid)
- goto recheck;
- }
- }
- }
/*
- * If the function wasn't found or was out-of-date, we have to compile it
+ * funccache.c manages re-use of existing PLpgSQL_function caches.
+ *
+ * In PL/pgSQL we use fn_extra directly as the pointer to the long-lived
+ * function cache entry; we have no need for any query-lifespan cache.
+ * Also, we don't need to make the cache key depend on composite result
+ * type (at least for now).
*/
- if (!function_valid)
- {
- /*
- * Calculate hashkey if we didn't already; we'll need it to store the
- * completed function.
- */
- if (!hashkey_valid)
- compute_function_hashkey(fcinfo, procStruct, &hashkey,
- forValidator);
-
- /*
- * Do the hard part.
- */
- function = do_compile(fcinfo, procTup, function,
- &hashkey, forValidator);
- }
-
- ReleaseSysCache(procTup);
+ function = (PLpgSQL_function *)
+ cached_function_compile(fcinfo,
+ fcinfo->flinfo->fn_extra,
+ plpgsql_compile_callback,
+ plpgsql_delete_callback,
+ sizeof(PLpgSQL_function),
+ false,
+ forValidator);
/*
* Save pointer in FmgrInfo to avoid search on subsequent calls
@@ -244,8 +144,8 @@ struct compile_error_callback_arg
/*
* This is the slow part of plpgsql_compile().
*
- * The passed-in "function" pointer is either NULL or an already-allocated
- * function struct to overwrite.
+ * The passed-in "cfunc" struct is expected to be zeroes, except
+ * for the CachedFunction fields, which we don't touch here.
*
* While compiling a function, the CurrentMemoryContext is the
* per-function memory context of the function we are compiling. That
@@ -263,13 +163,14 @@ struct compile_error_callback_arg
* NB: this code is not re-entrant. We assume that nothing we do here could
* result in the invocation of another plpgsql function.
*/
-static PLpgSQL_function *
-do_compile(FunctionCallInfo fcinfo,
- HeapTuple procTup,
- PLpgSQL_function *function,
- PLpgSQL_func_hashkey *hashkey,
- bool forValidator)
+static void
+plpgsql_compile_callback(FunctionCallInfo fcinfo,
+ HeapTuple procTup,
+ const CachedFunctionHashKey *hashkey,
+ CachedFunction *cfunc,
+ bool forValidator)
{
+ PLpgSQL_function *function = (PLpgSQL_function *) cfunc;
Form_pg_proc procStruct = (Form_pg_proc) GETSTRUCT(procTup);
bool is_dml_trigger = CALLED_AS_TRIGGER(fcinfo);
bool is_event_trigger = CALLED_AS_EVENT_TRIGGER(fcinfo);
@@ -320,21 +221,6 @@ do_compile(FunctionCallInfo fcinfo,
* reasons.
*/
plpgsql_check_syntax = forValidator;
-
- /*
- * Create the new function struct, if not done already. The function
- * structs are never thrown away, so keep them in TopMemoryContext.
- */
- if (function == NULL)
- {
- function = (PLpgSQL_function *)
- MemoryContextAllocZero(TopMemoryContext, sizeof(PLpgSQL_function));
- }
- else
- {
- /* re-using a previously existing struct, so clear it out */
- memset(function, 0, sizeof(PLpgSQL_function));
- }
plpgsql_curr_compile = function;
/*
@@ -349,8 +235,6 @@ do_compile(FunctionCallInfo fcinfo,
function->fn_signature = format_procedure(fcinfo->flinfo->fn_oid);
MemoryContextSetIdentifier(func_cxt, function->fn_signature);
function->fn_oid = fcinfo->flinfo->fn_oid;
- function->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
- function->fn_tid = procTup->t_self;
function->fn_input_collation = fcinfo->fncollation;
function->fn_cxt = func_cxt;
function->out_param_varno = -1; /* set up for no OUT param */
@@ -400,10 +284,14 @@ do_compile(FunctionCallInfo fcinfo,
numargs = get_func_arg_info(procTup,
&argtypes, &argnames, &argmodes);
- plpgsql_resolve_polymorphic_argtypes(numargs, argtypes, argmodes,
- fcinfo->flinfo->fn_expr,
- forValidator,
- plpgsql_error_funcname);
+ /*
+ * XXX can't we get rid of this in favor of using funccache.c's
+ * results? But why are we considering argmodes here not there??
+ */
+ cfunc_resolve_polymorphic_argtypes(numargs, argtypes, argmodes,
+ fcinfo->flinfo->fn_expr,
+ forValidator,
+ plpgsql_error_funcname);
in_arg_varnos = (int *) palloc(numargs * sizeof(int));
out_arg_variables = (PLpgSQL_variable **) palloc(numargs * sizeof(PLpgSQL_variable *));
@@ -819,11 +707,6 @@ do_compile(FunctionCallInfo fcinfo,
if (plpgsql_DumpExecTree)
plpgsql_dumptree(function);
- /*
- * add it to the hash table
- */
- plpgsql_HashTableInsert(function, hashkey);
-
/*
* Pop the error context stack
*/
@@ -834,14 +717,13 @@ do_compile(FunctionCallInfo fcinfo,
MemoryContextSwitchTo(plpgsql_compile_tmp_cxt);
plpgsql_compile_tmp_cxt = NULL;
- return function;
}
/* ----------
* plpgsql_compile_inline Make an execution tree for an anonymous code block.
*
- * Note: this is generally parallel to do_compile(); is it worth trying to
- * merge the two?
+ * Note: this is generally parallel to plpgsql_compile_callback(); is it worth
+ * trying to merge the two?
*
* Note: we assume the block will be thrown away so there is no need to build
* persistent data structures.
@@ -2437,242 +2319,3 @@ plpgsql_add_initdatums(int **varnos)
datums_last = plpgsql_nDatums;
return n;
}
-
-
-/*
- * Compute the hashkey for a given function invocation
- *
- * The hashkey is returned into the caller-provided storage at *hashkey.
- */
-static void
-compute_function_hashkey(FunctionCallInfo fcinfo,
- Form_pg_proc procStruct,
- PLpgSQL_func_hashkey *hashkey,
- bool forValidator)
-{
- /* Make sure any unused bytes of the struct are zero */
- MemSet(hashkey, 0, sizeof(PLpgSQL_func_hashkey));
-
- /* get function OID */
- hashkey->funcOid = fcinfo->flinfo->fn_oid;
-
- /* get call context */
- hashkey->isTrigger = CALLED_AS_TRIGGER(fcinfo);
- hashkey->isEventTrigger = CALLED_AS_EVENT_TRIGGER(fcinfo);
-
- /*
- * If DML trigger, include trigger's OID in the hash, so that each trigger
- * usage gets a different hash entry, allowing for e.g. different relation
- * rowtypes or transition table names. In validation mode we do not know
- * what relation or transition table names are intended to be used, so we
- * leave trigOid zero; the hash entry built in this case will never be
- * used for any actual calls.
- *
- * We don't currently need to distinguish different event trigger usages
- * in the same way, since the special parameter variables don't vary in
- * type in that case.
- */
- if (hashkey->isTrigger && !forValidator)
- {
- TriggerData *trigdata = (TriggerData *) fcinfo->context;
-
- hashkey->trigOid = trigdata->tg_trigger->tgoid;
- }
-
- /* get input collation, if known */
- hashkey->inputCollation = fcinfo->fncollation;
-
- if (procStruct->pronargs > 0)
- {
- /* get the argument types */
- memcpy(hashkey->argtypes, procStruct->proargtypes.values,
- procStruct->pronargs * sizeof(Oid));
-
- /* resolve any polymorphic argument types */
- plpgsql_resolve_polymorphic_argtypes(procStruct->pronargs,
- hashkey->argtypes,
- NULL,
- fcinfo->flinfo->fn_expr,
- forValidator,
- NameStr(procStruct->proname));
- }
-}
-
-/*
- * This is the same as the standard resolve_polymorphic_argtypes() function,
- * except that:
- * 1. We go ahead and report the error if we can't resolve the types.
- * 2. We treat RECORD-type input arguments (not output arguments) as if
- * they were polymorphic, replacing their types with the actual input
- * types if we can determine those. This allows us to create a separate
- * function cache entry for each named composite type passed to such an
- * argument.
- * 3. In validation mode, we have no inputs to look at, so assume that
- * polymorphic arguments are integer, integer-array or integer-range.
- */
-static void
-plpgsql_resolve_polymorphic_argtypes(int numargs,
- Oid *argtypes, char *argmodes,
- Node *call_expr, bool forValidator,
- const char *proname)
-{
- int i;
-
- if (!forValidator)
- {
- int inargno;
-
- /* normal case, pass to standard routine */
- if (!resolve_polymorphic_argtypes(numargs, argtypes, argmodes,
- call_expr))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("could not determine actual argument "
- "type for polymorphic function \"%s\"",
- proname)));
- /* also, treat RECORD inputs (but not outputs) as polymorphic */
- inargno = 0;
- for (i = 0; i < numargs; i++)
- {
- char argmode = argmodes ? argmodes[i] : PROARGMODE_IN;
-
- if (argmode == PROARGMODE_OUT || argmode == PROARGMODE_TABLE)
- continue;
- if (argtypes[i] == RECORDOID || argtypes[i] == RECORDARRAYOID)
- {
- Oid resolvedtype = get_call_expr_argtype(call_expr,
- inargno);
-
- if (OidIsValid(resolvedtype))
- argtypes[i] = resolvedtype;
- }
- inargno++;
- }
- }
- else
- {
- /* special validation case (no need to do anything for RECORD) */
- for (i = 0; i < numargs; i++)
- {
- switch (argtypes[i])
- {
- case ANYELEMENTOID:
- case ANYNONARRAYOID:
- case ANYENUMOID: /* XXX dubious */
- case ANYCOMPATIBLEOID:
- case ANYCOMPATIBLENONARRAYOID:
- argtypes[i] = INT4OID;
- break;
- case ANYARRAYOID:
- case ANYCOMPATIBLEARRAYOID:
- argtypes[i] = INT4ARRAYOID;
- break;
- case ANYRANGEOID:
- case ANYCOMPATIBLERANGEOID:
- argtypes[i] = INT4RANGEOID;
- break;
- case ANYMULTIRANGEOID:
- argtypes[i] = INT4MULTIRANGEOID;
- break;
- default:
- break;
- }
- }
- }
-}
-
-/*
- * delete_function - clean up as much as possible of a stale function cache
- *
- * We can't release the PLpgSQL_function struct itself, because of the
- * possibility that there are fn_extra pointers to it. We can release
- * the subsidiary storage, but only if there are no active evaluations
- * in progress. Otherwise we'll just leak that storage. Since the
- * case would only occur if a pg_proc update is detected during a nested
- * recursive call on the function, a leak seems acceptable.
- *
- * Note that this can be called more than once if there are multiple fn_extra
- * pointers to the same function cache. Hence be careful not to do things
- * twice.
- */
-static void
-delete_function(PLpgSQL_function *func)
-{
- /* remove function from hash table (might be done already) */
- plpgsql_HashTableDelete(func);
-
- /* release the function's storage if safe and not done already */
- if (func->use_count == 0)
- plpgsql_free_function_memory(func);
-}
-
-/* exported so we can call it from _PG_init() */
-void
-plpgsql_HashTableInit(void)
-{
- HASHCTL ctl;
-
- /* don't allow double-initialization */
- Assert(plpgsql_HashTable == NULL);
-
- ctl.keysize = sizeof(PLpgSQL_func_hashkey);
- ctl.entrysize = sizeof(plpgsql_HashEnt);
- plpgsql_HashTable = hash_create("PLpgSQL function hash",
- FUNCS_PER_USER,
- &ctl,
- HASH_ELEM | HASH_BLOBS);
-}
-
-static PLpgSQL_function *
-plpgsql_HashTableLookup(PLpgSQL_func_hashkey *func_key)
-{
- plpgsql_HashEnt *hentry;
-
- hentry = (plpgsql_HashEnt *) hash_search(plpgsql_HashTable,
- func_key,
- HASH_FIND,
- NULL);
- if (hentry)
- return hentry->function;
- else
- return NULL;
-}
-
-static void
-plpgsql_HashTableInsert(PLpgSQL_function *function,
- PLpgSQL_func_hashkey *func_key)
-{
- plpgsql_HashEnt *hentry;
- bool found;
-
- hentry = (plpgsql_HashEnt *) hash_search(plpgsql_HashTable,
- func_key,
- HASH_ENTER,
- &found);
- if (found)
- elog(WARNING, "trying to insert a function that already exists");
-
- hentry->function = function;
- /* prepare back link from function to hashtable key */
- function->fn_hashkey = &hentry->key;
-}
-
-static void
-plpgsql_HashTableDelete(PLpgSQL_function *function)
-{
- plpgsql_HashEnt *hentry;
-
- /* do nothing if not in table */
- if (function->fn_hashkey == NULL)
- return;
-
- hentry = (plpgsql_HashEnt *) hash_search(plpgsql_HashTable,
- function->fn_hashkey,
- HASH_REMOVE,
- NULL);
- if (hentry == NULL)
- elog(WARNING, "trying to delete function that does not exist");
-
- /* remove back link, which no longer points to allocated storage */
- function->fn_hashkey = NULL;
-}
diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c
index 6b5394fc5fa..bc7a61feb4d 100644
--- a/src/pl/plpgsql/src/pl_funcs.c
+++ b/src/pl/plpgsql/src/pl_funcs.c
@@ -718,7 +718,7 @@ plpgsql_free_function_memory(PLpgSQL_function *func)
int i;
/* Better not call this on an in-use function */
- Assert(func->use_count == 0);
+ Assert(func->cfunc.use_count == 0);
/* Release plans associated with variable declarations */
for (i = 0; i < func->ndatums; i++)
@@ -767,6 +767,13 @@ plpgsql_free_function_memory(PLpgSQL_function *func)
func->fn_cxt = NULL;
}
+/* Deletion callback used by funccache.c */
+void
+plpgsql_delete_callback(CachedFunction *cfunc)
+{
+ plpgsql_free_function_memory((PLpgSQL_function *) cfunc);
+}
+
/**********************************************************************
* Debug functions for analyzing the compiled code
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index 1bf12232862..e9a72929947 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -202,7 +202,6 @@ _PG_init(void)
MarkGUCPrefixReserved("plpgsql");
- plpgsql_HashTableInit();
RegisterXactCallback(plpgsql_xact_cb, NULL);
RegisterSubXactCallback(plpgsql_subxact_cb, NULL);
@@ -247,7 +246,7 @@ plpgsql_call_handler(PG_FUNCTION_ARGS)
save_cur_estate = func->cur_estate;
/* Mark the function as busy, so it can't be deleted from under us */
- func->use_count++;
+ func->cfunc.use_count++;
/*
* If we'll need a procedure-lifespan resowner to execute any CALL or DO
@@ -284,7 +283,7 @@ plpgsql_call_handler(PG_FUNCTION_ARGS)
PG_FINALLY();
{
/* Decrement use-count, restore cur_estate */
- func->use_count--;
+ func->cfunc.use_count--;
func->cur_estate = save_cur_estate;
/* Be sure to release the procedure resowner if any */
@@ -334,7 +333,7 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS)
func = plpgsql_compile_inline(codeblock->source_text);
/* Mark the function as busy, just pro forma */
- func->use_count++;
+ func->cfunc.use_count++;
/*
* Set up a fake fcinfo with just enough info to satisfy
@@ -398,8 +397,8 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS)
ResourceOwnerDelete(simple_eval_resowner);
/* Function should now have no remaining use-counts ... */
- func->use_count--;
- Assert(func->use_count == 0);
+ func->cfunc.use_count--;
+ Assert(func->cfunc.use_count == 0);
/* ... so we can free subsidiary storage */
plpgsql_free_function_memory(func);
@@ -415,8 +414,8 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS)
ResourceOwnerDelete(simple_eval_resowner);
/* Function should now have no remaining use-counts ... */
- func->use_count--;
- Assert(func->use_count == 0);
+ func->cfunc.use_count--;
+ Assert(func->cfunc.use_count == 0);
/* ... so we can free subsidiary storage */
plpgsql_free_function_memory(func);
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index b67847b5111..41e52b8ce71 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -21,6 +21,7 @@
#include "commands/trigger.h"
#include "executor/spi.h"
#include "utils/expandedrecord.h"
+#include "utils/funccache.h"
#include "utils/typcache.h"
@@ -941,40 +942,6 @@ typedef struct PLpgSQL_stmt_dynexecute
List *params; /* USING expressions */
} PLpgSQL_stmt_dynexecute;
-/*
- * Hash lookup key for functions
- */
-typedef struct PLpgSQL_func_hashkey
-{
- Oid funcOid;
-
- bool isTrigger; /* true if called as a DML trigger */
- bool isEventTrigger; /* true if called as an event trigger */
-
- /* be careful that pad bytes in this struct get zeroed! */
-
- /*
- * For a trigger function, the OID of the trigger is part of the hash key
- * --- we want to compile the trigger function separately for each trigger
- * it is used with, in case the rowtype or transition table names are
- * different. Zero if not called as a DML trigger.
- */
- Oid trigOid;
-
- /*
- * We must include the input collation as part of the hash key too,
- * because we have to generate different plans (with different Param
- * collations) for different collation settings.
- */
- Oid inputCollation;
-
- /*
- * We include actual argument types in the hash key to support polymorphic
- * PLpgSQL functions. Be careful that extra positions are zeroed!
- */
- Oid argtypes[FUNC_MAX_ARGS];
-} PLpgSQL_func_hashkey;
-
/*
* Trigger type
*/
@@ -990,13 +957,12 @@ typedef enum PLpgSQL_trigtype
*/
typedef struct PLpgSQL_function
{
+ CachedFunction cfunc; /* fields managed by funccache.c */
+
char *fn_signature;
Oid fn_oid;
- TransactionId fn_xmin;
- ItemPointerData fn_tid;
PLpgSQL_trigtype fn_is_trigger;
Oid fn_input_collation;
- PLpgSQL_func_hashkey *fn_hashkey; /* back-link to hashtable key */
MemoryContext fn_cxt;
Oid fn_rettype;
@@ -1036,9 +1002,8 @@ typedef struct PLpgSQL_function
bool requires_procedure_resowner; /* contains CALL or DO? */
bool has_exception_block; /* contains BEGIN...EXCEPTION? */
- /* these fields change when the function is used */
+ /* this field changes when the function is used */
struct PLpgSQL_execstate *cur_estate;
- unsigned long use_count;
} PLpgSQL_function;
/*
@@ -1287,7 +1252,6 @@ extern PGDLLEXPORT int plpgsql_recognize_err_condition(const char *condname,
extern PLpgSQL_condition *plpgsql_parse_err_condition(char *condname);
extern void plpgsql_adddatum(PLpgSQL_datum *newdatum);
extern int plpgsql_add_initdatums(int **varnos);
-extern void plpgsql_HashTableInit(void);
/*
* Functions in pl_exec.c
@@ -1335,6 +1299,7 @@ extern PGDLLEXPORT const char *plpgsql_stmt_typename(PLpgSQL_stmt *stmt);
extern const char *plpgsql_getdiag_kindname(PLpgSQL_getdiag_kind kind);
extern void plpgsql_mark_local_assignment_targets(PLpgSQL_function *func);
extern void plpgsql_free_function_memory(PLpgSQL_function *func);
+extern void plpgsql_delete_callback(CachedFunction *cfunc);
extern void plpgsql_dumptree(PLpgSQL_function *func);
/*
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ff75a508876..144c4e9662c 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -381,6 +381,11 @@ CURLM
CURLoption
CV
CachedExpression
+CachedFunction
+CachedFunctionCompileCallback
+CachedFunctionDeleteCallback
+CachedFunctionHashEntry
+CachedFunctionHashKey
CachedPlan
CachedPlanSource
CallContext
--
2.43.5
From f0b52fe1844bf1be0a3fcc718069c7c1a41393e1 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 29 Mar 2025 16:56:57 -0400
Subject: [PATCH v10 4/6] Restructure check_sql_fn_retval().
To support using the plan cache for SQL functions, we'll need to be
able to redo the work of check_sql_fn_retval() on just one query's
list-of-rewritten-queries at a time, since the plan cache will treat
each query independently. This would be simple enough, except for a
bizarre historical behavior: the existing code will take the last
canSetTag query in the function as determining the result, even if
it came from not-the-last original query. (The case is only possible
when the last original query(s) are deleted by a DO INSTEAD NOTHING
rule.) This behavior is undocumented except in source code comments,
and it seems hard to believe that anyone's relying on it. It would
be a mess to support with the plan cache, because a change in the
rules applicable to some table could change which CachedPlanSource
is supposed to produce the function result, even if the function
itself has not changed. Let's just get rid of that silliness and
insist that the last source query in the function is the one that
must produce the result.
Having mandated that, we can refactor check_sql_fn_retval() into
an outer and an inner function where the inner one considers only
a single list-of-rewritten-queries; the inner one will be usable
in a post-rewrite callback hook as contemplated by the previous
commit.
Likewise refactor check_sql_fn_statements() so that we have a
version that can be applied to just one list of Queries.
(As things stand, it's not really necessary to recheck that
during a replan, but maybe future changes in the rule system
would create cases where it matters.)
Also remove check_sql_fn_retval()'s targetlist output argument,
putting the equivalent functionality into a separate function.
This is needed because the plan cache would be in the way of
passing that data directly. No outside caller needed that
anyway.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/8216639.NyiUUSuA9g@aivenlaptop
---
src/backend/catalog/pg_proc.c | 2 +-
src/backend/executor/functions.c | 176 ++++++++++++++++++---------
src/backend/optimizer/util/clauses.c | 4 +-
src/include/executor/functions.h | 3 +-
4 files changed, 121 insertions(+), 64 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index fe0490259e9..880b597fb3a 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -960,7 +960,7 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
(void) check_sql_fn_retval(querytree_list,
rettype, rettupdesc,
proc->prokind,
- false, NULL);
+ false);
}
error_context_stack = sqlerrcontext.previous;
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 6aa8e9c4d8a..5b06df84335 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -153,11 +153,16 @@ static Datum postquel_get_single_result(TupleTableSlot *slot,
MemoryContext resultcontext);
static void sql_exec_error_callback(void *arg);
static void ShutdownSQLFunction(Datum arg);
+static void check_sql_fn_statement(List *queryTreeList);
+static bool check_sql_stmt_retval(List *queryTreeList,
+ Oid rettype, TupleDesc rettupdesc,
+ char prokind, bool insertDroppedCols);
static bool coerce_fn_result_column(TargetEntry *src_tle,
Oid res_type, int32 res_typmod,
bool tlist_is_modifiable,
List **upper_tlist,
bool *upper_tlist_nontrivial);
+static List *get_sql_fn_result_tlist(List *queryTreeList);
static void sqlfunction_startup(DestReceiver *self, int operation, TupleDesc typeinfo);
static bool sqlfunction_receive(TupleTableSlot *slot, DestReceiver *self);
static void sqlfunction_shutdown(DestReceiver *self);
@@ -592,7 +597,6 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
Form_pg_proc procedureStruct;
SQLFunctionCachePtr fcache;
List *queryTree_list;
- List *resulttlist;
ListCell *lc;
Datum tmp;
bool isNull;
@@ -748,8 +752,7 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
rettype,
rettupdesc,
procedureStruct->prokind,
- false,
- &resulttlist);
+ false);
/*
* Construct a JunkFilter we can use to coerce the returned rowtype to the
@@ -762,6 +765,14 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
{
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(llast_node(List, queryTree_list));
/*
* If the result is composite, *and* we are returning the whole tuple
@@ -1541,29 +1552,39 @@ check_sql_fn_statements(List *queryTreeLists)
foreach(lc, queryTreeLists)
{
List *sublist = lfirst_node(List, lc);
- ListCell *lc2;
- foreach(lc2, sublist)
- {
- Query *query = lfirst_node(Query, lc2);
+ check_sql_fn_statement(sublist);
+ }
+}
- /*
- * Disallow calling procedures with output arguments. The current
- * implementation would just throw the output values away, unless
- * the statement is the last one. Per SQL standard, we should
- * assign the output values by name. By disallowing this here, we
- * preserve an opportunity for future improvement.
- */
- if (query->commandType == CMD_UTILITY &&
- IsA(query->utilityStmt, CallStmt))
- {
- CallStmt *stmt = (CallStmt *) query->utilityStmt;
+/*
+ * As above, for a single sublist of Queries.
+ */
+static void
+check_sql_fn_statement(List *queryTreeList)
+{
+ ListCell *lc;
- if (stmt->outargs != NIL)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("calling procedures with output arguments is not supported in SQL functions")));
- }
+ foreach(lc, queryTreeList)
+ {
+ Query *query = lfirst_node(Query, lc);
+
+ /*
+ * Disallow calling procedures with output arguments. The current
+ * implementation would just throw the output values away, unless the
+ * statement is the last one. Per SQL standard, we should assign the
+ * output values by name. By disallowing this here, we preserve an
+ * opportunity for future improvement.
+ */
+ if (query->commandType == CMD_UTILITY &&
+ IsA(query->utilityStmt, CallStmt))
+ {
+ CallStmt *stmt = (CallStmt *) query->utilityStmt;
+
+ if (stmt->outargs != NIL)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("calling procedures with output arguments is not supported in SQL functions")));
}
}
}
@@ -1602,17 +1623,45 @@ check_sql_fn_statements(List *queryTreeLists)
* In addition to coercing individual output columns, we can modify the
* output to include dummy NULL columns for any dropped columns appearing
* in rettupdesc. This is done only if the caller asks for it.
- *
- * If resultTargetList isn't NULL, then *resultTargetList is set to the
- * targetlist that defines the final statement's result. Exception: if the
- * function is defined to return VOID then *resultTargetList is set to NIL.
*/
bool
check_sql_fn_retval(List *queryTreeLists,
Oid rettype, TupleDesc rettupdesc,
char prokind,
- bool insertDroppedCols,
- List **resultTargetList)
+ bool insertDroppedCols)
+{
+ List *queryTreeList;
+
+ /*
+ * We consider only the last sublist of Query nodes, so that only the last
+ * original statement is a candidate to produce the result. This is a
+ * change from pre-v18 versions, which would back up to the last statement
+ * that includes a canSetTag query, thus ignoring any ending statement(s)
+ * that rewrite to DO INSTEAD NOTHING. That behavior was undocumented and
+ * there seems no good reason for it, except that it was an artifact of
+ * the original coding.
+ *
+ * If the function body is completely empty, handle that the same as if
+ * the last query had rewritten to nothing.
+ */
+ if (queryTreeLists != NIL)
+ queryTreeList = llast_node(List, queryTreeLists);
+ else
+ queryTreeList = NIL;
+
+ return check_sql_stmt_retval(queryTreeList,
+ rettype, rettupdesc,
+ prokind, insertDroppedCols);
+}
+
+/*
+ * As for check_sql_fn_retval, but we are given just the last query's
+ * rewritten-queries list.
+ */
+static bool
+check_sql_stmt_retval(List *queryTreeList,
+ Oid rettype, TupleDesc rettupdesc,
+ char prokind, bool insertDroppedCols)
{
bool is_tuple_result = false;
Query *parse;
@@ -1625,9 +1674,6 @@ check_sql_fn_retval(List *queryTreeLists,
bool upper_tlist_nontrivial = false;
ListCell *lc;
- if (resultTargetList)
- *resultTargetList = NIL; /* initialize in case of VOID result */
-
/*
* If it's declared to return VOID, we don't care what's in the function.
* (This takes care of procedures with no output parameters, as well.)
@@ -1636,30 +1682,20 @@ check_sql_fn_retval(List *queryTreeLists,
return false;
/*
- * Find the last canSetTag query in the function body (which is presented
- * to us as a list of sublists of Query nodes). This isn't necessarily
- * the last parsetree, because rule rewriting can insert queries after
- * what the user wrote. Note that it might not even be in the last
- * sublist, for example if the last query rewrites to DO INSTEAD NOTHING.
- * (It might not be unreasonable to throw an error in such a case, but
- * this is the historical behavior and it doesn't seem worth changing.)
+ * Find the last canSetTag query in the list of Query nodes. This isn't
+ * necessarily the last parsetree, because rule rewriting can insert
+ * queries after what the user wrote.
*/
parse = NULL;
parse_cell = NULL;
- foreach(lc, queryTreeLists)
+ foreach(lc, queryTreeList)
{
- List *sublist = lfirst_node(List, lc);
- ListCell *lc2;
+ Query *q = lfirst_node(Query, lc);
- foreach(lc2, sublist)
+ if (q->canSetTag)
{
- Query *q = lfirst_node(Query, lc2);
-
- if (q->canSetTag)
- {
- parse = q;
- parse_cell = lc2;
- }
+ parse = q;
+ parse_cell = lc;
}
}
@@ -1812,12 +1848,7 @@ check_sql_fn_retval(List *queryTreeLists,
* further checking. Assume we're returning the whole tuple.
*/
if (rettupdesc == NULL)
- {
- /* Return tlist if requested */
- if (resultTargetList)
- *resultTargetList = tlist;
return true;
- }
/*
* Verify that the targetlist matches the return tuple type. We scan
@@ -1984,10 +2015,6 @@ tlist_coercion_finished:
lfirst(parse_cell) = newquery;
}
- /* Return tlist (possibly modified) if requested */
- if (resultTargetList)
- *resultTargetList = upper_tlist;
-
return is_tuple_result;
}
@@ -2063,6 +2090,37 @@ coerce_fn_result_column(TargetEntry *src_tle,
return true;
}
+/*
+ * Extract the targetlist of the last canSetTag query in the given list
+ * of parsed-and-rewritten Queries. Returns NIL if there is none.
+ */
+static List *
+get_sql_fn_result_tlist(List *queryTreeList)
+{
+ Query *parse = NULL;
+ ListCell *lc;
+
+ foreach(lc, queryTreeList)
+ {
+ Query *q = lfirst_node(Query, lc);
+
+ if (q->canSetTag)
+ parse = q;
+ }
+ if (parse &&
+ parse->commandType == CMD_SELECT)
+ return parse->targetList;
+ else if (parse &&
+ (parse->commandType == CMD_INSERT ||
+ parse->commandType == CMD_UPDATE ||
+ parse->commandType == CMD_DELETE ||
+ parse->commandType == CMD_MERGE) &&
+ parse->returningList)
+ return parse->returningList;
+ else
+ return NIL;
+}
+
/*
* CreateSQLFunctionDestReceiver -- create a suitable DestReceiver object
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 43dfecfb47f..816536ab865 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -4742,7 +4742,7 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid,
if (check_sql_fn_retval(list_make1(querytree_list),
result_type, rettupdesc,
funcform->prokind,
- false, NULL))
+ false))
goto fail; /* reject whole-tuple-result cases */
/*
@@ -5288,7 +5288,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
if (!check_sql_fn_retval(list_make1(querytree_list),
fexpr->funcresulttype, rettupdesc,
funcform->prokind,
- true, NULL) &&
+ true) &&
(functypclass == TYPEFUNC_COMPOSITE ||
functypclass == TYPEFUNC_COMPOSITE_DOMAIN ||
functypclass == TYPEFUNC_RECORD))
diff --git a/src/include/executor/functions.h b/src/include/executor/functions.h
index a6ae2e72d79..58bdff9b039 100644
--- a/src/include/executor/functions.h
+++ b/src/include/executor/functions.h
@@ -48,8 +48,7 @@ extern void check_sql_fn_statements(List *queryTreeLists);
extern bool check_sql_fn_retval(List *queryTreeLists,
Oid rettype, TupleDesc rettupdesc,
char prokind,
- bool insertDroppedCols,
- List **resultTargetList);
+ bool insertDroppedCols);
extern DestReceiver *CreateSQLFunctionDestReceiver(void);
--
2.43.5
From d85adbeae4f83ce0355c8f626f7c8b0aa961735f Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 29 Mar 2025 20:31:50 -0400
Subject: [PATCH v10 5/6] Add a test case showing undesirable RLS behavior in
SQL functions.
In the historical implementation of SQL functions, once we have
built a set of plans for a SQL function we'll continue to use
them during subsequent function invocations in the same query.
This isn't ideal, and this somewhat-contrived test case shows
one reason why not: we don't notice changes in RLS-relevant
state. I'm putting this as a separate patch in the series
so that the change in behavior will be apparent.
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/test/regress/expected/rowsecurity.out | 59 +++++++++++++++++++++++
src/test/regress/sql/rowsecurity.sql | 44 +++++++++++++++++
2 files changed, 103 insertions(+)
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 87929191d06..8f2c8319172 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -4695,6 +4695,65 @@ RESET ROLE;
DROP FUNCTION rls_f();
DROP VIEW rls_v;
DROP TABLE rls_t;
+-- Check that RLS changes invalidate SQL 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 function rls_f(text) returns text
+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;
+set plan_cache_mode to force_custom_plan;
+-- 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)
+
+-- For other users, changes in row_security setting
+-- 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)
+
+reset role;
+set plan_cache_mode to force_generic_plan;
+-- Table owner bypasses RLS, although cached plan will be invalidated
+select rls_f(c) from test_t order by rls_f;
+ rls_f
+-------------
+ aoffa,b,c,d
+ boffa,b,c,d
+(2 rows)
+
+-- For other users, changes in row_security setting
+-- 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)
+
+reset role;
+reset plan_cache_mode;
+reset rls_test.blah;
+drop function rls_f(text);
+drop table rls_t, test_t;
--
-- Clean up objects
--
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index f61dbbf9581..9da967a9ef2 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -2307,6 +2307,50 @@ DROP FUNCTION rls_f();
DROP VIEW rls_v;
DROP TABLE rls_t;
+-- Check that RLS changes invalidate SQL 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 function rls_f(text) returns text
+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;
+
+set plan_cache_mode to force_custom_plan;
+
+-- Table owner bypasses RLS
+select rls_f(c) from test_t order by rls_f;
+
+-- For other users, changes in row_security setting
+-- should lead to RLS error during query rewrite
+set role regress_rls_alice;
+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, although cached plan will be invalidated
+select rls_f(c) from test_t order by rls_f;
+
+-- For other users, changes in row_security setting
+-- 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;
+reset role;
+
+reset plan_cache_mode;
+reset rls_test.blah;
+drop function rls_f(text);
+drop table rls_t, test_t;
+
--
-- Clean up objects
--
--
2.43.5
From 7427db23bb4de73dddfdbbf9d6ed902079894aa1 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 29 Mar 2025 21:57:50 -0400
Subject: [PATCH v10 6/6] 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 <a.pyhalov@postgrespro.ru>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
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
pgsql-hackers by date: