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

Commit420c166

Browse files
committed
Fix failure to mark all aggregates with appropriate transtype.
In commit915b703 I gave get_agg_clause_costs() the responsibility ofmarking Aggref nodes with the appropriate aggtranstype. I failed to noticethat where it was being called from, it might see only a subset of theAggref nodes that were in the original targetlist. Specifically, if thereare duplicate aggregate calls in the tlist, either make_sort_input_targetor make_window_input_target might put just a single instance into thegrouping_target, and then only that instance would get marked. Fix bymoving the call back into grouping_planner(), before we start buildingassorted PathTargets from the query tlist. Per report from Stefan Huehner.Report: <20160702131056.GD3165@huehner.biz>
1 parentb54f7a9 commit420c166

File tree

3 files changed

+65
-21
lines changed

3 files changed

+65
-21
lines changed

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

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ static Size estimate_hashagg_tablesize(Path *path,
114114
staticRelOptInfo*create_grouping_paths(PlannerInfo*root,
115115
RelOptInfo*input_rel,
116116
PathTarget*target,
117+
constAggClauseCosts*agg_costs,
117118
List*rollup_lists,
118119
List*rollup_groupclauses);
119120
staticRelOptInfo*create_window_paths(PlannerInfo*root,
@@ -1499,6 +1500,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
14991500
PathTarget*grouping_target;
15001501
PathTarget*scanjoin_target;
15011502
boolhave_grouping;
1503+
AggClauseCostsagg_costs;
15021504
WindowFuncLists*wflists=NULL;
15031505
List*activeWindows=NIL;
15041506
List*rollup_lists=NIL;
@@ -1623,6 +1625,28 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
16231625
*/
16241626
root->processed_tlist=tlist;
16251627

1628+
/*
1629+
* Collect statistics about aggregates for estimating costs, and mark
1630+
* all the aggregates with resolved aggtranstypes. We must do this
1631+
* before slicing and dicing the tlist into various pathtargets, else
1632+
* some copies of the Aggref nodes might escape being marked with the
1633+
* correct transtypes.
1634+
*
1635+
* Note: currently, we do not detect duplicate aggregates here. This
1636+
* may result in somewhat-overestimated cost, which is fine for our
1637+
* purposes since all Paths will get charged the same. But at some
1638+
* point we might wish to do that detection in the planner, rather
1639+
* than during executor startup.
1640+
*/
1641+
MemSet(&agg_costs,0,sizeof(AggClauseCosts));
1642+
if (parse->hasAggs)
1643+
{
1644+
get_agg_clause_costs(root, (Node*)tlist,AGGSPLIT_SIMPLE,
1645+
&agg_costs);
1646+
get_agg_clause_costs(root,parse->havingQual,AGGSPLIT_SIMPLE,
1647+
&agg_costs);
1648+
}
1649+
16261650
/*
16271651
* Locate any window functions in the tlist. (We don't need to look
16281652
* anywhere else, since expressions used in ORDER BY will be in there
@@ -1822,6 +1846,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
18221846
current_rel=create_grouping_paths(root,
18231847
current_rel,
18241848
grouping_target,
1849+
&agg_costs,
18251850
rollup_lists,
18261851
rollup_groupclauses);
18271852
}
@@ -3244,6 +3269,7 @@ estimate_hashagg_tablesize(Path *path, const AggClauseCosts *agg_costs,
32443269
*
32453270
* input_rel: contains the source-data Paths
32463271
* target: the pathtarget for the result Paths to compute
3272+
* agg_costs: cost info about all aggregates in query (in AGGSPLIT_SIMPLE mode)
32473273
* rollup_lists: list of grouping sets, or NIL if not doing grouping sets
32483274
* rollup_groupclauses: list of grouping clauses for grouping sets,
32493275
*or NIL if not doing grouping sets
@@ -3260,14 +3286,14 @@ static RelOptInfo *
32603286
create_grouping_paths(PlannerInfo*root,
32613287
RelOptInfo*input_rel,
32623288
PathTarget*target,
3289+
constAggClauseCosts*agg_costs,
32633290
List*rollup_lists,
32643291
List*rollup_groupclauses)
32653292
{
32663293
Query*parse=root->parse;
32673294
Path*cheapest_path=input_rel->cheapest_total_path;
32683295
RelOptInfo*grouped_rel;
32693296
PathTarget*partial_grouping_target=NULL;
3270-
AggClauseCostsagg_costs;
32713297
AggClauseCostsagg_partial_costs;/* parallel only */
32723298
AggClauseCostsagg_final_costs;/* parallel only */
32733299
Sizehashaggtablesize;
@@ -3364,20 +3390,6 @@ create_grouping_paths(PlannerInfo *root,
33643390
returngrouped_rel;
33653391
}
33663392

3367-
/*
3368-
* Collect statistics about aggregates for estimating costs. Note: we do
3369-
* not detect duplicate aggregates here; a somewhat-overestimated cost is
3370-
* okay for our purposes.
3371-
*/
3372-
MemSet(&agg_costs,0,sizeof(AggClauseCosts));
3373-
if (parse->hasAggs)
3374-
{
3375-
get_agg_clause_costs(root, (Node*)target->exprs,AGGSPLIT_SIMPLE,
3376-
&agg_costs);
3377-
get_agg_clause_costs(root,parse->havingQual,AGGSPLIT_SIMPLE,
3378-
&agg_costs);
3379-
}
3380-
33813393
/*
33823394
* Estimate number of groups.
33833395
*/
@@ -3414,7 +3426,7 @@ create_grouping_paths(PlannerInfo *root,
34143426
*/
34153427
can_hash= (parse->groupClause!=NIL&&
34163428
parse->groupingSets==NIL&&
3417-
agg_costs.numOrderedAggs==0&&
3429+
agg_costs->numOrderedAggs==0&&
34183430
grouping_is_hashable(parse->groupClause));
34193431

34203432
/*
@@ -3446,7 +3458,7 @@ create_grouping_paths(PlannerInfo *root,
34463458
/* We don't know how to do grouping sets in parallel. */
34473459
try_parallel_aggregation= false;
34483460
}
3449-
elseif (agg_costs.hasNonPartial||agg_costs.hasNonSerial)
3461+
elseif (agg_costs->hasNonPartial||agg_costs->hasNonSerial)
34503462
{
34513463
/* Insufficient support for partial mode. */
34523464
try_parallel_aggregation= false;
@@ -3627,7 +3639,7 @@ create_grouping_paths(PlannerInfo *root,
36273639
(List*)parse->havingQual,
36283640
rollup_lists,
36293641
rollup_groupclauses,
3630-
&agg_costs,
3642+
agg_costs,
36313643
dNumGroups));
36323644
}
36333645
elseif (parse->hasAggs)
@@ -3645,7 +3657,7 @@ create_grouping_paths(PlannerInfo *root,
36453657
AGGSPLIT_SIMPLE,
36463658
parse->groupClause,
36473659
(List*)parse->havingQual,
3648-
&agg_costs,
3660+
agg_costs,
36493661
dNumGroups));
36503662
}
36513663
elseif (parse->groupClause)
@@ -3727,7 +3739,7 @@ create_grouping_paths(PlannerInfo *root,
37273739
if (can_hash)
37283740
{
37293741
hashaggtablesize=estimate_hashagg_tablesize(cheapest_path,
3730-
&agg_costs,
3742+
agg_costs,
37313743
dNumGroups);
37323744

37333745
/*
@@ -3751,7 +3763,7 @@ create_grouping_paths(PlannerInfo *root,
37513763
AGGSPLIT_SIMPLE,
37523764
parse->groupClause,
37533765
(List*)parse->havingQual,
3754-
&agg_costs,
3766+
agg_costs,
37553767
dNumGroups));
37563768
}
37573769

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,3 +296,27 @@ order by s2 desc;
296296
0 | 0
297297
(3 rows)
298298

299+
-- test for failure to set all aggregates' aggtranstype
300+
explain (verbose, costs off)
301+
select sum(tenthous) as s1, sum(tenthous) + random()*0 as s2
302+
from tenk1 group by thousand order by thousand limit 3;
303+
QUERY PLAN
304+
-------------------------------------------------------------------------------------------------------------------
305+
Limit
306+
Output: (sum(tenthous)), (((sum(tenthous))::double precision + (random() * '0'::double precision))), thousand
307+
-> GroupAggregate
308+
Output: sum(tenthous), ((sum(tenthous))::double precision + (random() * '0'::double precision)), thousand
309+
Group Key: tenk1.thousand
310+
-> Index Only Scan using tenk1_thous_tenthous on public.tenk1
311+
Output: thousand, tenthous
312+
(7 rows)
313+
314+
select sum(tenthous) as s1, sum(tenthous) + random()*0 as s2
315+
from tenk1 group by thousand order by thousand limit 3;
316+
s1 | s2
317+
-------+-------
318+
45000 | 45000
319+
45010 | 45010
320+
45020 | 45020
321+
(3 rows)
322+

‎src/test/regress/sql/limit.sql‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,11 @@ order by s2 desc;
9191

9292
select generate_series(0,2)as s1, generate_series((random()*.1)::int,2)as s2
9393
order by s2desc;
94+
95+
-- test for failure to set all aggregates' aggtranstype
96+
explain (verbose, costs off)
97+
selectsum(tenthous)as s1,sum(tenthous)+ random()*0as s2
98+
from tenk1group by thousandorder by thousandlimit3;
99+
100+
selectsum(tenthous)as s1,sum(tenthous)+ random()*0as s2
101+
from tenk1group by thousandorder by thousandlimit3;

0 commit comments

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