Prevent RevalidateCachedPlan from making any permanent change in
ActiveSnapshot. Having it affect ActiveSnapshot only in the unusual case of needing to replan seems a bad idea, and there's also the problem that the created snap might be in a relatively short-lived context, as noted by Jan Wieck. Also, there's no need to force a new snap at all unless we are called with no snap currently set, which is an unusual case in itself.
This commit is contained in:
parent
689dea424d
commit
fd53a67dcd
54
src/backend/utils/cache/plancache.c
vendored
54
src/backend/utils/cache/plancache.c
vendored
@ -33,7 +33,7 @@
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/backend/utils/cache/plancache.c,v 1.8 2007/04/16 18:21:07 tgl Exp $
|
||||
* $PostgreSQL: pgsql/src/backend/utils/cache/plancache.c,v 1.9 2007/05/14 18:13:21 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -69,6 +69,7 @@ static List *cached_plans_list = NIL;
|
||||
|
||||
static void StoreCachedPlan(CachedPlanSource *plansource, List *stmt_list,
|
||||
MemoryContext plan_context);
|
||||
static List *do_planning(List *querytrees, int cursorOptions);
|
||||
static void AcquireExecutorLocks(List *stmt_list, bool acquire);
|
||||
static void AcquirePlannerLocks(List *stmt_list, bool acquire);
|
||||
static void LockRelid(Oid relid, LOCKMODE lockmode, void *arg);
|
||||
@ -462,12 +463,8 @@ RevalidateCachedPlan(CachedPlanSource *plansource, bool useResOwner)
|
||||
|
||||
if (plansource->fully_planned)
|
||||
{
|
||||
/*
|
||||
* Generate plans for queries. Assume snapshot is not set yet
|
||||
* (XXX this may be wasteful, won't all callers have done that?)
|
||||
*/
|
||||
slist = pg_plan_queries(slist, plansource->cursor_options, NULL,
|
||||
true);
|
||||
/* Generate plans for queries */
|
||||
slist = do_planning(slist, plansource->cursor_options);
|
||||
}
|
||||
|
||||
/*
|
||||
@ -522,6 +519,49 @@ RevalidateCachedPlan(CachedPlanSource *plansource, bool useResOwner)
|
||||
return plan;
|
||||
}
|
||||
|
||||
/*
|
||||
* Invoke the planner on some rewritten queries. This is broken out of
|
||||
* RevalidateCachedPlan just to avoid plastering "volatile" all over that
|
||||
* function's variables.
|
||||
*/
|
||||
static List *
|
||||
do_planning(List *querytrees, int cursorOptions)
|
||||
{
|
||||
List *stmt_list;
|
||||
|
||||
/*
|
||||
* If a snapshot is already set (the normal case), we can just use that
|
||||
* for planning. But if it isn't, we have to tell pg_plan_queries to make
|
||||
* a snap if it needs one. In that case we should arrange to reset
|
||||
* ActiveSnapshot afterward, to ensure that RevalidateCachedPlan has no
|
||||
* caller-visible effects on the snapshot. Having to replan is an unusual
|
||||
* case, and it seems a really bad idea for RevalidateCachedPlan to affect
|
||||
* the snapshot only in unusual cases. (Besides, the snap might have
|
||||
* been created in a short-lived context.)
|
||||
*/
|
||||
if (ActiveSnapshot != NULL)
|
||||
stmt_list = pg_plan_queries(querytrees, cursorOptions, NULL, false);
|
||||
else
|
||||
{
|
||||
PG_TRY();
|
||||
{
|
||||
stmt_list = pg_plan_queries(querytrees, cursorOptions, NULL, true);
|
||||
}
|
||||
PG_CATCH();
|
||||
{
|
||||
/* Restore global vars and propagate error */
|
||||
ActiveSnapshot = NULL;
|
||||
PG_RE_THROW();
|
||||
}
|
||||
PG_END_TRY();
|
||||
|
||||
ActiveSnapshot = NULL;
|
||||
}
|
||||
|
||||
return stmt_list;
|
||||
}
|
||||
|
||||
|
||||
/*
|
||||
* ReleaseCachedPlan: release active use of a cached plan.
|
||||
*
|
||||
|
Loading…
Reference in New Issue
Block a user