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

Commit1450db7

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 parentd881f6f commit1450db7

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
staticboolexec_eval_simple_expr(PLpgSQL_execstate*estate,
@@ -6036,12 +6037,18 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
60366037
* release it, so we don't leak plans intra-transaction.
60376038
*/
60386039
if (expr->expr_simple_plan_lxid==curlxid)
6039-
{
60406040
ReleaseCachedPlan(expr->expr_simple_plan,
60416041
estate->simple_eval_resowner);
6042-
expr->expr_simple_plan=NULL;
6043-
expr->expr_simple_plan_lxid=InvalidLocalTransactionId;
6044-
}
6042+
6043+
/*
6044+
* Reset to "not simple" to leave sane state (with no dangling
6045+
* pointers) in case we fail while replanning. expr_simple_plansource
6046+
* can be left alone however, as that cannot move.
6047+
*/
6048+
expr->expr_simple_expr=NULL;
6049+
expr->expr_rw_param=NULL;
6050+
expr->expr_simple_plan=NULL;
6051+
expr->expr_simple_plan_lxid=InvalidLocalTransactionId;
60456052

60466053
/* Do the replanning work in the eval_mcontext */
60476054
oldcontext=MemoryContextSwitchTo(get_eval_mcontext(estate));
@@ -6057,11 +6064,15 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
60576064
Assert(cplan!=NULL);
60586065

60596066
/*
6060-
* This test probably can't fail either, but if it does, cope by
6061-
* declaring the plan to be non-simple. On success, we'll acquire a
6062-
* refcount on the new plan, stored in simple_eval_resowner.
6067+
* Recheck exec_is_simple_query, which could now report false in
6068+
* edge-case scenarios such as a non-SRF having been replaced with a
6069+
* SRF. Also recheck CachedPlanAllowsSimpleValidityCheck, just to be
6070+
* sure. If either test fails, cope by declaring the plan to be
6071+
* non-simple. On success, we'll acquire a refcount on the new plan,
6072+
* stored in simple_eval_resowner.
60636073
*/
6064-
if (CachedPlanAllowsSimpleValidityCheck(expr->expr_simple_plansource,
6074+
if (exec_is_simple_query(expr)&&
6075+
CachedPlanAllowsSimpleValidityCheck(expr->expr_simple_plansource,
60656076
cplan,
60666077
estate->simple_eval_resowner))
60676078
{
@@ -6073,9 +6084,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
60736084
{
60746085
/* Release SPI_plan_get_cached_plan's refcount */
60756086
ReleaseCachedPlan(cplan,CurrentResourceOwner);
6076-
/* Mark expression as non-simple, and fail */
6077-
expr->expr_simple_expr=NULL;
6078-
expr->expr_rw_param=NULL;
60796087
return false;
60806088
}
60816089

@@ -7919,7 +7927,6 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
79197927
{
79207928
List*plansources;
79217929
CachedPlanSource*plansource;
7922-
Query*query;
79237930
CachedPlan*cplan;
79247931
MemoryContextoldcontext;
79257932

@@ -7935,31 +7942,88 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
79357942
* called immediately after creating the CachedPlanSource, we need not
79367943
* worry about the query being stale.
79377944
*/
7945+
if (!exec_is_simple_query(expr))
7946+
return;
7947+
7948+
/* exec_is_simple_query verified that there's just one CachedPlanSource */
7949+
plansources=SPI_plan_get_plan_sources(expr->plan);
7950+
plansource= (CachedPlanSource*)linitial(plansources);
79387951

79397952
/*
7940-
* We can only test queries that resulted in exactly one CachedPlanSource
7953+
* Get the generic plan for the query. If replanning is needed, do that
7954+
* work in the eval_mcontext. (Note that replanning could throw an error,
7955+
* in which case the expr is left marked "not simple", which is fine.)
7956+
*/
7957+
oldcontext=MemoryContextSwitchTo(get_eval_mcontext(estate));
7958+
cplan=SPI_plan_get_cached_plan(expr->plan);
7959+
MemoryContextSwitchTo(oldcontext);
7960+
7961+
/* Can't fail, because we checked for a single CachedPlanSource above */
7962+
Assert(cplan!=NULL);
7963+
7964+
/*
7965+
* Verify that plancache.c thinks the plan is simple enough to use
7966+
* CachedPlanIsSimplyValid. Given the restrictions above, it's unlikely
7967+
* that this could fail, but if it does, just treat plan as not simple. On
7968+
* success, save a refcount on the plan in the simple-expression resowner.
7969+
*/
7970+
if (CachedPlanAllowsSimpleValidityCheck(plansource,cplan,
7971+
estate->simple_eval_resowner))
7972+
{
7973+
/* Remember that we have the refcount */
7974+
expr->expr_simple_plansource=plansource;
7975+
expr->expr_simple_plan=cplan;
7976+
expr->expr_simple_plan_lxid=MyProc->lxid;
7977+
7978+
/* Share the remaining work with the replan code path */
7979+
exec_save_simple_expr(expr,cplan);
7980+
}
7981+
7982+
/*
7983+
* Release the plan refcount obtained by SPI_plan_get_cached_plan. (This
7984+
* refcount is held by the wrong resowner, so we can't just repurpose it.)
7985+
*/
7986+
ReleaseCachedPlan(cplan,CurrentResourceOwner);
7987+
}
7988+
7989+
/*
7990+
* exec_is_simple_query - precheck a query tree to see if it might be simple
7991+
*
7992+
* Check the analyzed-and-rewritten form of a query to see if we will be
7993+
* able to treat it as a simple expression. It is caller's responsibility
7994+
* that the CachedPlanSource be up-to-date.
7995+
*/
7996+
staticbool
7997+
exec_is_simple_query(PLpgSQL_expr*expr)
7998+
{
7999+
List*plansources;
8000+
CachedPlanSource*plansource;
8001+
Query*query;
8002+
8003+
/*
8004+
* We can only test queries that resulted in exactly one CachedPlanSource.
79418005
*/
79428006
plansources=SPI_plan_get_plan_sources(expr->plan);
79438007
if (list_length(plansources)!=1)
7944-
return;
8008+
return false;
79458009
plansource= (CachedPlanSource*)linitial(plansources);
79468010

79478011
/*
79488012
* 1. There must be one single querytree.
79498013
*/
79508014
if (list_length(plansource->query_list)!=1)
7951-
return;
8015+
return false;
79528016
query= (Query*)linitial(plansource->query_list);
79538017

79548018
/*
7955-
* 2. It must be a plain SELECT query without any input tables
8019+
* 2. It must be a plain SELECT query without any input tables.
79568020
*/
79578021
if (!IsA(query,Query))
7958-
return;
8022+
return false;
79598023
if (query->commandType!=CMD_SELECT)
7960-
return;
8024+
return false;
79618025
if (query->rtable!=NIL)
7962-
return;
8026+
return false;
79638027

79648028
/*
79658029
* 3. Can't have any subplans, aggregates, qual clauses either. (These
@@ -7983,51 +8047,18 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
79838047
query->limitOffset||
79848048
query->limitCount||
79858049
query->setOperations)
7986-
return;
8050+
return false;
79878051

79888052
/*
7989-
* 4. The query must have a single attribute as result
8053+
* 4. The query must have a single attribute as result.
79908054
*/
79918055
if (list_length(query->targetList)!=1)
7992-
return;
8056+
return false;
79938057

79948058
/*
79958059
* OK, we can treat it as a simple plan.
7996-
*
7997-
* Get the generic plan for the query. If replanning is needed, do that
7998-
* work in the eval_mcontext. (Note that replanning could throw an error,
7999-
* in which case the expr is left marked "not simple", which is fine.)
8000-
*/
8001-
oldcontext=MemoryContextSwitchTo(get_eval_mcontext(estate));
8002-
cplan=SPI_plan_get_cached_plan(expr->plan);
8003-
MemoryContextSwitchTo(oldcontext);
8004-
8005-
/* Can't fail, because we checked for a single CachedPlanSource above */
8006-
Assert(cplan!=NULL);
8007-
8008-
/*
8009-
* Verify that plancache.c thinks the plan is simple enough to use
8010-
* CachedPlanIsSimplyValid. Given the restrictions above, it's unlikely
8011-
* that this could fail, but if it does, just treat plan as not simple. On
8012-
* success, save a refcount on the plan in the simple-expression resowner.
8013-
*/
8014-
if (CachedPlanAllowsSimpleValidityCheck(plansource,cplan,
8015-
estate->simple_eval_resowner))
8016-
{
8017-
/* Remember that we have the refcount */
8018-
expr->expr_simple_plansource=plansource;
8019-
expr->expr_simple_plan=cplan;
8020-
expr->expr_simple_plan_lxid=MyProc->lxid;
8021-
8022-
/* Share the remaining work with the replan code path */
8023-
exec_save_simple_expr(expr,cplan);
8024-
}
8025-
8026-
/*
8027-
* Release the plan refcount obtained by SPI_plan_get_cached_plan. (This
8028-
* refcount is held by the wrong resowner, so we can't just repurpose it.)
80298060
*/
8030-
ReleaseCachedPlan(cplan,CurrentResourceOwner);
8061+
return true;
80318062
}
80328063

80338064
/*

‎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