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

Commit82a931d

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 parent566a7d9 commit82a931d

File tree

3 files changed

+138
-56
lines changed

3 files changed

+138
-56
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: 87 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,7 @@ static void exec_eval_cleanup(PLpgSQL_execstate *estate);
339339
staticvoidexec_prepare_plan(PLpgSQL_execstate*estate,
340340
PLpgSQL_expr*expr,intcursorOptions);
341341
staticvoidexec_simple_check_plan(PLpgSQL_execstate*estate,PLpgSQL_expr*expr);
342+
staticboolexec_is_simple_query(PLpgSQL_expr*expr);
342343
staticvoidexec_save_simple_expr(PLpgSQL_expr*expr,CachedPlan*cplan);
343344
staticvoidexec_check_rw_parameter(PLpgSQL_expr*expr);
344345
staticvoidexec_check_assignable(PLpgSQL_execstate*estate,intdno);
@@ -6092,12 +6093,18 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
60926093
* release it, so we don't leak plans intra-transaction.
60936094
*/
60946095
if (expr->expr_simple_plan_lxid==curlxid)
6095-
{
60966096
ReleaseCachedPlan(expr->expr_simple_plan,
60976097
estate->simple_eval_resowner);
6098-
expr->expr_simple_plan=NULL;
6099-
expr->expr_simple_plan_lxid=InvalidLocalTransactionId;
6100-
}
6098+
6099+
/*
6100+
* Reset to "not simple" to leave sane state (with no dangling
6101+
* pointers) in case we fail while replanning. expr_simple_plansource
6102+
* can be left alone however, as that cannot move.
6103+
*/
6104+
expr->expr_simple_expr=NULL;
6105+
expr->expr_rw_param=NULL;
6106+
expr->expr_simple_plan=NULL;
6107+
expr->expr_simple_plan_lxid=InvalidLocalTransactionId;
61016108

61026109
/* Do the replanning work in the eval_mcontext */
61036110
oldcontext=MemoryContextSwitchTo(get_eval_mcontext(estate));
@@ -6113,11 +6120,15 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
61136120
Assert(cplan!=NULL);
61146121

