Skip to content

Fix constant GROUP BY on empty input #2045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
krylosov-aa wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix constant GROUP BY on empty input #2045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb15a55
Fix constant GROUP BY on empty input
krylosov-aa Sep 29, 2026
6f60be0
Cover parallel and TupleSplit constant GROUP BY
krylosov-aa Sep 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion contrib/pax_storage/src/test/regress/expected/bfv_aggregate.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1777,7 +1777,7 @@ explain (costs off)
select 1, sum(col1) from group_by_const group by 1;
QUERY PLAN
------------------------------------------------
Finalize Aggregate
Finalize GroupAggregate
-> Gather Motion 3:1 (slice1; segments: 3)
-> Partial GroupAggregate
-> Seq Scan on group_by_const
Expand Down
74 changes: 61 additions & 13 deletions src/backend/cdb/cdbgroupingpaths.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,37 @@ cdb_create_multistage_grouping_paths(PlannerInfo *root,
break;
case MULTI_DQAS:
{
ListCell *lc;

/*
* If all aggregate FILTER conditions are false, TupleSplit
* returns no rows even though the input is nonempty. A
* constant GROUP BY must still return one group in this
* case, but the GroupAggregate nodes in this plan would
* return none.
*
* Do not build this plan when every DQA has a FILTER. An
* unfiltered DQA ensures that TupleSplit produces rows for
* nonempty input.
*/
if (ctx.parseGroupClause && !ctx.groupClause)
{
bool has_unfiltered_agg = false;

foreach(lc, agg_costs->distinctAggrefs)
{
Aggref *aggref = lfirst_node(Aggref, lc);

if (!aggref->aggfilter)
{
has_unfiltered_agg = true;
break;
}
}
if (!has_unfiltered_agg)
break;
}

fetch_multi_dqas_info(root, cheapest_path, &ctx, &info);
/*
* GPDB_14_MERGE_FIXME: We have done some copy job in
Expand All @@ -530,7 +561,6 @@ cdb_create_multistage_grouping_paths(PlannerInfo *root,
* removing the origin plan's aggfilter can work around
* this problem. We'll look at it again later.
*/
ListCell *lc;
foreach(lc, root->agginfos)
{
AggInfo *agginfo = (AggInfo *) lfirst(lc);
Expand Down Expand Up @@ -1102,7 +1132,7 @@ add_first_stage_group_agg_path(PlannerInfo *root,
ctx->agg_partial_costs);
add_path(ctx->partial_rel, first_stage_agg_path, root);
}
else if (ctx->hasAggs || ctx->groupClause || ctx->hasDistinctOn)
else if (ctx->hasAggs || ctx->parseGroupClause || ctx->hasDistinctOn)
{
add_path(ctx->partial_rel,
(Path *) create_agg_path(root,
Expand Down Expand Up @@ -1141,10 +1171,19 @@ add_second_stage_group_agg_path(PlannerInfo *root,
CdbPathLocus singleQE_locus;
CdbPathLocus group_locus;
bool need_redistribute;
AggStrategy aggstrategy;

/* The input should be distributed, otherwise no point in a two-stage Agg. */
Assert(CdbPathLocus_IsPartitioned(initial_agg_path->locus));

/*
* GROUP BY must return no rows for empty input, even if all grouping
* keys were removed as redundant. Use AGG_SORTED to preserve this
* behavior; AGG_PLAIN would produce one row.
*/
aggstrategy = (ctx->parseGroupClause != NIL ||
ctx->final_groupClause != NIL) ? AGG_SORTED : AGG_PLAIN;
Comment thread
krylosov-aa marked this conversation as resolved.

group_locus = choose_grouping_locus(root,
initial_agg_path,
ctx->final_group_tles,
Expand Down Expand Up @@ -1189,7 +1228,7 @@ add_second_stage_group_agg_path(PlannerInfo *root,
output_rel,
path,
ctx->target,
(ctx->final_groupClause ? AGG_SORTED : AGG_PLAIN),
aggstrategy,
ctx->hasAggs ? AGGSPLIT_FINAL_DESERIAL : AGGSPLIT_SIMPLE,
false, /* streaming */
ctx->final_groupClause,
Expand Down Expand Up @@ -1227,7 +1266,7 @@ add_second_stage_group_agg_path(PlannerInfo *root,
output_rel,
path,
ctx->target,
(ctx->final_groupClause ? AGG_SORTED : AGG_PLAIN),
aggstrategy,
ctx->hasAggs ? AGGSPLIT_FINAL_DESERIAL : AGGSPLIT_SIMPLE,
false, /* streaming */
ctx->final_groupClause,
Expand Down Expand Up @@ -1440,13 +1479,16 @@ static void add_single_mixed_dqa_hash_agg_path(PlannerInfo *root,
CdbPathLocus distinct_locus;
bool distinct_need_redistribute;
CdbPathLocus singleQE_locus;
AggStrategy aggstrategy;

if (!gp_enable_agg_distinct)
return;

if (ctx->groupClause)
return;

aggstrategy = ctx->parseGroupClause ? AGG_SORTED : AGG_PLAIN;

/*
* If subpath is projection capable, we do not want to generate a
* projection plan. The reason is that the projection plan does not
Expand All @@ -1471,7 +1513,7 @@ static void add_single_mixed_dqa_hash_agg_path(PlannerInfo *root,
output_rel,
path,
ctx->partial_grouping_target,
AGG_PLAIN,
aggstrategy,
AGGSPLIT_INITIAL_SERIAL,
false, /* streaming */
ctx->groupClause,
Expand All @@ -1487,7 +1529,7 @@ static void add_single_mixed_dqa_hash_agg_path(PlannerInfo *root,
output_rel,
path,
ctx->target,
AGG_PLAIN,
aggstrategy,
AGGSPLIT_FINAL_DESERIAL,
false, /* streaming */
ctx->groupClause,
Expand Down Expand Up @@ -1517,10 +1559,16 @@ add_single_dqa_hash_agg_path(PlannerInfo *root,
bool group_need_redistribute;
CdbPathLocus distinct_locus;
bool distinct_need_redistribute;
AggStrategy aggstrategy;

if (!gp_enable_agg_distinct)
return;

if (ctx->groupClause)
aggstrategy = AGG_HASHED;
else
aggstrategy = ctx->parseGroupClause ? AGG_SORTED : AGG_PLAIN;

/*
* If subpath is projection capable, we do not want to generate a
* projection plan. The reason is that the projection plan does not
Expand Down Expand Up @@ -1598,7 +1646,7 @@ add_single_dqa_hash_agg_path(PlannerInfo *root,
output_rel,
path,
ctx->target,
ctx->groupClause ? AGG_HASHED : AGG_PLAIN,
aggstrategy,
AGGSPLIT_DEDUPLICATED,
false, /* streaming */
ctx->groupClause,
Expand Down Expand Up @@ -1630,7 +1678,7 @@ add_single_dqa_hash_agg_path(PlannerInfo *root,
output_rel,
path,
strip_aggdistinct(ctx->partial_grouping_target),
ctx->groupClause ? AGG_HASHED : AGG_PLAIN,
aggstrategy,
AGGSPLIT_INITIAL_SERIAL | AGGSPLITOP_DEDUPLICATED,
false, /* streaming */
ctx->groupClause,
Expand All @@ -1645,7 +1693,7 @@ add_single_dqa_hash_agg_path(PlannerInfo *root,
output_rel,
path,
ctx->target,
ctx->groupClause ? AGG_HASHED : AGG_PLAIN,
aggstrategy,
AGGSPLIT_FINAL_DESERIAL | AGGSPLITOP_DEDUPLICATED,
false, /* streaming */
ctx->groupClause,
Expand Down Expand Up @@ -1714,7 +1762,7 @@ add_single_dqa_hash_agg_path(PlannerInfo *root,
output_rel,
path,
ctx->target,
ctx->groupClause ? AGG_HASHED : AGG_PLAIN,
aggstrategy,
AGGSPLIT_DEDUPLICATED,
false, /* streaming */
ctx->groupClause,
Expand Down Expand Up @@ -1767,7 +1815,7 @@ add_single_dqa_hash_agg_path(PlannerInfo *root,
output_rel,
path,
strip_aggdistinct(ctx->partial_grouping_target),
ctx->groupClause ? AGG_HASHED : AGG_PLAIN,
aggstrategy,
AGGSPLIT_INITIAL_SERIAL | AGGSPLITOP_DEDUPLICATED,
false, /* streaming */
ctx->groupClause,
Expand All @@ -1781,7 +1829,7 @@ add_single_dqa_hash_agg_path(PlannerInfo *root,
output_rel,
path,
ctx->target,
ctx->groupClause ? AGG_HASHED : AGG_PLAIN,
aggstrategy,
AGGSPLIT_FINAL_DESERIAL | AGGSPLITOP_DEDUPLICATED,
false, /* streaming */
ctx->groupClause,
Expand Down Expand Up @@ -1899,7 +1947,7 @@ add_multi_dqas_hash_agg_path(PlannerInfo *root,
path = cdbpath_create_motion_path(root, path, NIL, false,
distinct_locus);

AggStrategy split = AGG_PLAIN;
AggStrategy split = ctx->parseGroupClause ? AGG_SORTED : AGG_PLAIN;
unsigned long DEDUPLICATED_FLAG = 0;
PathTarget *partial_target = info->partial_target;
double input_rows = path->rows;
Expand Down
2 changes: 1 addition & 1 deletion src/test/regress/expected/bfv_aggregate.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1777,7 +1777,7 @@ explain (costs off)
select 1, sum(col1) from group_by_const group by 1;
QUERY PLAN
------------------------------------------------
Finalize Aggregate
Finalize GroupAggregate
-> Gather Motion 3:1 (slice1; segments: 3)
-> Partial GroupAggregate
-> Seq Scan on group_by_const
Expand Down
Loading
Loading