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

Commit07e5a21

Browse files
committed
Fix mishandling of sortgroupref labels while splitting SRF targetlists.
split_pathtarget_at_srfs() neglected to worry about sortgroupref labelsin the intermediate PathTargets it constructs. I think we'd supposedthat their labeling didn't matter, but it does at least for the case thatGroupAggregate/GatherMerge nodes appear immediately under the ProjectSetstep(s). This results in "ERROR: ORDER/GROUP BY expression not found intargetlist" during create_plan(), as reported by Rajkumar Raghuwanshi.To fix, make this logic track the sortgroupref labeling of expressions,not just their contents. This also restores the pre-v10 behavior thatseparate GROUP BY expressions will be kept distinct even if they aretextually equal().Discussion:https://postgr.es/m/CAKcux6=1_Ye9kx8YLBPmJs_xE72PPc6vNi5q2AOHowMaCWjJ2w@mail.gmail.com
1 parentbee6a68 commit07e5a21

File tree

3 files changed

+198
-17
lines changed

3 files changed

+198
-17
lines changed

‎src/backend/optimizer/util/tlist.c

Lines changed: 129 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,38 @@
2525
((IsA(node, FuncExpr) && ((FuncExpr *) (node))->funcretset) || \
2626
(IsA(node, OpExpr) && ((OpExpr *) (node))->opretset))
2727

28-
/* Workspace for split_pathtarget_walker */
28+
/*
29+
* Data structures for split_pathtarget_at_srfs(). To preserve the identity
30+
* of sortgroupref items even if they are textually equal(), what we track is
31+
* not just bare expressions but expressions plus their sortgroupref indexes.
32+
*/
33+
typedefstruct
34+
{
35+
Node*expr;/* some subexpression of a PathTarget */
36+
Indexsortgroupref;/* its sortgroupref, or 0 if none */
37+
}split_pathtarget_item;
38+
2939
typedefstruct
3040
{
41+
/* This is a List of bare expressions: */
3142
List*input_target_exprs;/* exprs available from input */
32-
List*level_srfs;/* list of lists of SRF exprs */
33-
List*level_input_vars;/* vars needed by SRFs of each level */
34-
List*level_input_srfs;/* SRFs needed by SRFs of each level */
43+
/* These are Lists of Lists of split_pathtarget_items: */
44+
List*level_srfs;/* SRF exprs to evaluate at each level */
45+
List*level_input_vars;/* input vars needed at each level */
46+
List*level_input_srfs;/* input SRFs needed at each level */
47+
/* These are Lists of split_pathtarget_items: */
3548
List*current_input_vars;/* vars needed in current subexpr */
3649
List*current_input_srfs;/* SRFs needed in current subexpr */
50+
/* Auxiliary data for current split_pathtarget_walker traversal: */
3751
intcurrent_depth;/* max SRF depth in current subexpr */
52+
Indexcurrent_sgref;/* current subexpr's sortgroupref, or 0 */
3853
}split_pathtarget_context;
3954

4055
staticboolsplit_pathtarget_walker(Node*node,
4156
split_pathtarget_context*context);
57+
staticvoidadd_sp_item_to_pathtarget(PathTarget*target,
58+
split_pathtarget_item*item);
59+
staticvoidadd_sp_items_to_pathtarget(PathTarget*target,List*items);
4260

4361

