Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitfbc7a71

Browse files
committed
Rearrange validity checks for plpgsql "simple" expressions.
Buildfarm experience shows what probably should've occurred to me before:if a cache flush occurs partway through building a generic plan, thenthe plansource may have is_valid = false even though the plan is valid.We need to accept this case, use the generated plan, and then try toreplan the next time. We can't try to replan immediately, because thatwould produce an infinite loop in CLOBBER_CACHE_ALWAYS builds; moreoverit's really overkill. (We can assume that the plan is valid, it's justpossibly a bit stale. Note that the pre-existing code behaved this way,and the non-simple-expression code paths do too.) Conversely, not usingthe generated plan would drop us into the not-a-simple-expression codepath, which is bad for performance and would also cause regression-testfailures due to visibly different error-reporting behavior.Hence, refactor the validity-check functions so that the initial checkand recheck cases can react differently to plansource->is_valid.This makes their usage a bit simpler, too.Discussion:https://postgr.es/m/7072.1585332104@sss.pgh.pa.us
1 parent8d1b964 commitfbc7a71

File tree

3 files changed

+42
-26
lines changed

3 files changed

+42
-26
lines changed

‎src/backend/utils/cache/plancache.c

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,6 +1290,10 @@ ReleaseCachedPlan(CachedPlan *plan, bool useResOwner)
12901290
* to be invalidated, for example due to a change in a function that was
12911291
* inlined into the plan.)
12921292
*
1293+
* If the plan is simply valid, and "owner" is not NULL, record a refcount on
1294+
* the plan in that resowner before returning. It is caller's responsibility
1295+
* to be sure that a refcount is held on any plan that's being actively used.
1296+
*
12931297
* This must only be called on known-valid generic plans (eg, ones just
12941298
* returned by GetCachedPlan). If it returns true, the caller may re-use
12951299
* the cached plan as long as CachedPlanIsSimplyValid returns true; that
@@ -1298,16 +1302,24 @@ ReleaseCachedPlan(CachedPlan *plan, bool useResOwner)
12981302
*/
12991303
bool
13001304
CachedPlanAllowsSimpleValidityCheck(CachedPlanSource*plansource,
1301-
CachedPlan*plan)
1305+
CachedPlan*plan,ResourceOwnerowner)
13021306
{
13031307
ListCell*lc;
13041308

1305-
/* Sanity-check that the caller gave us a validated generic plan. */
1309+
/*
1310+
* Sanity-check that the caller gave us a validated generic plan. Notice
1311+
* that we *don't* assert plansource->is_valid as you might expect; that's
1312+
* because it's possible that that's already false when GetCachedPlan
1313+
* returns, e.g. because ResetPlanCache happened partway through. We
1314+
* should accept the plan as long as plan->is_valid is true, and expect to
1315+
* replan after the next CachedPlanIsSimplyValid call.
1316+
*/
13061317
Assert(plansource->magic==CACHEDPLANSOURCE_MAGIC);
13071318
Assert(plan->magic==CACHEDPLAN_MAGIC);
1308-
Assert(plansource->is_valid);
13091319
Assert(plan->is_valid);
13101320
Assert(plan==plansource->gplan);
1321+
Assert(plansource->search_path!=NULL);
1322+
Assert(OverrideSearchPathMatchesCurrent(plansource->search_path));
13111323

13121324
/* We don't support oneshot plans here. */
13131325
if (plansource->is_oneshot)
@@ -1371,6 +1383,15 @@ CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource,
13711383
* Okay, it's simple. Note that what we've primarily established here is
13721384
* that no locks need be taken before checking the plan's is_valid flag.
13731385
*/
1386+
1387+
/* Bump refcount if requested. */
1388+
if (owner)
1389+
{
1390+
ResourceOwnerEnlargePlanCacheRefs(owner);
1391+
plan->refcount++;
1392+
ResourceOwnerRememberPlanCacheRef(owner,plan);
1393+
}
1394+
13741395
return true;
13751396
}
13761397

