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

Commitccbb54c

Browse files
committed
Further fixes for MULTIEXPR_SUBLINK fix.
Some more things I didn't think about in commits3f7323c et al:MULTIEXPR_SUBLINK subplans might have been converted to initplansinstead of regular subplans, in which case they won't show up inthe modified targetlist. Fortunately, this would only happen ifthey have no input parameters, which means that the problem weoriginally needed to fix can't happen with them. Therefore, there'sno need to clone their output parameters, and thus it doesn't hurtthat we'll fail to see them in the first pass over the targetlist.Nonetheless, this complicates matters greatly, because now we haveto distinguish output Params of initplans (which shouldn't getrenumbered) from those of regular subplans (which should).This also breaks the simplistic scheme I used of assuming that thesubplans found in the targetlist have consecutive subLinkIds.We really can't avoid the need to know the subplans' subLinkIds inthis code. To fix that, add subLinkId as the last field of SubPlan.We can get away with that change in back branches because SubPlannodes will never be stored in the catalogs, and there's no ABIbreak for external code that might be looking at the existingfields of SubPlan.Secondly, rewriteTargetListIU might have rolled up multipleFieldStores or SubscriptingRefs into one targetlist entry,breaking the assumption that there's at most one Param to fixper targetlist entry. (That assumption is OK I think in theruleutils.c code I stole the logic from in18f5108, becausethat only deals with pre-rewrite query trees. But it'sdefinitely not OK here.) Abandon that shortcut and just do afull tree walk on the targetlist to ensure we find all theParams we have to change.Per bug #17606 from Andre Lin. As before, only v10-v13 need thepatch.Discussion:https://postgr.es/m/17606-e5c8ad18d31db96a@postgresql.org
1 parent43e409c commitccbb54c

File tree

8 files changed

+119
-83
lines changed

8 files changed

+119
-83
lines changed

‎src/backend/nodes/copyfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1719,6 +1719,7 @@ _copySubPlan(const SubPlan *from)
17191719
COPY_NODE_FIELD(args);
17201720
COPY_SCALAR_FIELD(startup_cost);
17211721
COPY_SCALAR_FIELD(per_call_cost);
1722+
COPY_SCALAR_FIELD(subLinkId);
17221723

17231724
returnnewnode;
17241725
}

‎src/backend/nodes/equalfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,7 @@ _equalSubPlan(const SubPlan *a, const SubPlan *b)
455455
COMPARE_NODE_FIELD(args);
456456
COMPARE_SCALAR_FIELD(startup_cost);
457457
COMPARE_SCALAR_FIELD(per_call_cost);
458+
COMPARE_SCALAR_FIELD(subLinkId);
458459

459460
return true;
460461
}

‎src/backend/nodes/outfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1349,6 +1349,7 @@ _outSubPlan(StringInfo str, const SubPlan *node)
13491349
WRITE_NODE_FIELD(args);
13501350
WRITE_FLOAT_FIELD(startup_cost,"%.2f");
13511351
WRITE_FLOAT_FIELD(per_call_cost,"%.2f");
1352+
WRITE_INT_FIELD(subLinkId);
13521353
}
13531354

13541355
staticvoid

‎src/backend/nodes/readfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2536,6 +2536,7 @@ _readSubPlan(void)
25362536
READ_NODE_FIELD(args);
25372537
READ_FLOAT_FIELD(startup_cost);
25382538
READ_FLOAT_FIELD(per_call_cost);
2539+
READ_INT_FIELD(subLinkId);
25392540

25402541
READ_DONE();
25412542
}

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