4462
/*****************************************************************************
@@ -822,6 +840,9 @@ apply_pathtarget_labeling_to_tlist(List *tlist, PathTarget *target)
822840
* already meant as a reference to a lower subexpression). So, don't expand
823841
* any tlist expressions that appear in input_target, if that's not NULL.
824842
*
843+
* It's also important that we preserve any sortgroupref annotation appearing
844+
* in the given target, especially on expressions matching input_target items.
845+
*
825846
* The outputs of this function are two parallel lists, one a list of
826847
* PathTargets and the other an integer list of bool flags indicating
827848
* whether the corresponding PathTarget contains any evaluatable SRFs.
@@ -845,6 +866,7 @@ split_pathtarget_at_srfs(PlannerInfo *root,
845866
intmax_depth;
846867
boolneed_extra_projection;
847868
List*prev_level_tlist;
869+
intlci;
848870
ListCell*lc,
849871
*lc1,
850872
*lc2,
@@ -884,10 +906,15 @@ split_pathtarget_at_srfs(PlannerInfo *root,
884906
need_extra_projection= false;
885907

886908
/* Scan each expression in the PathTarget looking for SRFs */
909+
lci=0;
887910
foreach(lc,target->exprs)
888911
{
889912
Node*node= (Node*)lfirst(lc);
890913

914+
/* Tell split_pathtarget_walker about this expr's sortgroupref */
915+
context.current_sgref=get_pathtarget_sortgroupref(target,lci);
916+
lci++;
917+
891918
/*
892919
* Find all SRFs and Vars (and Var-like nodes) in this expression, and
893920
* enter them into appropriate lists within the context struct.
@@ -981,16 +1008,14 @@ split_pathtarget_at_srfs(PlannerInfo *root,
9811008
* This target should actually evaluate any SRFs of the current
9821009
* level, and it needs to propagate forward any Vars needed by
9831010
* later levels, as well as SRFs computed earlier and needed by
984-
* later levels. We rely on add_new_columns_to_pathtarget() to
985-
* remove duplicate items. Also, for safety, make a separate copy
986-
* of each item for each PathTarget.
1011+
* later levels.
9871012
*/
988-
add_new_columns_to_pathtarget(ntarget,copyObject(level_srfs));
1013+
add_sp_items_to_pathtarget(ntarget,level_srfs);
9891014
for_each_cell(lc,lnext(lc2))
9901015
{
9911016
List*input_vars= (List*)lfirst(lc);
9921017

993-
add_new_columns_to_pathtarget(ntarget,copyObject(input_vars));
1018+
add_sp_items_to_pathtarget(ntarget,input_vars);
9941019
}
9951020
for_each_cell(lc,lnext(lc3))
9961021
{
@@ -999,10 +1024,10 @@ split_pathtarget_at_srfs(PlannerInfo *root,
9991024

10001025
foreach(lcx,input_srfs)
10011026
{
1002-
Expr*srf= (Expr*)lfirst(lcx);
1027+
split_pathtarget_item*item=lfirst(lcx);
10031028

1004-
if (list_member(prev_level_tlist,srf))
1005-
add_new_column_to_pathtarget(ntarget,copyObject(srf));
1029+
if (list_member(prev_level_tlist,item->expr))
1030+
add_sp_item_to_pathtarget(ntarget,item);
10061031
}
10071032
}
10081033
set_pathtarget_cost_width(root,ntarget);
@@ -1037,12 +1062,17 @@ split_pathtarget_walker(Node *node, split_pathtarget_context *context)
10371062
* input_target can be treated like a Var (which indeed it will be after
10381063
* setrefs.c gets done with it), even if it's actually a SRF. Record it
10391064
* as being needed for the current expression, and ignore any
1040-
* substructure.
1065+
* substructure. (Note in particular that this preserves the identity of
1066+
* any expressions that appear as sortgrouprefs in input_target.)
10411067
*/
10421068
if (list_member(context->input_target_exprs,node))
10431069
{
1070+
split_pathtarget_item*item=palloc(sizeof(split_pathtarget_item));
1071+
1072+
item->expr=node;
1073+
item->sortgroupref=context->current_sgref;
10441074
context->current_input_vars=lappend(context->current_input_vars,
1045-
node);
1075+
item);
10461076
return false;
10471077
}
10481078

@@ -1057,8 +1087,12 @@ split_pathtarget_walker(Node *node, split_pathtarget_context *context)
10571087
IsA(node,GroupingFunc)||
10581088
IsA(node,WindowFunc))
10591089
{
1090+
split_pathtarget_item*item=palloc(sizeof(split_pathtarget_item));
1091+
1092+
item->expr=node;
1093+
item->sortgroupref=context->current_sgref;
10601094
context->current_input_vars=lappend(context->current_input_vars,
1061-
node);
1095+
item);
10621096
return false;
10631097
}
10641098

