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

Commit1fa46db

Browse files
committed
When replanning a plpgsql "simple expression", check it's still simple.
The previous coding here assumed that we didn't need to recheck anyof the querytree tests made in exec_simple_check_plan(). I thinkwe supposed that those properties were fully determined by thesyntax of the source text and hence couldn't change. That is truefor most of them, but at least hasTargetSRFs and hasAggs can changeby dint of forcibly dropping an originally-referenced function andrecreating it with new properties. That leads to "unexpected plannode type" or similar failures.These tests are pretty cheap compared to the cost of replanning, sorather than sweat over exactly which properties need to be rechecked,let's just recheck them all. Hence, factor out those tests into a newfunction exec_is_simple_query(), and rearrange callers as needed.A second problem in the same area was that if we failed duringreplanning or during exec_save_simple_expr(), we'd potentiallyleave behind now-dangling pointers to the old simple expression,potentially resulting in crashes later. To fix, clear those pointersbefore replanning.The v12 code looks quite different in this area but still has thebug about needing to recheck query simplicity. I chose to back-patchall of the plpgsql_simple.sql test script, which formerly didn't existin this branch.Per bug #18497 from Nikita Kalinin. Back-patch to all supportedbranches.Discussion:https://postgr.es/m/18497-fe93b6da82ce31d4@postgresql.org
1 parentb91b3f0 commit1fa46db

File tree

3 files changed

+137
-53
lines changed

3 files changed

+137
-53
lines changed

‎src/pl/plpgsql/src/expected/plpgsql_simple.out

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,32 @@ select simplecaller();
8989
4
9090
(1 row)
9191

92+
-- Check case where called function changes from non-SRF to SRF (bug #18497)
93+
create or replace function simplecaller() returns int language plpgsql
94+
as $$
95+
declare x int;
96+
begin
97+
x := simplesql();
98+
return x;
99+
end$$;
100+
select simplecaller();
101+
simplecaller
102+
--------------
103+
4
104+
(1 row)
105+
106+
drop function simplesql();
107+
create function simplesql() returns setof int language sql
108+
as $$select 22 + 22$$;
109+
select simplecaller();
110+
simplecaller
111+
--------------
112+
44
113+
(1 row)
114+
115+
select simplecaller();
116+
simplecaller
117+
--------------
118+
44
119+
(1 row)
120+

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

Lines changed: 86 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@ static void exec_prepare_plan(PLpgSQL_execstate *estate,
350350
PLpgSQL_expr*expr,intcursorOptions,
351351
boolkeepplan);
352352
staticvoidexec_simple_check_plan(PLpgSQL_execstate*estate,PLpgSQL_expr*expr);
353+
staticboolexec_is_simple_query(PLpgSQL_expr*expr);
353354
staticvoidexec_save_simple_expr(PLpgSQL_expr*expr,CachedPlan*cplan);
354355
staticvoidexec_check_rw_parameter(PLpgSQL_expr*expr,inttarget_dno);
355356
staticboolcontains_target_param(Node*node,int*target_dno);
@@ -6253,10 +6254,17 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
62536254
CurrentResourceOwner=estate->simple_eval_resowner;
62546255
ReleaseCachedPlan(expr->expr_simple_plan, true);
62556256
CurrentResourceOwner=saveResourceOwner;
6256-
expr->expr_simple_plan=NULL;
6257-
expr->expr_simple_plan_lxid=InvalidLocalTransactionId;
62586257
}
62596258

6259+
/*
6260+
* Reset to "not simple" to leave sane state (with no dangling
6261+
* pointers) in case we fail while replanning. expr_simple_plansource
6262+
* can be left alone however, as that cannot move.
6263+
*/
6264+
expr->expr_simple_expr=NULL;
6265+
expr->expr_simple_plan=NULL;
6266+
expr->expr_simple_plan_lxid=InvalidLocalTransactionId;
6267+
62606268
/* Do the replanning work in the eval_mcontext */
62616269
oldcontext=MemoryContextSwitchTo(get_eval_mcontext(estate));
62626270
cplan=SPI_plan_get_cached_plan(expr->plan);
@@ -6271,11 +6279,15 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
62716279
Assert(cplan!=NULL);
62726280