Lines changed: 71 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ static bool subplan_is_hashable(Plan *plan);
8686
staticbooltestexpr_is_hashable(Node*testexpr,List*param_ids);
8787
staticbooltest_opexpr_is_hashable(OpExpr*testexpr,List*param_ids);
8888
staticboolhash_ok_operator(OpExpr*expr);
89+
staticboolSS_make_multiexprs_unique_walker(Node*node,void*context);
8990
staticboolcontain_dml(Node*node);
9091
staticboolcontain_dml_walker(Node*node,void*context);
9192
staticboolcontain_outer_selfref(Node*node);
@@ -335,6 +336,7 @@ build_subplan(PlannerInfo *root, Plan *plan, PlannerInfo *subroot,
335336
*/
336337
splan=makeNode(SubPlan);
337338
splan->subLinkType=subLinkType;
339+
splan->subLinkId=subLinkId;
338340
splan->testexpr=NULL;
339341
splan->paramIds=NIL;
340342
get_first_col_type(plan,&splan->firstColType,&splan->firstColTypmod,
@@ -858,12 +860,17 @@ hash_ok_operator(OpExpr *expr)
858860
* SubPlans, inheritance_planner() must call this to assign new, unique Param
859861
* IDs to the cloned MULTIEXPR_SUBLINKs' output parameters. See notes in
860862
* ExecScanSubPlan.
863+
*
864+
* We do not need to renumber Param IDs for MULTIEXPR_SUBLINK plans that are
865+
* initplans, because those don't have input parameters that could cause
866+
* confusion. Such initplans will not appear in the targetlist anyway, but
867+
* they still complicate matters because the surviving regular subplans might
868+
* not have consecutive subLinkIds.
861869
*/
862870
void
863871
SS_make_multiexprs_unique(PlannerInfo*root,PlannerInfo*subroot)
864872
{
865-
List*new_multiexpr_params=NIL;
866-
intoffset;
873+
List*param_mapping=NIL;
867874
ListCell*lc;
868875

869876
/*
@@ -876,6 +883,9 @@ SS_make_multiexprs_unique(PlannerInfo *root, PlannerInfo *subroot)
876883
SubPlan*splan;
877884
Plan*plan;
878885
List*params;
886+
intoldId,
887+
newId;
888+
ListCell*lc2;
879889

880890
if (!IsA(tent->expr,SubPlan))
881891
continue;
@@ -898,86 +908,77 @@ SS_make_multiexprs_unique(PlannerInfo *root, PlannerInfo *subroot)
898908
&splan->setParam);
899909

900910
/*
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.
911+
* Append the new replacement-Params list to root->multiexpr_params.
912+
* Then its index in that list becomes the new subLinkId of the
913+
* SubPlan.
910914
*/
911-
new_multiexpr_params=lappend(new_multiexpr_params,params);
915+
root->multiexpr_params=lappend(root->multiexpr_params,params);
916+
oldId=splan->subLinkId;
917+
newId=list_length(root->multiexpr_params);
918+
Assert(newId>oldId);
919+
splan->subLinkId=newId;
920+
921+
/*
922+
* Add a mapping entry to param_mapping so that we can update the
923+
* associated Params below. Leave zeroes in the list for any
924+
* subLinkIds we don't encounter; those must have been converted to
925+
* initplans.
926+
*/
927+
while (list_length(param_mapping)<oldId)
928+
param_mapping=lappend_int(param_mapping,0);
929+
lc2=list_nth_cell(param_mapping,oldId-1);
930+
lfirst_int(lc2)=newId;
912931
}
913932

914933
/*
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 in the cloned targetlist, but they could
918-
* be buried under FieldStores and SubscriptingRefs and CoerceToDomains
919-
* (cf processIndirection()), and underneath those there could be an
920-
* implicit type coercion.
934+
* Unless all the MULTIEXPRs were converted to initplans, we must now find
935+
* the Param nodes that reference the MULTIEXPR outputs and update their
936+
* sublink IDs so they'll reference the new outputs. While such Params
937+
* must be in the cloned targetlist, they could be buried under stuff such
938+
* as FieldStores and SubscriptingRefs and type coercions.
921939
*/
922-
offset=list_length(root->multiexpr_params);
940+
if (param_mapping!=NIL)
941+
SS_make_multiexprs_unique_walker((Node*)subroot->parse->targetList,
942+
(void*)param_mapping);
943+
}
923944

924-
foreach(lc,subroot->parse->targetList)
945+
/*
946+
* Locate PARAM_MULTIEXPR Params in an expression tree, and update as needed.
947+
* (We can update-in-place because the tree was already copied.)
948+
*/
949+
staticbool
950+
SS_make_multiexprs_unique_walker(Node*node,void*context)
951+
{
952+
if (node==NULL)
953+
return false;
954+
if (IsA(node,Param))
925955
{
926-
TargetEntry*tent= (TargetEntry*)lfirst(lc);
927-
Node*expr;
928-
Param*p;
956+
Param*p= (Param*)node;
957+
List*param_mapping= (List*)context;
929958
intsubqueryid;
930959
intcolno;
960+
intnewId;
931961

932-
expr= (Node*)tent->expr;
933-
while (expr)
934-
{
935-
if (IsA(expr,FieldStore))
936-
{
937-
FieldStore*fstore= (FieldStore*)expr;
938-
939-
expr= (Node*)linitial(fstore->newvals);
940-
}
941-
elseif (IsA(expr,SubscriptingRef))
942-
{
943-
SubscriptingRef*sbsref= (SubscriptingRef*)expr;
944-
945-
if (sbsref->refassgnexpr==NULL)
946-
break;
947-
948-
expr= (Node*)sbsref->refassgnexpr;
949-
}
950-
elseif (IsA(expr,CoerceToDomain))
951-
{
952-
CoerceToDomain*cdomain= (CoerceToDomain*)expr;
953-
954-
if (cdomain->coercionformat!=COERCE_IMPLICIT_CAST)
955-
break;
956-
expr= (Node*)cdomain->arg;
957-
}
958-
else
959-
break;
960-
}
961-
expr=strip_implicit_coercions(expr);
962-
if (expr==NULL|| !IsA(expr,Param))
963-
continue;
964-
p= (Param*)expr;
965962
if (p->paramkind!=PARAM_MULTIEXPR)
966-
continue;
963+
return false;
967964
subqueryid=p->paramid >>16;
968965
colno=p->paramid&0xFFFF;
969-
Assert(subqueryid>0&&
970-
subqueryid <=list_length(new_multiexpr_params));
971-
Assert(colno>0&&
972-
colno <=list_length((List*)list_nth(new_multiexpr_params,
973-
subqueryid-1)));
974-
subqueryid+=offset;
975-
p->paramid= (subqueryid <<16)+colno;
976-
}
977966

978-
/* Finally, attach new replacement lists to the global list */
979-
root->multiexpr_params=list_concat(root->multiexpr_params,
980-
new_multiexpr_params);
967+
/*
968+
* If subqueryid doesn't have a mapping entry, it must refer to an
969+
* initplan, so don't change the Param.
970+
*/
971+
Assert(subqueryid>0);
972+
if (subqueryid>list_length(param_mapping))
973+
return false;
974+
newId=list_nth_int(param_mapping,subqueryid-1);
975+
if (newId==0)
976+
return false;
977+
p->paramid= (newId <<16)+colno;
978+
return false;
979+
}
980+
returnexpression_tree_walker(node,SS_make_multiexprs_unique_walker,
981+
context);
981982
}
982983