@@ -1068,15 +1102,20 @@ split_pathtarget_walker(Node *node, split_pathtarget_context *context)
10681102
*/
10691103
if (IS_SRF_CALL(node))
10701104
{
1105+
split_pathtarget_item*item=palloc(sizeof(split_pathtarget_item));
10711106
List*save_input_vars=context->current_input_vars;
10721107
List*save_input_srfs=context->current_input_srfs;
10731108
intsave_current_depth=context->current_depth;
10741109
intsrf_depth;
10751110
ListCell*lc;
10761111

1112+
item->expr=node;
1113+
item->sortgroupref=context->current_sgref;
1114+
10771115
context->current_input_vars=NIL;
10781116
context->current_input_srfs=NIL;
10791117
context->current_depth=0;
1118+
context->current_sgref=0;/* subexpressions are not sortgroup items */
10801119

10811120
(void)expression_tree_walker(node,split_pathtarget_walker,
10821121
(void*)context);
@@ -1094,7 +1133,7 @@ split_pathtarget_walker(Node *node, split_pathtarget_context *context)
10941133

10951134
/* Record this SRF as needing to be evaluated at appropriate level */
10961135
lc=list_nth_cell(context->level_srfs,srf_depth);
1097-
lfirst(lc)=lappend(lfirst(lc),node);
1136+
lfirst(lc)=lappend(lfirst(lc),item);
10981137

10991138
/* Record its inputs as being needed at the same level */
11001139
lc=list_nth_cell(context->level_input_vars,srf_depth);
@@ -1108,7 +1147,7 @@ split_pathtarget_walker(Node *node, split_pathtarget_context *context)
11081147
* surrounding expression.
11091148
*/
11101149
context->current_input_vars=save_input_vars;
1111-
context->current_input_srfs=lappend(save_input_srfs,node);
1150+
context->current_input_srfs=lappend(save_input_srfs,item);
11121151
context->current_depth=Max(save_current_depth,srf_depth);
11131152

11141153
/* We're done here */
@@ -1119,6 +1158,79 @@ split_pathtarget_walker(Node *node, split_pathtarget_context *context)
11191158
* Otherwise, the node is a scalar (non-set) expression, so recurse to
11201159
* examine its inputs.
11211160
*/
1161+
context->current_sgref=0;/* subexpressions are not sortgroup items */
11221162
returnexpression_tree_walker(node,split_pathtarget_walker,
11231163
(void*)context);
11241164
}
1165+
1166+
/*
1167+
* Add a split_pathtarget_item to the PathTarget, unless a matching item is
1168+
* already present. This is like add_new_column_to_pathtarget, but allows
1169+
* for sortgrouprefs to be handled. An item having zero sortgroupref can
1170+
* be merged with one that has a sortgroupref, acquiring the latter's
1171+
* sortgroupref.
1172+
*
1173+
* Note that we don't worry about possibly adding duplicate sortgrouprefs
1174+
* to the PathTarget. That would be bad, but it should be impossible unless
1175+
* the target passed to split_pathtarget_at_srfs already had duplicates.
1176+
* As long as it didn't, we can have at most one split_pathtarget_item with
1177+
* any particular nonzero sortgroupref.
1178+
*/
1179+
staticvoid
1180+
add_sp_item_to_pathtarget(PathTarget*target,split_pathtarget_item*item)
1181+
{
1182+
intlci;
1183+
ListCell*lc;
1184+
1185+
/*
1186+
* Look for a pre-existing entry that is equal() and does not have a
1187+
* conflicting sortgroupref already.
1188+
*/
1189+
lci=0;
1190+
foreach(lc,target->exprs)
1191+
{
1192+
Node*node= (Node*)lfirst(lc);
1193+
Indexsgref=get_pathtarget_sortgroupref(target,lci);
1194+
1195+
if ((item->sortgroupref==sgref||
1196+
item->sortgroupref==0||
1197+
sgref==0)&&
1198+
equal(item->expr,node))
1199+
{
1200+
/* Found a match. Assign item's sortgroupref if it has one. */
1201+
if (item->sortgroupref)
1202+
{
1203+
if (target->sortgrouprefs==NULL)
1204+
{
1205+
target->sortgrouprefs= (Index*)
1206+
palloc0(list_length(target->exprs)*sizeof(Index));
1207+
}
1208+
target->sortgrouprefs[lci]=item->sortgroupref;
1209+
}
1210+
return;
1211+
}
1212+
lci++;
1213+
}
1214+
1215+
/*
1216+
* No match, so add item to PathTarget. Copy the expr for safety.
1217+
*/
1218+
add_column_to_pathtarget(target, (Expr*)copyObject(item->expr),
1219+
item->sortgroupref);
1220+
}
1221+
1222+
/*
1223+
* Apply add_sp_item_to_pathtarget to each element of list.
1224+
*/
1225+
staticvoid
1226+
add_sp_items_to_pathtarget(PathTarget*target,List*items)
1227+
{
1228+
ListCell*lc;
1229+
1230+
foreach(lc,items)
1231+
{
1232+
split_pathtarget_item*item=lfirst(lc);
1233+
1234+
add_sp_item_to_pathtarget(target,item);
1235+
}
1236+
}

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,68 @@ explain (costs off, verbose)
659659
(11 rows)
660660

