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

Commit3f7323c

Browse files
committed
Repair rare failure of MULTIEXPR_SUBLINK subplans in inherited updates.
Prior to v14, if we have a MULTIEXPR SubPlan (that is, use of the syntaxUPDATE ... SET (c1, ...) = (SELECT ...)) in an UPDATE with an inheritedor partitioned target table, inheritance_planner() will clone thetargetlist and therefore also the MULTIEXPR SubPlan and the Param nodesreferencing it for each child target table. Up to now, we've allowedall the clones to share the underlying subplan as well as the outputparameter IDs -- that is, the runtime ParamExecData slots. Thattechnique is borrowed from the far older code that supports initplans,and it works okay in that case because the cloned SubPlan nodes areessentially identical. So it doesn't matter which one of the clonesthe shared ParamExecData.execPlan field might point to.However, this fails to hold for MULTIEXPR SubPlans, because they canhave nonempty "args" lists (values to be passed into the subplan), andthose lists could get mutated to different states in the various clones.In the submitted reproducer, as well as the test case added here, oneclone contains Vars with varno OUTER_VAR where another has INNER_VAR,because the child tables are respectively on the outer or inner side ofthe join. Sharing the execPlan pointer can result in trying to evaluatean args list that doesn't match the local execution state, with mayhemensuing. The result often is to trigger consistency checks in theexecutor, but I believe this could end in a crash or incorrect updates.To fix, assign new Param IDs to each of the cloned SubPlans, so thatthey don't share ParamExecData slots at runtime. It still seems finefor the clones to share the underlying subplan, and extra ParamExecDataslots are cheap enough that this fix shouldn't cost much.This has been busted since we invented MULTIEXPR SubPlans in 9.5.Probably the lack of previous reports is because query plans in whichthe different clones of a MULTIEXPR mutate to effectively-differentstates are pretty rare. There's no issue in v14 and later, becausewithout inheritance_planner() there's never a reason to cloneMULTIEXPR SubPlans.Per bug #17596 from Andre Lin. Patch v10-v13 only.Discussion:https://postgr.es/m/17596-c5357f61427a81dc@postgresql.org
1 parent7d50165 commit3f7323c

File tree

6 files changed

+184
-0
lines changed

6 files changed

+184
-0
lines changed

‎src/backend/executor/nodeSubplan.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,21 @@ ExecScanSubPlan(SubPlanState *node,
245245
* ones, so this should be safe.) Unlike ExecReScanSetParamPlan, we do
246246
* *not* set bits in the parent plan node's chgParam, because we don't
247247
* want to cause a rescan of the parent.
248+
*
249+
* Note: we are also relying on MULTIEXPR SubPlans not sharing any output
250+
* parameters with other SubPlans, because if one does then it is unclear
251+
* which SubPlanState node the parameter's execPlan field will be pointing
252+
* to when we come to evaluate the parameter. We can allow plain initplan
253+
* SubPlans to share output parameters, because it doesn't actually matter
254+
* which initplan SubPlan we reference as long as they all point to the
255+
* same underlying subplan. However, that fails to hold for MULTIEXPRs
256+
* because they can have non-empty args lists, and the "same" args might
257+
* have mutated into different forms in different parts of a plan tree.
258+
* There is not a problem in ordinary queries because MULTIEXPR will
259+
* appear only in an UPDATE's top-level target list, so it won't get
260+
* duplicated anyplace. However, when inheritance_planner clones a
261+
* partially-planned targetlist it must take care to assign non-duplicate
262+
* param IDs to the cloned copy.
248263
*/
249264
if (subLinkType==MULTIEXPR_SUBLINK)
250265
{

‎src/backend/optimizer/plan/planner.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1606,6 +1606,10 @@ inheritance_planner(PlannerInfo *root)
16061606
/* and we haven't created PlaceHolderInfos, either */
16071607
Assert(subroot->placeholder_list==NIL);
16081608

1609+
/* Fix MULTIEXPR_SUBLINK params if any */
1610+
if (root->multiexpr_params)
1611+
SS_make_multiexprs_unique(root,subroot);
1612+
16091613
/* Generate Path(s) for accessing this result relation */
16101614
grouping_planner(subroot, true,0.0/* retrieve all tuples */ );
16111615

‎src/backend/optimizer/plan/subselect.c

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,101 @@ hash_ok_operator(OpExpr *expr)
851851
}
852852
}
853853