61156122
/*
6116-
* This test probably can't fail either, but if it does, cope by
6117-
* declaring the plan to be non-simple. On success, we'll acquire a
6118-
* refcount on the new plan, stored in simple_eval_resowner.
6123+
* Recheck exec_is_simple_query, which could now report false in
6124+
* edge-case scenarios such as a non-SRF having been replaced with a
6125+
* SRF. Also recheck CachedPlanAllowsSimpleValidityCheck, just to be
6126+
* sure. If either test fails, cope by declaring the plan to be
6127+
* non-simple. On success, we'll acquire a refcount on the new plan,
6128+
* stored in simple_eval_resowner.
61196129
*/
6120-
if (CachedPlanAllowsSimpleValidityCheck(expr->expr_simple_plansource,
6130+
if (exec_is_simple_query(expr)&&
6131+
CachedPlanAllowsSimpleValidityCheck(expr->expr_simple_plansource,
61216132
cplan,
61226133
estate->simple_eval_resowner))
61236134
{
@@ -6129,9 +6140,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
61296140
{
61306141
/* Release SPI_plan_get_cached_plan's refcount */
61316142
ReleaseCachedPlan(cplan,CurrentResourceOwner);
6132-
/* Mark expression as non-simple, and fail */
6133-
expr->expr_simple_expr=NULL;
6134-
expr->expr_rw_param=NULL;
61356143
return false;
61366144
}
61376145

@@ -7972,7 +7980,6 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
79727980
{
79737981
List*plansources;
79747982
CachedPlanSource*plansource;
7975-
Query*query;
79767983
CachedPlan*cplan;
79777984
MemoryContextoldcontext;
79787985

@@ -7988,31 +7995,88 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
79887995
* called immediately after creating the CachedPlanSource, we need not
79897996
* worry about the query being stale.
79907997
*/
7998+
if (!exec_is_simple_query(expr))
7999+
return;
8000+
8001+
/* exec_is_simple_query verified that there's just one CachedPlanSource */
8002+
plansources=SPI_plan_get_plan_sources(expr->plan);
8003+
plansource= (CachedPlanSource*)linitial(plansources);
79918004

79928005
/*
7993-
* We can only test queries that resulted in exactly one CachedPlanSource
8006+
* Get the generic plan for the query. If replanning is needed, do that
8007+
* work in the eval_mcontext. (Note that replanning could throw an error,
8008+
* in which case the expr is left marked "not simple", which is fine.)
8009+
*/
8010+
oldcontext=MemoryContextSwitchTo(get_eval_mcontext(estate));
8011+
cplan=SPI_plan_get_cached_plan(expr->plan);
8012+
MemoryContextSwitchTo(oldcontext);
8013+
8014+
/* Can't fail, because we checked for a single CachedPlanSource above */
8015+
Assert(cplan!=NULL);
8016+
8017+
/*
8018+
* Verify that plancache.c thinks the plan is simple enough to use
8019+
* CachedPlanIsSimplyValid. Given the restrictions above, it's unlikely
8020+
* that this could fail, but if it does, just treat plan as not simple. On
8021+
* success, save a refcount on the plan in the simple-expression resowner.
8022+
*/
8023+
if (CachedPlanAllowsSimpleValidityCheck(plansource,cplan,
8024+
estate->simple_eval_resowner))
8025+
{
8026+
/* Remember that we have the refcount */
8027+
expr->expr_simple_plansource=plansource;
8028+
expr->expr_simple_plan=cplan;
8029+
expr->expr_simple_plan_lxid=MyProc->lxid;
8030+
8031+
/* Share the remaining work with the replan code path */
8032+
exec_save_simple_expr(expr,cplan);
8033+
}
8034+
8035+
/*
8036+
* Release the plan refcount obtained by SPI_plan_get_cached_plan. (This
8037+
* refcount is held by the wrong resowner, so we can't just repurpose it.)
8038+
*/
8039+
ReleaseCachedPlan(cplan,CurrentResourceOwner);
8040+
}
8041+
8042+
/*
8043+
* exec_is_simple_query - precheck a query tree to see if it might be simple
8044+
*
8045+
* Check the analyzed-and-rewritten form of a query to see if we will be
8046+
* able to treat it as a simple expression. It is caller's responsibility
8047+
* that the CachedPlanSource be up-to-date.
8048+
*/
8049+
staticbool
8050+
exec_is_simple_query(PLpgSQL_expr*expr)
8051+
{
8052+
List*plansources;
8053+
CachedPlanSource*plansource;
8054+
Query*query;
8055+
8056+
/*
8057+
* We can only test queries that resulted in exactly one CachedPlanSource.
79948058
*/
79958059
plansources=SPI_plan_get_plan_sources(expr->plan);
79968060
if (list_length(plansources)!=1)
7997-
return;
8061+
return false;
79988062
plansource= (CachedPlanSource*)linitial(plansources);
79998063

80008064
/*
80018065
* 1. There must be one single querytree.
80028066
*/
80038067
if (list_length(plansource->query_list)!=1)
8004-
return;
8068+
return false;
80058069
query= (Query*)linitial(plansource->query_list);
80068070

80078071
/*
8008-
* 2. It must be a plain SELECT query without any input tables
8072+
* 2. It must be a plain SELECT query without any input tables.
80098073
*/
80108074
if (!IsA(query,Query))
8011-
return;
8075+
return false;
80128076
if (query->commandType!=CMD_SELECT)
8013-
return;
8077+
return false;
80148078
if (query->rtable!=NIL)
8015-
return;
8079+
return false;
80168080

80178081
/*
80188082
* 3. Can't have any subplans, aggregates, qual clauses either. (These
@@ -8036,51 +8100,18 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
80368100
query->limitOffset||
80378101
query->limitCount||
80388102
query->setOperations)
8039-
return;
8103+
return false;
80408104

80418105
/*
8042-
* 4. The query must have a single attribute as result
8106+
* 4. The query must have a single attribute as result.
80438107
*/
80448108
if (list_length(query->targetList)!=1)
8045-
return;
8109+
return false;
80468110

80478111
/*
80488112
* OK, we can treat it as a simple plan.
8049-
*
8050-
* Get the generic plan for the query. If replanning is needed, do that
8051-
* work in the eval_mcontext. (Note that replanning could throw an error,
8052-
* in which case the expr is left marked "not simple", which is fine.)
8053-
*/
8054-
oldcontext=MemoryContextSwitchTo(get_eval_mcontext(estate));
8055-
cplan=SPI_plan_get_cached_plan(expr->plan);
8056-
MemoryContextSwitchTo(oldcontext);
8057-
8058-
/* Can't fail, because we checked for a single CachedPlanSource above */
8059-
Assert(cplan!=NULL);
8060-
8061-
/*
8062-
* Verify that plancache.c thinks the plan is simple enough to use
8063-
* CachedPlanIsSimplyValid. Given the restrictions above, it's unlikely
8064-
* that this could fail, but if it does, just treat plan as not simple. On
8065-
* success, save a refcount on the plan in the simple-expression resowner.
8066-
*/
8067-
if (CachedPlanAllowsSimpleValidityCheck(plansource,cplan,
8068-
estate->simple_eval_resowner))
8069-
{
8070-
/* Remember that we have the refcount */
8071-
expr->expr_simple_plansource=plansource;
8072-
expr->expr_simple_plan=cplan;
8073-
expr->expr_simple_plan_lxid=MyProc->lxid;
8074-
8075-
/* Share the remaining work with the replan code path */
8076-
exec_save_simple_expr(expr,cplan);
8077-
}
8078-
8079-
/*
8080-
* Release the plan refcount obtained by SPI_plan_get_cached_plan. (This
8081-
* refcount is held by the wrong resowner, so we can't just repurpose it.)
80828113
*/
8083-
ReleaseCachedPlan(cplan,CurrentResourceOwner);
8114+
return true;
80848115
}
80858116

80868117
/*

‎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