Hello.
At Wed, 10 Apr 2019 03:51:07 +0000, PG Bug reporting form <noreply@postgresql.org> wrote in
<15746-6e0482a4c0f915cb@postgresql.org>
> The following bug has been logged on the website:
>
> Bug reference: 15746
> Logged by: Roman Zharkov
> Email address: r.zharkov@postgrespro.ru
> PostgreSQL version: 10.7
> Operating system: centos 7, fedora 28
> Description:
>
> Hello,
> I found a problem within regression tests. The plpgsql test fails when
> running twice on the same database.
> Here is small script illustrates the problem:
>
> begin;
> create function sql_to_date(integer) returns date as $$
> select $1::text::date
> $$ language sql immutable strict;
> create cast (integer as date) with function sql_to_date(integer) as
> assignment;
> create function cast_invoker(integer) returns date as $$
> begin
> return $1;
> end$$ language plpgsql;
>
> select cast_invoker(20150717);
>
> drop function cast_invoker(integer);
> drop function sql_to_date(integer) cascade;
> commit;
>
> begin;
> create function sql_to_date(integer) returns date as $$
> select $1::text::date
> $$ language sql immutable strict;
> create cast (integer as date) with function sql_to_date(integer) as
> assignment;
> create function cast_invoker(integer) returns date as $$
> begin
> return $1;
> end$$ language plpgsql;
>
> select cast_invoker(20150717);
>
> drop function cast_invoker(integer);
> drop function sql_to_date(integer) cascade;
> commit;
>
> Results:
>
> begin;
> create function sql_to_date(integer) returns date as $$
> select $1::text::date
> $$ language sql immutable strict;
> create cast (integer as date) with function sql_to_date(integer) as
> assignment;
> create function cast_invoker(integer) returns date as $$
> begin
> return $1;
> end$$ language plpgsql;
> select cast_invoker(20150717);
> cast_invoker
> --------------
> 07-17-2015
> (1 row)
>
> drop function cast_invoker(integer);
> drop function sql_to_date(integer) cascade;
> NOTICE: drop cascades to cast from integer to date
> commit;
> begin;
> create function sql_to_date(integer) returns date as $$
> select $1::text::date
> $$ language sql immutable strict;
> create cast (integer as date) with function sql_to_date(integer) as
> assignment;
> create function cast_invoker(integer) returns date as $$
> begin
> return $1;
> end$$ language plpgsql;
> select cast_invoker(20150717);
> ERROR: cache lookup failed for function 16414
> CONTEXT: PL/pgSQL function cast_invoker(integer) while casting return value
> to function's return type
> drop function cast_invoker(integer);
> ERROR: current transaction is aborted, commands ignored until end of
> transaction block
> drop function sql_to_date(integer) cascade;
> ERROR: current transaction is aborted, commands ignored until end of
> transaction block
> commit;
>
> The problem reproduces in the 10, 11 versions.
The cause is stale cast function id in cached expression of
plpgsql. (get_cast_hashentry)
Happens since 9.5 to 11. Once happens, the symptom persists
until session-end. Doesn't happen on 9.4 since it doesn't cache
cast expressions. 12 invalidates cached cast expressions
(04fe805a17).
It seems to me possible that a cast calls a wrong function and
leads to a crash. But I don't come up with a good way to fix
this, but applying a part of the patch 04fe805a17 on 11(.2) seems
working.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 7f1f962f60..aec1a68a4b 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -5909,6 +5909,57 @@ expression_planner(Expr *expr)
return (Expr *) result;
}
+/*
+ * expression_planner_with_deps
+ * Perform planner's transformations on a standalone expression,
+ * returning expression dependency information along with the result.
+ *
+ * This is identical to expression_planner() except that it also returns
+ * information about possible dependencies of the expression, ie identities of
+ * objects whose definitions affect the result. As in a PlannedStmt, these
+ * are expressed as a list of relation Oids and a list of PlanInvalItems.
+ */
+Expr *
+expression_planner_with_deps(Expr *expr,
+ List **relationOids,
+ List **invalItems)
+{
+ Node *result;
+ PlannerGlobal glob;
+ PlannerInfo root;
+
+ /* Make up dummy planner state so we can use setrefs machinery */
+ MemSet(&glob, 0, sizeof(glob));
+ glob.type = T_PlannerGlobal;
+ glob.relationOids = NIL;
+ glob.invalItems = NIL;
+
+ MemSet(&root, 0, sizeof(root));
+ root.type = T_PlannerInfo;
+ root.glob = &glob;
+
+ /*
+ * Convert named-argument function calls, insert default arguments and
+ * simplify constant subexprs. Collect identities of inlined functions
+ * and elided domains, too.
+ */
+ result = eval_const_expressions(&root, (Node *) expr);
+
+ /* Fill in opfuncid values if missing */
+ fix_opfuncids(result);
+
+ /*
+ * Now walk the finished expression to find anything else we ought to
+ * record as an expression dependency.
+ */
+ (void) extract_query_dependencies_walker(result, &root);
+
+ *relationOids = glob.relationOids;
+ *invalItems = glob.invalItems;
+
+ return (Expr *) result;
+}
+
/*
* plan_cluster_use_sort
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 80e6e0da0d..2ed1f65767 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -138,8 +138,7 @@ static List *set_returning_clause_references(PlannerInfo *root,
Plan *topplan,
Index resultRelation,
int rtoffset);
-static bool extract_query_dependencies_walker(Node *node,
- PlannerInfo *context);
+
/*****************************************************************************
*
@@ -2602,7 +2601,15 @@ extract_query_dependencies(Node *query,
*hasRowSecurity = glob.dependsOnRole;
}
-static bool
+/*
+ * Tree walker for extract_query_dependencies.
+ *
+ * This is exported so that expression_planner_with_deps can call it on
+ * simple expressions (post-planning, not before planning, in that case).
+ * In that usage, glob.dependsOnRole isn't meaningful, but the relationOids
+ * and invalItems lists are added to as needed.
+ */
+bool
extract_query_dependencies_walker(Node *node, PlannerInfo *context)
{
if (node == NULL)
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 0ad3e3c736..87c82a4a6a 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -57,6 +57,7 @@
#include "nodes/nodeFuncs.h"
#include "optimizer/cost.h"
#include "optimizer/planmain.h"
+#include "optimizer/planner.h"
#include "optimizer/prep.h"
#include "parser/analyze.h"
#include "parser/parsetree.h"
@@ -86,6 +87,10 @@
* guarantee to save a CachedPlanSource without error.
*/
static CachedPlanSource *first_saved_plan = NULL;
+/*
+ * This is the head of the backend's list of CachedExpressions.
+ */
+static dlist_head cached_expression_list = DLIST_STATIC_INIT(cached_expression_list);
static void ReleaseGenericPlan(CachedPlanSource *plansource);
static List *RevalidateCachedQuery(CachedPlanSource *plansource,
@@ -103,7 +108,7 @@ static void ScanQueryForLocks(Query *parsetree, bool acquire);
static bool ScanQueryWalker(Node *node, bool *acquire);
static TupleDesc PlanCacheComputeResultDesc(List *stmt_list);
static void PlanCacheRelCallback(Datum arg, Oid relid);
-static void PlanCacheFuncCallback(Datum arg, int cacheid, uint32 hashvalue);
+static void PlanCacheObjectCallback(Datum arg, int cacheid, uint32 hashvalue);
static void PlanCacheSysCallback(Datum arg, int cacheid, uint32 hashvalue);
@@ -116,7 +121,7 @@ void
InitPlanCache(void)
{
CacheRegisterRelcacheCallback(PlanCacheRelCallback, (Datum) 0);
- CacheRegisterSyscacheCallback(PROCOID, PlanCacheFuncCallback, (Datum) 0);
+ CacheRegisterSyscacheCallback(PROCOID, PlanCacheObjectCallback, (Datum) 0);
CacheRegisterSyscacheCallback(NAMESPACEOID, PlanCacheSysCallback, (Datum) 0);
CacheRegisterSyscacheCallback(OPEROID, PlanCacheSysCallback, (Datum) 0);
CacheRegisterSyscacheCallback(AMOPOPID, PlanCacheSysCallback, (Datum) 0);
@@ -1450,6 +1455,85 @@ CachedPlanGetTargetList(CachedPlanSource *plansource,
return FetchStatementTargetList((Node *) pstmt);
}
+/*
+ * GetCachedExpression: construct a CachedExpression for an expression.
+ *
+ * This performs the same transformations on the expression as
+ * expression_planner(), ie, convert an expression as emitted by parse
+ * analysis to be ready to pass to the executor.
+ *
+ * The result is stashed in a private, long-lived memory context.
+ * (Note that this might leak a good deal of memory in the caller's
+ * context before that.) The passed-in expr tree is not modified.
+ */
+CachedExpression *
+GetCachedExpression(Node *expr)
+{
+ CachedExpression *cexpr;
+ List *relationOids;
+ List *invalItems;
+ MemoryContext cexpr_context;
+ MemoryContext oldcxt;
+
+ /*
+ * Pass the expression through the planner, and collect dependencies.
+ * Everything built here is leaked in the caller's context; that's
+ * intentional to minimize the size of the permanent data structure.
+ */
+ expr = (Node *) expression_planner_with_deps((Expr *) expr,
+ &relationOids,
+ &invalItems);
+
+ /*
+ * Make a private memory context, and copy what we need into that. To
+ * avoid leaking a long-lived context if we fail while copying data, we
+ * initially make the context under the caller's context.
+ */
+ cexpr_context = AllocSetContextCreate(CurrentMemoryContext,
+ "CachedExpression",
+ ALLOCSET_SMALL_SIZES);
+
+ oldcxt = MemoryContextSwitchTo(cexpr_context);
+
+ cexpr = (CachedExpression *) palloc(sizeof(CachedExpression));
+ cexpr->magic = CACHEDEXPR_MAGIC;
+ cexpr->expr = copyObject(expr);
+ cexpr->is_valid = true;
+ cexpr->relationOids = copyObject(relationOids);
+ cexpr->invalItems = copyObject(invalItems);
+ cexpr->context = cexpr_context;
+
+ MemoryContextSwitchTo(oldcxt);
+
+ /*
+ * Reparent the expr's memory context under CacheMemoryContext so that it
+ * will live indefinitely.
+ */
+ MemoryContextSetParent(cexpr_context, CacheMemoryContext);
+
+ /*
+ * Add the entry to the global list of cached expressions.
+ */
+ dlist_push_tail(&cached_expression_list, &cexpr->node);
+
+ return cexpr;
+}
+
+/*
+ * FreeCachedExpression
+ * Delete a CachedExpression.
+ */
+void
+FreeCachedExpression(CachedExpression *cexpr)
+{
+ /* Sanity check */
+ Assert(cexpr->magic == CACHEDEXPR_MAGIC);
+ /* Unlink from global list */
+ dlist_delete(&cexpr->node);
+ /* Free all storage associated with CachedExpression */
+ MemoryContextDelete(cexpr->context);
+}
+
/*
* QueryListGetPrimaryStmt
* Get the "primary" stmt within a list, ie, the one marked canSetTag.
@@ -1710,6 +1794,7 @@ static void
PlanCacheRelCallback(Datum arg, Oid relid)
{
CachedPlanSource *plansource;
+ dlist_iter iter;
for (plansource = first_saved_plan; plansource; plansource = plansource->next_saved)
{
@@ -1759,11 +1844,30 @@ PlanCacheRelCallback(Datum arg, Oid relid)
}
}
}
+
+ /* Likewise check cached expressions */
+ dlist_foreach(iter, &cached_expression_list)
+ {
+ CachedExpression *cexpr = dlist_container(CachedExpression,
+ node, iter.cur);
+
+ Assert(cexpr->magic == CACHEDEXPR_MAGIC);
+
+ /* No work if it's already invalidated */
+ if (!cexpr->is_valid)
+ continue;
+
+ if ((relid == InvalidOid) ? cexpr->relationOids != NIL :
+ list_member_oid(cexpr->relationOids, relid))
+ {
+ cexpr->is_valid = false;
+ }
+ }
}
/*
- * PlanCacheFuncCallback
- * Syscache inval callback function for PROCOID cache
+ * PlanCacheObjectCallback
+ * Syscache inval callback function for PROCOID and TYPEOID caches
*
* Invalidate all plans mentioning the object with the specified hash value,
* or all plans mentioning any member of this cache if hashvalue == 0.
@@ -1772,9 +1876,10 @@ PlanCacheRelCallback(Datum arg, Oid relid)
* now only user-defined functions are tracked this way.
*/
static void
-PlanCacheFuncCallback(Datum arg, int cacheid, uint32 hashvalue)
+PlanCacheObjectCallback(Datum arg, int cacheid, uint32 hashvalue)
{
CachedPlanSource *plansource;
+ dlist_iter iter;
for (plansource = first_saved_plan; plansource; plansource = plansource->next_saved)
{
@@ -1842,6 +1947,34 @@ PlanCacheFuncCallback(Datum arg, int cacheid, uint32 hashvalue)
}
}
}
+
+ /* Likewise check cached expressions */
+ dlist_foreach(iter, &cached_expression_list)
+ {
+ CachedExpression *cexpr = dlist_container(CachedExpression,
+ node, iter.cur);
+ ListCell *lc;
+
+ Assert(cexpr->magic == CACHEDEXPR_MAGIC);
+
+ /* No work if it's already invalidated */
+ if (!cexpr->is_valid)
+ continue;
+
+ foreach(lc, cexpr->invalItems)
+ {
+ PlanInvalItem *item = (PlanInvalItem *) lfirst(lc);
+
+ if (item->cacheId != cacheid)
+ continue;
+ if (hashvalue == 0 ||
+ item->hashValue == hashvalue)
+ {
+ cexpr->is_valid = false;
+ break;
+ }
+ }
+ }
}
/*
diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h
index a081ca689a..8de97f9399 100644
--- a/src/include/optimizer/planmain.h
+++ b/src/include/optimizer/planmain.h
@@ -122,5 +122,6 @@ extern void extract_query_dependencies(Node *query,
List **relationOids,
List **invalItems,
bool *hasRowSecurity);
+extern bool extract_query_dependencies_walker(Node *node, PlannerInfo *root);
#endif /* PLANMAIN_H */
diff --git a/src/include/optimizer/planner.h b/src/include/optimizer/planner.h
index 3e733b34ed..902ab403ed 100644
--- a/src/include/optimizer/planner.h
+++ b/src/include/optimizer/planner.h
@@ -55,6 +55,9 @@ extern Path *get_cheapest_fractional_path(RelOptInfo *rel,
double tuple_fraction);
extern Expr *expression_planner(Expr *expr);
+extern Expr *expression_planner_with_deps(Expr *expr,
+ List **relationOids,
+ List **invalItems);
extern Expr *preprocess_phv_expression(PlannerInfo *root, Expr *expr);
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index ab20aa04b0..dc2b5ba946 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -16,6 +16,7 @@
#define PLANCACHE_H
#include "access/tupdesc.h"
+#include "lib/ilist.h"
#include "nodes/params.h"
#include "utils/queryenvironment.h"
@@ -24,6 +25,7 @@ struct RawStmt;
#define CACHEDPLANSOURCE_MAGIC 195726186
#define CACHEDPLAN_MAGIC 953717834
+#define CACHEDEXPR_MAGIC 838275847
/*
* CachedPlanSource (which might better have been called CachedQuery)
@@ -143,6 +145,30 @@ typedef struct CachedPlan
MemoryContext context; /* context containing this CachedPlan */
} CachedPlan;
+/*
+ * CachedExpression is a low-overhead mechanism for caching the planned form
+ * of standalone scalar expressions. While such expressions are not usually
+ * subject to cache invalidation events, that can happen, for example because
+ * of replacement of a SQL function that was inlined into the expression.
+ * The plancache takes care of storing the expression tree and marking it
+ * invalid if a cache invalidation occurs, but the caller must notice the
+ * !is_valid status and discard the obsolete expression without reusing it.
+ * We do not store the original parse tree, only the planned expression;
+ * this is an optimization based on the assumption that we usually will not
+ * need to replan for the life of the session.
+ */
+typedef struct CachedExpression
+{
+ int magic; /* should equal CACHEDEXPR_MAGIC */
+ Node *expr; /* planned form of expression */
+ bool is_valid; /* is the expression still valid? */
+ /* remaining fields should be treated as private to plancache.c: */
+ List *relationOids; /* OIDs of relations the expr depends on */
+ List *invalItems; /* other dependencies, as PlanInvalItems */
+ MemoryContext context; /* context containing this CachedExpression */
+ dlist_node node; /* link in global list of CachedExpressions */
+} CachedExpression;
+
extern void InitPlanCache(void);
extern void ResetPlanCache(void);
@@ -182,4 +208,7 @@ extern CachedPlan *GetCachedPlan(CachedPlanSource *plansource,
QueryEnvironment *queryEnv);
extern void ReleaseCachedPlan(CachedPlan *plan, bool useResOwner);
+extern CachedExpression *GetCachedExpression(Node *expr);
+extern void FreeCachedExpression(CachedExpression *cexpr);
+
#endif /* PLANCACHE_H */
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 8f8f7efe44..ecb8e79baa 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -152,6 +152,7 @@ typedef struct /* cast_hash table entry */
{
plpgsql_CastHashKey key; /* hash key --- MUST BE FIRST */
Expr *cast_expr; /* cast expression, or NULL if no-op cast */
+ CachedExpression *cast_cexpr; /* cached expression backing the above */
/* ExprState is valid only when cast_lxid matches current LXID */
ExprState *cast_exprstate; /* expression's eval tree */
bool cast_in_use; /* true while we're executing eval tree */
@@ -7527,18 +7528,35 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
cast_key.dsttypmod = dsttypmod;
cast_entry = (plpgsql_CastHashEntry *) hash_search(estate->cast_hash,
(void *) &cast_key,
- HASH_FIND, NULL);
+ HASH_ENTER, &found);
- if (cast_entry == NULL)
+ if (!found) /* initialize if new entry */
+ cast_entry->cast_cexpr = NULL;
+
+ if (cast_entry->cast_cexpr == NULL ||
+ !cast_entry->cast_cexpr->is_valid)
{
- /* We've not looked up this coercion before */
+ /*
+ * We've not looked up this coercion before, or we have but the cached
+ * expression has been invalidated.
+ */
Node *cast_expr;
+ CachedExpression *cast_cexpr;
CaseTestExpr *placeholder;
+ /*
+ * Drop old cached expression if there is one.
+ */
+ if (cast_entry->cast_cexpr)
+ {
+ FreeCachedExpression(cast_entry->cast_cexpr);
+ cast_entry->cast_cexpr = NULL;
+ }
+
/*
* Since we could easily fail (no such coercion), construct a
* temporary coercion expression tree in the short-lived
- * eval_mcontext, then if successful copy it to cast_hash_context.
+ * eval_mcontext, then if successful save it as a CachedExpression.
*/
oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate));
@@ -7599,33 +7617,23 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
/* Note: we don't bother labeling the expression tree with collation */
+ /* Plan the expression and build a CachedExpression */
+ cast_cexpr = GetCachedExpression(cast_expr);
+ cast_expr = cast_cexpr->expr;
+
/* Detect whether we have a no-op (RelabelType) coercion */
if (IsA(cast_expr, RelabelType) &&
((RelabelType *) cast_expr)->arg == (Expr *) placeholder)
cast_expr = NULL;
- if (cast_expr)
- {
- /* ExecInitExpr assumes we've planned the expression */
- cast_expr = (Node *) expression_planner((Expr *) cast_expr);
-
- /* Now copy the tree into cast_hash_context */
- MemoryContextSwitchTo(estate->cast_hash_context);
-
- cast_expr = copyObject(cast_expr);
- }
-
- MemoryContextSwitchTo(oldcontext);
-
- /* Now we can fill in a hashtable entry. */
- cast_entry = (plpgsql_CastHashEntry *) hash_search(estate->cast_hash,
- (void *) &cast_key,
- HASH_ENTER, &found);
- Assert(!found); /* wasn't there a moment ago */
+ /* Now we can fill in the hashtable entry. */
+ cast_entry->cast_cexpr = cast_cexpr;
cast_entry->cast_expr = (Expr *) cast_expr;
cast_entry->cast_exprstate = NULL;
cast_entry->cast_in_use = false;
cast_entry->cast_lxid = InvalidLocalTransactionId;
+
+ MemoryContextSwitchTo(oldcontext);
}
/* Done if we have determined that this is a no-op cast. */