854+
/*
855+
* SS_make_multiexprs_unique
856+
*
857+
* After cloning an UPDATE targetlist that contains MULTIEXPR_SUBLINK
858+
* SubPlans, inheritance_planner() must call this to assign new, unique Param
859+
* IDs to the cloned MULTIEXPR_SUBLINKs' output parameters. See notes in
860+
* ExecScanSubPlan.
861+
*/
862+
void
863+
SS_make_multiexprs_unique(PlannerInfo*root,PlannerInfo*subroot)
864+
{
865+
List*new_multiexpr_params=NIL;
866+
intoffset;
867+
ListCell*lc;
868+
869+
/*
870+
* Find MULTIEXPR SubPlans in the cloned query. We need only look at the
871+
* top level of the targetlist.
872+
*/
873+
foreach(lc,subroot->parse->targetList)
874+
{
875+
TargetEntry*tent= (TargetEntry*)lfirst(lc);
876+
SubPlan*splan;
877+
Plan*plan;
878+
List*params;
879+
880+
if (!IsA(tent->expr,SubPlan))
881+
continue;
882+
splan= (SubPlan*)tent->expr;
883+
if (splan->subLinkType!=MULTIEXPR_SUBLINK)
884+
continue;
885+
886+
/* Found one, get the associated subplan */
887+
plan= (Plan*)list_nth(root->glob->subplans,splan->plan_id-1);
888+
889+
/*
890+
* Generate new PARAM_EXEC Param nodes, and overwrite splan->setParam
891+
* with their IDs. This is just like what build_subplan did when it
892+
* made the SubPlan node we're cloning. But because the param IDs are
893+
* assigned globally, we'll get new IDs. (We assume here that the
894+
* subroot's tlist is a clone we can scribble on.)
895+
*/
896+
params=generate_subquery_params(root,
897+
plan->targetlist,
898+
&splan->setParam);
899+
900+
/*
901+
* We will append the replacement-Params lists to
902+
* root->multiexpr_params, but for the moment just make a local list.
903+
* Since we lack easy access here to the original subLinkId, we have
904+
* to fall back on the slightly shaky assumption that the MULTIEXPR
905+
* SubPlans appear in the targetlist in subLinkId order. This should
906+
* be safe enough given the way that the parser builds the targetlist
907+
* today. I wouldn't want to rely on it going forward, but since this
908+
* code has a limited lifespan it should be fine. We can partially
909+
* protect against problems with assertions below.
910+
*/
911+
new_multiexpr_params=lappend(new_multiexpr_params,params);
912+
}
913+
914+
/*
915+
* Now we must find the Param nodes that reference the MULTIEXPR outputs
916+
* and update their sublink IDs so they'll reference the new outputs.
917+
* Fortunately, those too must be at top level of the cloned targetlist.
918+
*/
919+
offset=list_length(root->multiexpr_params);
920+
921+
foreach(lc,subroot->parse->targetList)
922+
{
923+
TargetEntry*tent= (TargetEntry*)lfirst(lc);
924+
Param*p;
925+
intsubqueryid;
926+
intcolno;
927+
928+
if (!IsA(tent->expr,Param))
929+
continue;
930+
p= (Param*)tent->expr;
931+
if (p->paramkind!=PARAM_MULTIEXPR)
932+
continue;
933+
subqueryid=p->paramid >>16;
934+
colno=p->paramid&0xFFFF;
935+
Assert(subqueryid>0&&
936+
subqueryid <=list_length(new_multiexpr_params));
937+
Assert(colno>0&&
938+
colno <=list_length((List*)list_nth(new_multiexpr_params,
939+
subqueryid-1)));
940+
subqueryid+=offset;
941+
p->paramid= (subqueryid <<16)+colno;
942+
}
943+
944+
/* Finally, attach new replacement lists to the global list */
945+
root->multiexpr_params=list_concat(root->multiexpr_params,
946+
new_multiexpr_params);
947+
}
948+
854949

