Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit c4ae968

Browse files
committed
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.
1 parent 99b5454 commit c4ae968

File tree

1 file changed

+15
-2
lines changed

1 file changed

+15
-2
lines changed

src/backend/utils/cache/plancache.c

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -718,8 +718,21 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
718718
MemoryContext plan_context;
719719
MemoryContext oldcxt;
720720

721-
/* Assert that caller checked the querytree */
722-
Assert(plansource->is_valid);
721+
/*
722+
* Normally the querytree should be valid already, but if it's not,
723+
* rebuild it.
724+
*
725+
* NOTE: GetCachedPlan should have called RevalidateCachedQuery first, so
726+
* we ought to be holding sufficient locks to prevent any invalidation.
727+
* However, if we're building a custom plan after having built and
728+
* rejected a generic plan, it's possible to reach here with is_valid
729+
* false due to an invalidation while making the generic plan. In theory
730+
* the invalidation must be a false positive, perhaps a consequence of an
731+
* sinval reset event or the CLOBBER_CACHE_ALWAYS debug code. But for
732+
* safety, let's treat it as real and redo the RevalidateCachedQuery call.
733+
*/
734+
if (!plansource->is_valid)
735+
qlist = RevalidateCachedQuery(plansource);
723736

724737
/*
725738
* If we don't already have a copy of the querytree list that can be

0 commit comments

Comments
 (0)