983984

@@ -1110,6 +1111,7 @@ SS_process_ctes(PlannerInfo *root)
11101111
*/
11111112
splan=makeNode(SubPlan);
11121113
splan->subLinkType=CTE_SUBLINK;
1114+
splan->subLinkId=0;
11131115
splan->testexpr=NULL;
11141116
splan->paramIds=NIL;
11151117
get_first_col_type(plan,&splan->firstColType,&splan->firstColTypmod,
@@ -3075,6 +3077,7 @@ SS_make_initplan_from_plan(PlannerInfo *root,
30753077
*/
30763078
node=makeNode(SubPlan);
30773079
node->subLinkType=EXPR_SUBLINK;
3080+
node->subLinkId=0;
30783081
node->plan_id=list_length(root->glob->subplans);
30793082
node->plan_name=psprintf("InitPlan %d (returns $%d)",
30803083
node->plan_id,prm->paramid);

‎src/include/nodes/primnodes.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,8 @@ typedef struct SubPlan
731731
/* Estimated execution costs: */
732732
Coststartup_cost;/* one-time setup cost */
733733
Costper_call_cost;/* cost for each subplan evaluation */
734+
/* Copied from original SubLink, but placed at end for ABI stability */
735+
intsubLinkId;/* ID (1..n); 0 if not MULTIEXPR */
734736
}SubPlan;
735737

736738
/*

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

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1717,23 +1717,36 @@ reset enable_bitmapscan;
17171717
--
17181718
-- Check handling of MULTIEXPR SubPlans in inherited updates
17191719
--
1720-
create table inhpar(f1 int, f2 text[]);
1720+
create table inhpar(f1 int, f2 text[], f3 int);
17211721
insert into inhpar select generate_series(1,10);
17221722
create table inhcld() inherits(inhpar);
17231723
insert into inhcld select generate_series(11,10000);
17241724
vacuum analyze inhcld;
17251725
vacuum analyze inhpar;
17261726
explain (verbose, costs off)
1727-
update inhpar set (f1, f2[1]) = (select p2.unique2, p2.stringu1
1728-
from int4_tbl limit 1)
1727+
update inhpar set
1728+
(f1, f2[1]) = (select p2.unique2, p2.stringu1 from int4_tbl limit 1),
1729+
(f2[2], f2[3]) = (select 'x', 'y' from int4_tbl limit 1),
1730+
(f3, f2[4]) = (select p2.unique2, p2.stringu1 from int4_tbl limit 1),
1731+
(f2[5], f2[6]) = (select 'x', 'y' from int4_tbl limit 1)
17291732
from onek p2 where inhpar.f1 = p2.unique1;
1730-
QUERY PLAN
1731-
-----------------------------------------------------------------------------------------------
1733+
QUERY PLAN
1734+
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
17321735
Update on public.inhpar
17331736
Update on public.inhpar
17341737
Update on public.inhcld inhpar_1
1738+
InitPlan 2 (returns $4,$5)
1739+
-> Limit
1740+
Output: 'x'::text, 'y'::text
1741+
-> Seq Scan on public.int4_tbl int4_tbl_1
1742+
Output: 'x'::text, 'y'::text
1743+
InitPlan 4 (returns $10,$11)
1744+
-> Limit
1745+
Output: 'x'::text, 'y'::text
1746+
-> Seq Scan on public.int4_tbl int4_tbl_3
1747+
Output: 'x'::text, 'y'::text
17351748
-> Merge Join
1736-
Output: $4,inhpar.f2[1] := $5,(SubPlan 1 (returns $2,$3)), inhpar.ctid, p2.ctid
1749+
Output: $12, (((((inhpar.f2[1] := $13)[2] := $4)[3] := $5)[4] := $15)[5] := $10)[6] := $11, $14,(SubPlan 1 (returns $2,$3)), NULL::record, (SubPlan 3 (returns $8,$9)), NULL::record, inhpar.ctid, p2.ctid
17371750
Merge Cond: (p2.unique1 = inhpar.f1)
17381751
-> Index Scan using onek_unique1 on public.onek p2
17391752
Output: p2.unique2, p2.stringu1, p2.ctid, p2.unique1
@@ -1747,19 +1760,27 @@ from onek p2 where inhpar.f1 = p2.unique1;
17471760
Output: (p2.unique2), (p2.stringu1)
17481761
-> Seq Scan on public.int4_tbl
17491762
Output: p2.unique2, p2.stringu1
1763+
SubPlan 3 (returns $8,$9)
1764+
-> Limit
1765+
Output: (p2.unique2), (p2.stringu1)
1766+
-> Seq Scan on public.int4_tbl int4_tbl_2
1767+
Output: p2.unique2, p2.stringu1
17501768
-> Hash Join
1751-
Output: $6,inhpar_1.f2[1] := $7,(SubPlan 1 (returns $2,$3)), inhpar_1.ctid, p2.ctid
1769+
Output: $16, (((((inhpar_1.f2[1] := $17)[2] := $4)[3] := $5)[4] := $19)[5] := $10)[6] := $11, $18,(SubPlan 1 (returns $2,$3)), NULL::record, (SubPlan 3 (returns $8,$9)), NULL::record, inhpar_1.ctid, p2.ctid
17521770
Hash Cond: (inhpar_1.f1 = p2.unique1)
17531771
-> Seq Scan on public.inhcld inhpar_1
17541772
Output: inhpar_1.f2, inhpar_1.ctid, inhpar_1.f1
17551773
-> Hash
17561774
Output: p2.unique2, p2.stringu1, p2.ctid, p2.unique1
17571775
-> Seq Scan on public.onek p2
17581776
Output: p2.unique2, p2.stringu1, p2.ctid, p2.unique1
1759-
(27 rows)
1777+
(42 rows)
17601778

1761-
update inhpar set (f1, f2[1]) = (select p2.unique2, p2.stringu1
1762-
from int4_tbl limit 1)
1779+
update inhpar set
1780+
(f1, f2[1]) = (select p2.unique2, p2.stringu1 from int4_tbl limit 1),
1781+
(f2[2], f2[3]) = (select 'x', 'y' from int4_tbl limit 1),
1782+
(f3, f2[4]) = (select p2.unique2, p2.stringu1 from int4_tbl limit 1),
1783+
(f2[5], f2[6]) = (select 'x', 'y' from int4_tbl limit 1)
17631784
from onek p2 where inhpar.f1 = p2.unique1;
17641785
drop table inhpar cascade;
17651786
NOTICE: drop cascades to table inhcld

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -632,19 +632,25 @@ reset enable_bitmapscan;
632632
--
633633
-- Check handling of MULTIEXPR SubPlans in inherited updates
634634
--
635-
createtableinhpar(f1int, f2text[]);
635+
createtableinhpar(f1int, f2text[], f3int);
636636
insert into inhparselect generate_series(1,10);
637637
createtableinhcld() inherits(inhpar);
638638
insert into inhcldselect generate_series(11,10000);
639639
vacuum analyze inhcld;
640640
vacuum analyze inhpar;
641641

642642
explain (verbose, costs off)
643-
update inhparset (f1, f2[1])= (selectp2.unique2,p2.stringu1
644-
from int4_tbllimit1)
643+
update inhparset
644+
(f1, f2[1])= (selectp2.unique2,p2.stringu1from int4_tbllimit1),
645+
(f2[2], f2[3])= (select'x','y'from int4_tbllimit1),
646+
(f3, f2[4])= (selectp2.unique2,p2.stringu1from int4_tbllimit1),
647+
(f2[5], f2[6])= (select'x','y'from int4_tbllimit1)
645648
from onek p2whereinhpar.f1=p2.unique1;
646-
update inhparset (f1, f2[1])= (selectp2.unique2,p2.stringu1
647-
from int4_tbllimit1)
649+
update inhparset
650+
(f1, f2[1])= (selectp2.unique2,p2.stringu1from int4_tbllimit1),
651+
(f2[2], f2[3])= (select'x','y'from int4_tbllimit1),
652+
(f3, f2[4])= (selectp2.unique2,p2.stringu1from int4_tbllimit1),
653+
(f2[5], f2[6])= (select'x','y'from int4_tbllimit1)
648654
from onek p2whereinhpar.f1=p2.unique1;
649655

650656
droptable inhpar cascade;

0 commit comments

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