@@ -1408,7 +1429,9 @@ CachedPlanIsSimplyValid(CachedPlanSource *plansource, CachedPlan *plan,
14081429

14091430
/*
14101431
* Has cache invalidation fired on this plan? We can check this right
1411-
* away since there are no locks that we'd need to acquire first.
1432+
* away since there are no locks that we'd need to acquire first. Note
1433+
* that here we *do* check plansource->is_valid, so as to force plan
1434+
* rebuild if that's become false.
14121435
*/
14131436
if (!plansource->is_valid||plan!=plansource->gplan|| !plan->is_valid)
14141437
return false;

‎src/include/utils/plancache.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,8 @@ extern CachedPlan *GetCachedPlan(CachedPlanSource *plansource,
223223
externvoidReleaseCachedPlan(CachedPlan*plan,booluseResOwner);
224224

225225
externboolCachedPlanAllowsSimpleValidityCheck(CachedPlanSource*plansource,
226-
CachedPlan*plan);
226+
CachedPlan*plan,
227+
ResourceOwnerowner);
227228
externboolCachedPlanIsSimplyValid(CachedPlanSource*plansource,
228229
CachedPlan*plan,
229230
ResourceOwnerowner);

‎src/pl/plpgsql/src/pl_exec.c

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6181,14 +6181,13 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
61816181
Assert(cplan!=NULL);
61826182

61836183
/*
6184-
*These tests probably can't fail either, but ifthey do, cope by
6184+
*This test probably can't fail either, but ifit does, cope by
61856185
* declaring the plan to be non-simple. On success, we'll acquire a
61866186
* refcount on the new plan, stored in simple_eval_resowner.
61876187
*/
61886188
if (CachedPlanAllowsSimpleValidityCheck(expr->expr_simple_plansource,
6189-
cplan)&&
6190-
CachedPlanIsSimplyValid(expr->expr_simple_plansource,cplan,
6191-
estate->simple_eval_resowner))
6189+
cplan,
6190+
estate->simple_eval_resowner))
61926191
{
61936192
/* Remember that we have the refcount */
61946193
expr->expr_simple_plan=cplan;
@@ -8089,26 +8088,19 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
80898088
/*
80908089
* Verify that plancache.c thinks the plan is simple enough to use
80918090
* CachedPlanIsSimplyValid. Given the restrictions above, it's unlikely
8092-
* that this could fail, but if it does, just treat plan as not simple.
8091+
* that this could fail, but if it does, just treat plan as not simple. On
8092+
* success, save a refcount on the plan in the simple-expression resowner.
80938093
*/
8094-
if (CachedPlanAllowsSimpleValidityCheck(plansource,cplan))
8094+
if (CachedPlanAllowsSimpleValidityCheck(plansource,cplan,
8095+
estate->simple_eval_resowner))
80958096
{
8096-
/*
8097-
* OK, use CachedPlanIsSimplyValid to save a refcount on the plan in
8098-
* the simple-expression resowner. This shouldn't fail either, but if
8099-
* somehow it does, again we can cope by treating plan as not simple.
8100-
*/
8101-
if (CachedPlanIsSimplyValid(plansource,cplan,
8102-
estate->simple_eval_resowner))
8103-
{
8104-
/* Remember that we have the refcount */
8105-
expr->expr_simple_plansource=plansource;
8106-
expr->expr_simple_plan=cplan;
8107-
expr->expr_simple_plan_lxid=MyProc->lxid;
8097+
/* Remember that we have the refcount */
8098+
expr->expr_simple_plansource=plansource;
8099+
expr->expr_simple_plan=cplan;
8100+
expr->expr_simple_plan_lxid=MyProc->lxid;
81088101

8109-
/* Share the remaining work with the replan code path */
8110-
exec_save_simple_expr(expr,cplan);
8111-
}
8102+
/* Share the remaining work with the replan code path */
8103+
exec_save_simple_expr(expr,cplan);
81128104
}
81138105

81148106
/*

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp