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

Commitbf552b1

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 parent7768ac1 commitbf552b1

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);
@@ -6086,12 +6087,18 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
60866087
* release it, so we don't leak plans intra-transaction.
60876088
*/
60886089
if (expr->expr_simple_plan_lxid==curlxid)
6089-
{
60906090
ReleaseCachedPlan(expr->expr_simple_plan,
60916091
estate->simple_eval_resowner);
6092-
expr->expr_simple_plan=NULL;
6093-
expr->expr_simple_plan_lxid=InvalidLocalTransactionId;
6094-
}
6092+
6093+
/*
6094+
* Reset to "not simple" to leave sane state (with no dangling
6095+
* pointers) in case we fail while replanning. expr_simple_plansource
6096+
* can be left alone however, as that cannot move.
6097+
*/
6098+
expr->expr_simple_expr=NULL;
6099+
expr->expr_rw_param=NULL;
6100+
expr->expr_simple_plan=NULL;
6101+
expr->expr_simple_plan_lxid=InvalidLocalTransactionId;
60956102

60966103
/* Do the replanning work in the eval_mcontext */
60976104
oldcontext=MemoryContextSwitchTo(get_eval_mcontext(estate));
@@ -6107,11 +6114,15 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
61076114
Assert(cplan!=NULL);
61086115

61096116
/*
6110-
* This test probably can't fail either, but if it does, cope by
6111-
* declaring the plan to be non-simple. On success, we'll acquire a
6112-
* refcount on the new plan, stored in simple_eval_resowner.
6117+
* Recheck exec_is_simple_query, which could now report false in
6118+
* edge-case scenarios such as a non-SRF having been replaced with a
6119+
* SRF. Also recheck CachedPlanAllowsSimpleValidityCheck, just to be
6120+
* sure. If either test fails, cope by declaring the plan to be
6121+
* non-simple. On success, we'll acquire a refcount on the new plan,
6122+
* stored in simple_eval_resowner.
61136123
*/
6114-
if (CachedPlanAllowsSimpleValidityCheck(expr->expr_simple_plansource,
6124+
if (exec_is_simple_query(expr)&&
6125+
CachedPlanAllowsSimpleValidityCheck(expr->expr_simple_plansource,
61156126
cplan,
61166127
estate->simple_eval_resowner))
61176128
{
@@ -6123,9 +6134,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
61236134
{
61246135
/* Release SPI_plan_get_cached_plan's refcount */
61256136
ReleaseCachedPlan(cplan,CurrentResourceOwner);
6126-
/* Mark expression as non-simple, and fail */
6127-
expr->expr_simple_expr=NULL;
6128-
expr->expr_rw_param=NULL;
61296137
return false;
61306138
}
61316139

@@ -7969,7 +7977,6 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
79697977
{
79707978
List*plansources;
79717979
CachedPlanSource*plansource;
7972-
Query*query;
79737980
CachedPlan*cplan;
79747981
MemoryContextoldcontext;
79757982

@@ -7985,31 +7992,88 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
79857992
* called immediately after creating the CachedPlanSource, we need not
79867993
* worry about the query being stale.
79877994
*/
7995+
if (!exec_is_simple_query(expr))
7996+
return;
7997+
7998+
/* exec_is_simple_query verified that there's just one CachedPlanSource */
7999+
plansources=SPI_plan_get_plan_sources(expr->plan);
8000+
plansource= (CachedPlanSource*)linitial(plansources);
79888001

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

79978061
/*
79988062
* 1. There must be one single querytree.
79998063
*/
80008064
if (list_length(plansource->query_list)!=1)
8001-
return;
8065+
return false;
80028066
query= (Query*)linitial(plansource->query_list);
80038067

80048068
/*
8005-
* 2. It must be a plain SELECT query without any input tables
8069+
* 2. It must be a plain SELECT query without any input tables.
80068070
*/
80078071
if (!IsA(query,Query))
8008-
return;
8072+
return false;
80098073
if (query->commandType!=CMD_SELECT)
8010-
return;
8074+
return false;
80118075
if (query->rtable!=NIL)
8012-
return;
8076+
return false;
80138077

80148078
/*
80158079
* 3. Can't have any subplans, aggregates, qual clauses either. (These
@@ -8033,51 +8097,18 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
80338097
query->limitOffset||
80348098
query->limitCount||
80358099
query->setOperations)
8036-
return;
8100+
return false;
80378101

80388102
/*
8039-
* 4. The query must have a single attribute as result
8103+
* 4. The query must have a single attribute as result.
80408104
*/
80418105
if (list_length(query->targetList)!=1)
8042-
return;
8106+
return false;
80438107

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

80838114
/*

‎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