62736281
/*
6274-
* This test probably can't fail either, but if it does, cope by
6275-
* declaring the plan to be non-simple. On success, we'll acquire a
6276-
* refcount on the new plan, stored in simple_eval_resowner.
6282+
* Recheck exec_is_simple_query, which could now report false in
6283+
* edge-case scenarios such as a non-SRF having been replaced with a
6284+
* SRF. Also recheck CachedPlanAllowsSimpleValidityCheck, just to be
6285+
* sure. If either test fails, cope by declaring the plan to be
6286+
* non-simple. On success, we'll acquire a refcount on the new plan,
6287+
* stored in simple_eval_resowner.
62776288
*/
6278-
if (CachedPlanAllowsSimpleValidityCheck(expr->expr_simple_plansource,
6289+
if (exec_is_simple_query(expr)&&
6290+
CachedPlanAllowsSimpleValidityCheck(expr->expr_simple_plansource,
62796291
cplan,
62806292
estate->simple_eval_resowner))
62816293
{
@@ -6287,8 +6299,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
62876299
{
62886300
/* Release SPI_plan_get_cached_plan's refcount */
62896301
ReleaseCachedPlan(cplan, true);
6290-
/* Mark expression as non-simple, and fail */
6291-
expr->expr_simple_expr=NULL;
62926302
return false;
62936303
}
62946304

@@ -8116,7 +8126,6 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
81168126
{
81178127
List*plansources;
81188128
CachedPlanSource*plansource;
8119-
Query*query;
81208129
CachedPlan*cplan;
81218130
MemoryContextoldcontext;
81228131

@@ -8131,31 +8140,88 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
81318140
* called immediately after creating the CachedPlanSource, we need not
81328141
* worry about the query being stale.
81338142
*/
8143+
if (!exec_is_simple_query(expr))
8144+
return;
8145+
8146+
/* exec_is_simple_query verified that there's just one CachedPlanSource */
8147+
plansources=SPI_plan_get_plan_sources(expr->plan);
8148+
plansource= (CachedPlanSource*)linitial(plansources);
8149+
8150+
/*
8151+
* Get the generic plan for the query. If replanning is needed, do that
8152+
* work in the eval_mcontext. (Note that replanning could throw an error,
8153+
* in which case the expr is left marked "not simple", which is fine.)
8154+
*/
8155+
oldcontext=MemoryContextSwitchTo(get_eval_mcontext(estate));
8156+
cplan=SPI_plan_get_cached_plan(expr->plan);
8157+
MemoryContextSwitchTo(oldcontext);
8158+
8159+
/* Can't fail, because we checked for a single CachedPlanSource above */
8160+
Assert(cplan!=NULL);
8161+
8162+
/*
8163+
* Verify that plancache.c thinks the plan is simple enough to use
8164+
* CachedPlanIsSimplyValid. Given the restrictions above, it's unlikely
8165+
* that this could fail, but if it does, just treat plan as not simple. On
8166+
* success, save a refcount on the plan in the simple-expression resowner.
8167+
*/
8168+
if (CachedPlanAllowsSimpleValidityCheck(plansource,cplan,
8169+
estate->simple_eval_resowner))
8170+
{
8171+
/* Remember that we have the refcount */
8172+
expr->expr_simple_plansource=plansource;
8173+
expr->expr_simple_plan=cplan;
8174+
expr->expr_simple_plan_lxid=MyProc->lxid;
8175+
8176+
/* Share the remaining work with the replan code path */
8177+
exec_save_simple_expr(expr,cplan);
8178+
}
81348179

81358180
/*
8136-
* We can only test queries that resulted in exactly one CachedPlanSource
8181+
* Release the plan refcount obtained by SPI_plan_get_cached_plan. (This
8182+
* refcount is held by the wrong resowner, so we can't just repurpose it.)
8183+
*/
8184+
ReleaseCachedPlan(cplan, true);
8185+
}
8186+
8187+
/*
8188+
* exec_is_simple_query - precheck a query tree to see if it might be simple
8189+
*
8190+
* Check the analyzed-and-rewritten form of a query to see if we will be
8191+
* able to treat it as a simple expression. It is caller's responsibility
8192+
* that the CachedPlanSource be up-to-date.
8193+
*/
8194+
staticbool
8195+
exec_is_simple_query(PLpgSQL_expr*expr)
8196+
{
8197+
List*plansources;
8198+
CachedPlanSource*plansource;
8199+
Query*query;
8200+
8201+
/*
8202+
* We can only test queries that resulted in exactly one CachedPlanSource.
81378203
*/
81388204
plansources=SPI_plan_get_plan_sources(expr->plan);
81398205
if (list_length(plansources)!=1)
8140-
return;
8206+
return false;
81418207
plansource= (CachedPlanSource*)linitial(plansources);
81428208

81438209
/*
81448210
* 1. There must be one single querytree.
81458211
*/
81468212
if (list_length(plansource->query_list)!=1)
8147-
return;
8213+
return false;
81488214
query= (Query*)linitial(plansource->query_list);
81498215

81508216
/*
8151-
* 2. It must be a plain SELECT query without any input tables
8217+
* 2. It must be a plain SELECT query without any input tables.
81528218
*/
81538219
if (!IsA(query,Query))
8154-
return;
8220+
return false;
81558221
if (query->commandType!=CMD_SELECT)
8156-
return;
8222+
return false;
81578223
if (query->rtable!=NIL)
8158-
return;
8224+
return false;
81598225

81608226
/*
81618227
* 3. Can't have any subplans, aggregates, qual clauses either. (These
@@ -8179,51 +8245,18 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
81798245
query->limitOffset||
81808246
query->limitCount||
81818247
query->setOperations)
8182-
return;
8248+
return false;
81838249

81848250
/*
8185-
* 4. The query must have a single attribute as result
8251+
* 4. The query must have a single attribute as result.
81868252
*/
81878253
if (list_length(query->targetList)!=1)
8188-
return;
8254+
return false;
81898255

81908256
/*
81918257
* OK, we can treat it as a simple plan.
8192-
*
8193-
* Get the generic plan for the query. If replanning is needed, do that
8194-
* work in the eval_mcontext. (Note that replanning could throw an error,
8195-
* in which case the expr is left marked "not simple", which is fine.)
81968258
*/
8197-
oldcontext=MemoryContextSwitchTo(get_eval_mcontext(estate));
8198-
cplan=SPI_plan_get_cached_plan(expr->plan);
8199-
MemoryContextSwitchTo(oldcontext);
8200-
8201-
/* Can't fail, because we checked for a single CachedPlanSource above */
8202-
Assert(cplan!=NULL);
8203-
8204-
/*
8205-
* Verify that plancache.c thinks the plan is simple enough to use
8206-
* CachedPlanIsSimplyValid. Given the restrictions above, it's unlikely
8207-
* that this could fail, but if it does, just treat plan as not simple. On
8208-
* success, save a refcount on the plan in the simple-expression resowner.
8209-
*/
8210-
if (CachedPlanAllowsSimpleValidityCheck(plansource,cplan,
8211-
estate->simple_eval_resowner))
8212-
{
8213-
/* Remember that we have the refcount */
8214-
expr->expr_simple_plansource=plansource;
8215-
expr->expr_simple_plan=cplan;
8216-
expr->expr_simple_plan_lxid=MyProc->lxid;
8217-
8218-
/* Share the remaining work with the replan code path */
8219-
exec_save_simple_expr(expr,cplan);
8220-
}
8221-
8222-
/*
8223-
* Release the plan refcount obtained by SPI_plan_get_cached_plan. (This
8224-
* refcount is held by the wrong resowner, so we can't just repurpose it.)
8225-
*/
8226-
ReleaseCachedPlan(cplan, true);
8259+
return true;
82278260
}
82288261

82298262
/*

‎src/pl/plpgsql/src/sql/plpgsql_simple.sql

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,25 @@ create or replace function simplesql() returns int language sql
8080
as $$select2+2$$;
8181

8282
select simplecaller();
83+
84+
85+
-- Check case where called function changes from non-SRF to SRF (bug #18497)
86+
87+
create or replacefunctionsimplecaller() returnsint language plpgsql
88+
as $$
89+
declare xint;
90+
begin
91+
x := simplesql();
92+
return x;
93+
end$$;
94+
95+
select simplecaller();
96+
97+
dropfunction simplesql();
98+
99+
createfunctionsimplesql() returns setofint language sql
100+
as $$select22+22$$;
101+
102+
select simplecaller();
103+
104+
select simplecaller();

0 commit comments

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