661661
drop function sp_simple_func(integer);
662+
-- test handling of SRFs in targetlist (bug in 10.0)
663+
explain (costs off)
664+
select count(*), generate_series(1,2) from tenk1 group by twenty;
665+
QUERY PLAN
666+
----------------------------------------------------------
667+
ProjectSet
668+
-> Finalize GroupAggregate
669+
Group Key: twenty
670+
-> Gather Merge
671+
Workers Planned: 4
672+
-> Partial GroupAggregate
673+
Group Key: twenty
674+
-> Sort
675+
Sort Key: twenty
676+
-> Parallel Seq Scan on tenk1
677+
(10 rows)
678+
679+
select count(*), generate_series(1,2) from tenk1 group by twenty;
680+
count | generate_series
681+
-------+-----------------
682+
500 | 1
683+
500 | 2
684+
500 | 1
685+
500 | 2
686+
500 | 1
687+
500 | 2
688+
500 | 1
689+
500 | 2
690+
500 | 1
691+
500 | 2
692+
500 | 1
693+
500 | 2
694+
500 | 1
695+
500 | 2
696+
500 | 1
697+
500 | 2
698+
500 | 1
699+
500 | 2
700+
500 | 1
701+
500 | 2
702+
500 | 1
703+
500 | 2
704+
500 | 1
705+
500 | 2
706+
500 | 1
707+
500 | 2
708+
500 | 1
709+
500 | 2
710+
500 | 1
711+
500 | 2
712+
500 | 1
713+
500 | 2
714+
500 | 1
715+
500 | 2
716+
500 | 1
717+
500 | 2
718+
500 | 1
719+
500 | 2
720+
500 | 1
721+
500 | 2
722+
(40 rows)
723+
662724
-- test gather merge with parallel leader participation disabled
663725
set parallel_leader_participation = off;
664726
explain (costs off)

‎src/test/regress/sql/select_parallel.sql

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,13 @@ explain (costs off, verbose)
261261

262262
dropfunction sp_simple_func(integer);
263263

264+
-- test handling of SRFs in targetlist (bug in 10.0)
265+
266+
explain (costs off)
267+
selectcount(*), generate_series(1,2)from tenk1group by twenty;
268+
269+
selectcount(*), generate_series(1,2)from tenk1group by twenty;
270+
264271
-- test gather merge with parallel leader participation disabled
265272
set parallel_leader_participation= off;
266273

0 commit comments

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