database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-11-03 09:13:20 +03:00
Code Activity

Fix failure to mark all aggregates with appropriate transtype.

Browse Source 
In commit 915b703e1 I gave get_agg_clause_costs() the responsibility of
marking Aggref nodes with the appropriate aggtranstype.  I failed to notice
that where it was being called from, it might see only a subset of the
Aggref nodes that were in the original targetlist.  Specifically, if there
are duplicate aggregate calls in the tlist, either make_sort_input_target
or make_window_input_target might put just a single instance into the
grouping_target, and then only that instance would get marked.  Fix by
moving the call back into grouping_planner(), before we start building
assorted PathTargets from the query tlist.  Per report from Stefan Huehner.

Report: <20160702131056.GD3165@huehner.biz>
This commit is contained in:
Tom Lane
2016-07-02 13:23:02 -04:00
parent b54f7a9ac9
commit 420c166163
3 changed files with 65 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
src/backend/optimizer/plan/planner.c
View File

@@ -114,6 +114,7 @@ static Size estimate_hashagg_tablesize(Path *path,
static RelOptInfo *create_grouping_paths(PlannerInfo *root,
RelOptInfo *input_rel,
PathTarget *target,
const AggClauseCosts *agg_costs,
List *rollup_lists,
List *rollup_groupclauses);
static RelOptInfo *create_window_paths(PlannerInfo *root,
@@ -1499,6 +1500,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
PathTarget *grouping_target;
PathTarget *scanjoin_target;
bool have_grouping;
AggClauseCosts agg_costs;
WindowFuncLists *wflists = NULL;
List *activeWindows = NIL;
List *rollup_lists = NIL;
@@ -1623,6 +1625,28 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
*/
root->processed_tlist = tlist;
/*
* Collect statistics about aggregates for estimating costs, and mark
* all the aggregates with resolved aggtranstypes. We must do this
* before slicing and dicing the tlist into various pathtargets, else
* some copies of the Aggref nodes might escape being marked with the
* correct transtypes.
*
* Note: currently, we do not detect duplicate aggregates here. This
* may result in somewhat-overestimated cost, which is fine for our
* purposes since all Paths will get charged the same. But at some
* point we might wish to do that detection in the planner, rather
* than during executor startup.
*/
MemSet(&agg_costs, 0, sizeof(AggClauseCosts));
if (parse->hasAggs)
{
get_agg_clause_costs(root, (Node *) tlist, AGGSPLIT_SIMPLE,
&agg_costs);
get_agg_clause_costs(root, parse->havingQual, AGGSPLIT_SIMPLE,
&agg_costs);
}
/*
* Locate any window functions in the tlist. (We don't need to look
* anywhere else, since expressions used in ORDER BY will be in there
@@ -1822,6 +1846,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
current_rel = create_grouping_paths(root,
current_rel,
grouping_target,
&agg_costs,
rollup_lists,
rollup_groupclauses);
}
@@ -3244,6 +3269,7 @@ estimate_hashagg_tablesize(Path *path, const AggClauseCosts *agg_costs,
*
* input_rel: contains the source-data Paths
* target: the pathtarget for the result Paths to compute
* agg_costs: cost info about all aggregates in query (in AGGSPLIT_SIMPLE mode)
* rollup_lists: list of grouping sets, or NIL if not doing grouping sets
* rollup_groupclauses: list of grouping clauses for grouping sets,
* or NIL if not doing grouping sets
@@ -3260,6 +3286,7 @@ static RelOptInfo *
create_grouping_paths(PlannerInfo *root,
RelOptInfo *input_rel,
PathTarget *target,
const AggClauseCosts *agg_costs,
List *rollup_lists,
List *rollup_groupclauses)
{
@@ -3267,7 +3294,6 @@ create_grouping_paths(PlannerInfo *root,
Path *cheapest_path = input_rel->cheapest_total_path;
RelOptInfo *grouped_rel;
PathTarget *partial_grouping_target = NULL;
AggClauseCosts agg_costs;
AggClauseCosts agg_partial_costs; /* parallel only */
AggClauseCosts agg_final_costs; /* parallel only */
Size hashaggtablesize;
@@ -3364,20 +3390,6 @@ create_grouping_paths(PlannerInfo *root,
return grouped_rel;
}
/*
* Collect statistics about aggregates for estimating costs. Note: we do
* not detect duplicate aggregates here; a somewhat-overestimated cost is
* okay for our purposes.
*/
MemSet(&agg_costs, 0, sizeof(AggClauseCosts));
if (parse->hasAggs)
{
get_agg_clause_costs(root, (Node *) target->exprs, AGGSPLIT_SIMPLE,
&agg_costs);
get_agg_clause_costs(root, parse->havingQual, AGGSPLIT_SIMPLE,
&agg_costs);
}
/*
* Estimate number of groups.
*/
@@ -3414,7 +3426,7 @@ create_grouping_paths(PlannerInfo *root,
*/
can_hash = (parse->groupClause != NIL &&
parse->groupingSets == NIL &&
agg_costs.numOrderedAggs == 0 &&
agg_costs->numOrderedAggs == 0 &&
grouping_is_hashable(parse->groupClause));
/*
@@ -3446,7 +3458,7 @@ create_grouping_paths(PlannerInfo *root,
/* We don't know how to do grouping sets in parallel. */
try_parallel_aggregation = false;
}
else if (agg_costs.hasNonPartial || agg_costs.hasNonSerial)
else if (agg_costs->hasNonPartial || agg_costs->hasNonSerial)
{
/* Insufficient support for partial mode. */
try_parallel_aggregation = false;
@@ -3627,7 +3639,7 @@ create_grouping_paths(PlannerInfo *root,
(List *) parse->havingQual,
rollup_lists,
rollup_groupclauses,
&agg_costs,
agg_costs,
dNumGroups));
}
else if (parse->hasAggs)
@@ -3645,7 +3657,7 @@ create_grouping_paths(PlannerInfo *root,
AGGSPLIT_SIMPLE,
parse->groupClause,
(List *) parse->havingQual,
&agg_costs,
agg_costs,
dNumGroups));
}
else if (parse->groupClause)
@@ -3727,7 +3739,7 @@ create_grouping_paths(PlannerInfo *root,
if (can_hash)
{
hashaggtablesize = estimate_hashagg_tablesize(cheapest_path,
&agg_costs,
agg_costs,
dNumGroups);
/*
@@ -3751,7 +3763,7 @@ create_grouping_paths(PlannerInfo *root,
AGGSPLIT_SIMPLE,
parse->groupClause,
(List *) parse->havingQual,
&agg_costs,
agg_costs,
dNumGroups));
}