From c4ae968633b37e3b0a5da83e9d7d2a5518191564 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 17 Sep 2011 01:47:33 -0400 Subject: [PATCH] Fix Assert failure in new plancache code. The regression tests were failing with CLOBBER_CACHE_ALWAYS enabled, as reported by buildfarm member jaguar. There was an Assert in BuildCachedPlan that asserted that the CachedPlanSource hadn't been invalidated since we called RevalidateCachedQuery, which in theory can't happen because we are holding locks on all the relevant database objects. However, CLOBBER_CACHE_ALWAYS generates a false positive by making an invalidation happen anyway; and on reflection, that could also occur as a result of a badly-timed sinval reset due to queue overflow. We could just remove the Assert and forge ahead with the not-really-stale querytree, but it seems safer to do another RevalidateCachedQuery call just to make real sure everything's OK. --- src/backend/utils/cache/plancache.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index 62fdf2877d..9cccc876f8 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -718,8 +718,21 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist, MemoryContext plan_context; MemoryContext oldcxt; - /* Assert that caller checked the querytree */ - Assert(plansource->is_valid); + /* + * Normally the querytree should be valid already, but if it's not, + * rebuild it. + * + * NOTE: GetCachedPlan should have called RevalidateCachedQuery first, so + * we ought to be holding sufficient locks to prevent any invalidation. + * However, if we're building a custom plan after having built and + * rejected a generic plan, it's possible to reach here with is_valid + * false due to an invalidation while making the generic plan. In theory + * the invalidation must be a false positive, perhaps a consequence of an + * sinval reset event or the CLOBBER_CACHE_ALWAYS debug code. But for + * safety, let's treat it as real and redo the RevalidateCachedQuery call. + */ + if (!plansource->is_valid) + qlist = RevalidateCachedQuery(plansource); /* * If we don't already have a copy of the querytree list that can be -- 2.49.0