Skip to content

Commit ae9752e

Browse files
committed
ORCA: align CBitSet vec_size for grouping-set bitsets
CreateGroupingSetsForRollup / Cube and the GROUPING_SET_EMPTY case in GetColumnAttnosForGroupBy were constructing their accumulator/seed CBitSets via the default ctor (vec_size = 256), then Union'ing in per-grouping-set bitsets built with vec_size = num_cols. CBitSet::Union just splices in any missing CBitSetLinks wholesale, so the accumulator ended up with a link at offset 0 (vec_size 256) plus a stray link at offset num_cols (vec_size num_cols) covering the high tleSortGroupRef. CBitSet::Get then computed the offset using the destination's m_vector_size = 256 and never consulted the stray link, while CBitSetIter happily walked both -- so Get(k) disagreed with the iterator for any k >= num_cols. In CreateDXLProjectNullsForGroupingSets this caused tleSortGroupRefs >= num_cols to be misclassified as non-grouping columns and NULL'd out, even in grouping sets that included them. Visible as: select generate_series(1, a) g, a+b ab from (values (1,1),(2,2)) t(a,b) group by rollup(a, ab) order by 1,2; returning 0 rows instead of 6 -- the rollup(a, ab) branch projected the a column as NULL, so generate_series(1, NULL) produced no rows. Fix by passing num_cols when constructing the accumulator and seed bitsets so all participants in the Union share m_vector_size. Add the repro to groupingsets.sql.
1 parent f00c2b4 commit ae9752e

4 files changed

Lines changed: 43 additions & 5 deletions

File tree

src/backend/gpopt/translate/CTranslatorUtils.cpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -918,7 +918,7 @@ CTranslatorUtils::GetColumnAttnosForGroupBy(
918918
case GROUPING_SET_EMPTY:
919919
{
920920
col_attnos_arr_current = GPOS_NEW(mp) CBitSetArray(mp);
921-
CBitSet *bset = GPOS_NEW(mp) CBitSet(mp);
921+
CBitSet *bset = GPOS_NEW(mp) CBitSet(mp, num_cols);
922922
col_attnos_arr_current->Append(bset);
923923
break;
924924
}
@@ -1113,11 +1113,15 @@ CTranslatorUtils::CreateGroupingSetsForRollup(CMemoryPool *mp,
11131113
GPOS_ASSERT(grouping_set->kind == GROUPING_SET_ROLLUP);
11141114
CBitSetArray *col_attnos_arr = GPOS_NEW(mp) CBitSetArray(mp);
11151115
ListCell *lc = nullptr;
1116-
CBitSet *current_result = GPOS_NEW(mp) CBitSet(mp);
1116+
// All grouping-set bitsets must share m_vector_size with the per-grouping-set
1117+
// bitsets built by CreateAttnoSetForGroupingSet (num_cols). Mixing the default
1118+
// 256 with num_cols leaves misaligned CBitSetLink offsets after Union, causing
1119+
// Get() to disagree with the iterator.
1120+
CBitSet *current_result = GPOS_NEW(mp) CBitSet(mp, num_cols);
11171121
// Maintaining the order of grouping sets is essential because the
11181122
// UnionAll operator matches each child's distribution with the
11191123
// distribution of the first child
1120-
col_attnos_arr->Append(GPOS_NEW(mp) CBitSet(mp));
1124+
col_attnos_arr->Append(GPOS_NEW(mp) CBitSet(mp, num_cols));
11211125
ForEach(lc, grouping_set->content)
11221126
{
11231127
GroupingSet *gs_current = (GroupingSet *) lfirst(lc);
@@ -1152,8 +1156,9 @@ CTranslatorUtils::CreateGroupingSetsForCube(CMemoryPool *mp,
11521156
GPOS_ASSERT(grouping_set->kind == GROUPING_SET_CUBE);
11531157
CBitSetArray *col_attnos_arr = GPOS_NEW(mp) CBitSetArray(mp);
11541158

1155-
// add an empty set
1156-
col_attnos_arr->Append(GPOS_NEW(mp) CBitSet(mp));
1159+
// add an empty set — vec_size must match what CreateAttnoSetForGroupingSet
1160+
// produces (num_cols), otherwise Union below leaves misaligned links.
1161+
col_attnos_arr->Append(GPOS_NEW(mp) CBitSet(mp, num_cols));
11571162

11581163
ListCell *lc = nullptr;
11591164
ForEach(lc, grouping_set->content)

src/test/regress/expected/groupingsets.out

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2470,4 +2470,18 @@ group by rollup (a,b) order by a;
24702470
| | 6
24712471
(8 rows)
24722472

2473+
-- ORCA: rollup over a derived-expression group alias with a target-list SRF.
2474+
select generate_series(1, a) g, a+b ab
2475+
from (values (1,1),(2,2)) t(a,b)
2476+
group by rollup(a, ab) order by 1,2;
2477+
g | ab
2478+
---+----
2479+
1 | 2
2480+
1 | 4
2481+
1 |
2482+
1 |
2483+
2 | 4
2484+
2 |
2485+
(6 rows)
2486+
24732487
-- end

src/test/regress/expected/groupingsets_optimizer.out

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2645,4 +2645,18 @@ group by rollup (a,b) order by a;
26452645
| | 6
26462646
(8 rows)
26472647

2648+
-- ORCA: rollup over a derived-expression group alias with a target-list SRF.
2649+
select generate_series(1, a) g, a+b ab
2650+
from (values (1,1),(2,2)) t(a,b)
2651+
group by rollup(a, ab) order by 1,2;
2652+
g | ab
2653+
---+----
2654+
1 | 2
2655+
1 | 4
2656+
1 |
2657+
1 |
2658+
2 | 4
2659+
2 |
2660+
(6 rows)
2661+
26482662
-- end

src/test/regress/sql/groupingsets.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -723,4 +723,9 @@ select a, b, rank(b) within group (order by b nulls last)
723723
from (values (1,1),(1,4),(1,5),(3,1),(3,2)) v(a,b)
724724
group by rollup (a,b) order by a;
725725

726+
-- ORCA: rollup over a derived-expression group alias with a target-list SRF.
727+
select generate_series(1, a) g, a+b ab
728+
from (values (1,1),(2,2)) t(a,b)
729+
group by rollup(a, ab) order by 1,2;
730+
726731
-- end

0 commit comments

Comments
 (0)