855950
/*
856951
* SS_process_ctes: process a query's WITH list

‎src/include/optimizer/subselect.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include"nodes/pathnodes.h"
1717
#include"nodes/plannodes.h"
1818

19+
externvoidSS_make_multiexprs_unique(PlannerInfo*root,PlannerInfo*subroot);
1920
externvoidSS_process_ctes(PlannerInfo*root);
2021
externJoinExpr*convert_ANY_sublink_to_join(PlannerInfo*root,
2122
SubLink*sublink,

‎src/test/regress/expected/inherit.out

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,6 +1715,55 @@ reset enable_seqscan;
17151715
reset enable_indexscan;
17161716
reset enable_bitmapscan;
17171717
--
1718+
-- Check handling of MULTIEXPR SubPlans in inherited updates
1719+
--
1720+
create table inhpar(f1 int, f2 name);
1721+
insert into inhpar select generate_series(1,10);
1722+
create table inhcld() inherits(inhpar);
1723+
insert into inhcld select generate_series(11,10000);
1724+
vacuum analyze inhcld;
1725+
vacuum analyze inhpar;
1726+
explain (verbose, costs off)
1727+
update inhpar set (f1, f2) = (select p2.unique2, p2.stringu1
1728+
from int4_tbl limit 1)
1729+
from onek p2 where inhpar.f1 = p2.unique1;
1730+
QUERY PLAN
1731+
-----------------------------------------------------------------------------
1732+
Update on public.inhpar
1733+
Update on public.inhpar
1734+
Update on public.inhcld inhpar_1
1735+
-> Merge Join
1736+
Output: $4, $5, (SubPlan 1 (returns $2,$3)), inhpar.ctid, p2.ctid
1737+
Merge Cond: (p2.unique1 = inhpar.f1)
1738+
-> Index Scan using onek_unique1 on public.onek p2
1739+
Output: p2.unique2, p2.stringu1, p2.ctid, p2.unique1
1740+
-> Sort
1741+
Output: inhpar.ctid, inhpar.f1
1742+
Sort Key: inhpar.f1
1743+
-> Seq Scan on public.inhpar
1744+
Output: inhpar.ctid, inhpar.f1
1745+
SubPlan 1 (returns $2,$3)
1746+
-> Limit
1747+
Output: (p2.unique2), (p2.stringu1)
1748+
-> Seq Scan on public.int4_tbl
1749+
Output: p2.unique2, p2.stringu1
1750+
-> Hash Join
1751+
Output: $6, $7, (SubPlan 1 (returns $2,$3)), inhpar_1.ctid, p2.ctid
1752+
Hash Cond: (inhpar_1.f1 = p2.unique1)
1753+
-> Seq Scan on public.inhcld inhpar_1
1754+
Output: inhpar_1.ctid, inhpar_1.f1
1755+
-> Hash
1756+
Output: p2.unique2, p2.stringu1, p2.ctid, p2.unique1
1757+
-> Seq Scan on public.onek p2
1758+
Output: p2.unique2, p2.stringu1, p2.ctid, p2.unique1
1759+
(27 rows)
1760+
1761+
update inhpar set (f1, f2) = (select p2.unique2, p2.stringu1
1762+
from int4_tbl limit 1)
1763+
from onek p2 where inhpar.f1 = p2.unique1;
1764+
drop table inhpar cascade;
1765+
NOTICE: drop cascades to table inhcld
1766+
--
17181767
-- Check handling of a constant-null CHECK constraint
17191768
--
17201769
create table cnullparent (f1 int);

‎src/test/regress/sql/inherit.sql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,26 @@ reset enable_seqscan;
629629
reset enable_indexscan;
630630
reset enable_bitmapscan;
631631

632+
--
633+
-- Check handling of MULTIEXPR SubPlans in inherited updates
634+
--
635+
createtableinhpar(f1int, f2 name);
636+
insert into inhparselect generate_series(1,10);
637+
createtableinhcld() inherits(inhpar);
638+
insert into inhcldselect generate_series(11,10000);
639+
vacuum analyze inhcld;
640+
vacuum analyze inhpar;
641+
642+
explain (verbose, costs off)
643+
update inhparset (f1, f2)= (selectp2.unique2,p2.stringu1
644+
from int4_tbllimit1)
645+
from onek p2whereinhpar.f1=p2.unique1;
646+
update inhparset (f1, f2)= (selectp2.unique2,p2.stringu1
647+
from int4_tbllimit1)
648+
from onek p2whereinhpar.f1=p2.unique1;
649+
650+
droptable inhpar cascade;
651+
632652
--
633653
-- Check handling of a constant-null CHECK constraint
634654
--

0 commit comments

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