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

Commit6dfac24

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 parentc425113 commit6dfac24

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);
@@ -6094,12 +6095,18 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
60946095
* release it, so we don't leak plans intra-transaction.
60956096
*/
60966097
if (expr->expr_simple_plan_lxid==curlxid)
6097-
{
60986098
ReleaseCachedPlan(expr->expr_simple_plan,
60996099
estate->simple_eval_resowner);
6100-
expr->expr_simple_plan=NULL;
6101-
expr->expr_simple_plan_lxid=InvalidLocalTransactionId;
6102-
}
6100+
6101+
/*
6102+
* Reset to "not simple" to leave sane state (with no dangling
6103+
* pointers) in case we fail while replanning. expr_simple_plansource
6104+
* can be left alone however, as that cannot move.
6105+
*/
6106+
expr->expr_simple_expr=NULL;
6107+
expr->expr_rw_param=NULL;
6108+
expr->expr_simple_plan=NULL;
6109+
expr->expr_simple_plan_lxid=InvalidLocalTransactionId;
61036110

61046111
/* Do the replanning work in the eval_mcontext */
61056112
oldcontext=MemoryContextSwitchTo(get_eval_mcontext(estate));
@@ -6115,11 +6122,15 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
61156122
Assert(cplan!=NULL);
61166123

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

@@ -7974,7 +7982,6 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
79747982
{
79757983
List*plansources;
79767984
CachedPlanSource*plansource;
7977-
Query*query;
79787985
CachedPlan*cplan;
79797986
MemoryContextoldcontext;
79807987

@@ -7990,31 +7997,88 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
79907997
* called immediately after creating the CachedPlanSource, we need not
79917998
* worry about the query being stale.
79927999
*/
8000+
if (!exec_is_simple_query(expr))
8001+
return;
8002+
8003+
/* exec_is_simple_query verified that there's just one CachedPlanSource */
8004+
plansources=SPI_plan_get_plan_sources(expr->plan);
8005+
plansource= (CachedPlanSource*)linitial(plansources);
79938006

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

80028066
/*
80038067
* 1. There must be one single querytree.
80048068
*/
80058069
if (list_length(plansource->query_list)!=1)
8006-
return;
8070+
return false;
80078071
query= (Query*)linitial(plansource->query_list);
80088072

80098073
/*
8010-
* 2. It must be a plain SELECT query without any input tables
8074+
* 2. It must be a plain SELECT query without any input tables.
80118075
*/
80128076
if (!IsA(query,Query))
8013-
return;
8077+
return false;
80148078
if (query->commandType!=CMD_SELECT)
8015-
return;
8079+
return false;
80168080
if (query->rtable!=NIL)
8017-
return;
8081+
return false;
80188082

80198083
/*
80208084
* 3. Can't have any subplans, aggregates, qual clauses either. (These
@@ -8038,51 +8102,18 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
80388102
query->limitOffset||
80398103
query->limitCount||
80408104
query->setOperations)
8041-
return;
8105+
return false;
80428106

80438107
/*
8044-
* 4. The query must have a single attribute as result
8108+
* 4. The query must have a single attribute as result.
80458109
*/
80468110
if (list_length(query->targetList)!=1)
8047-
return;
8111+
return false;
80488112

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

80888119
/*

‎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