Skip to content

Commit 5e75f25

Browse files
committed
opt: handle unsimplified groupby orderings
This commit fixes a test-only assertion failure due to the GroupBy provided ordering logic relying on ordering simplification rules. The assertion errors happened because the provided ordering logic expected that a required ordering would only ever contain grouping columns. In reality, it is possible for the ordering to include `ConstAgg` and other "pass-through" columns. However, references to these columns are usually removed by rules like `SimplifyGroupByOrdering` because they are constant within a given group. Since the failure only occurs with rules disabled and only in test builds, there is no release note. Fixes #137508 Release note: None
1 parent ddf9096 commit 5e75f25

File tree

2 files changed

+54
-4
lines changed

2 files changed

+54
-4
lines changed

pkg/sql/opt/ordering/group_by.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,13 @@ func scalarGroupByBuildChildReqOrdering(
2323

2424
func groupByCanProvideOrdering(expr memo.RelExpr, required *props.OrderingChoice) bool {
2525
// GroupBy may require a certain ordering of its input, but can also pass
26-
// through a stronger ordering on the grouping columns.
26+
// through a stronger ordering on the grouping and pass-through columns.
27+
//
28+
// Note that an ordering on non-grouping columns is not actually useful, but
29+
// we handle it anyway since ordering simplification rules can be disabled.
2730
groupBy := expr.(*memo.GroupByExpr)
28-
return required.CanProjectCols(groupBy.GroupingCols) && required.Intersects(&groupBy.Ordering)
31+
return required.CanProjectCols(groupBy.Input.Relational().OutputCols) &&
32+
required.Intersects(&groupBy.Ordering)
2933
}
3034

3135
func groupByBuildChildReqOrdering(
@@ -36,9 +40,9 @@ func groupByBuildChildReqOrdering(
3640
}
3741
groupBy := parent.(*memo.GroupByExpr)
3842
result := *required
39-
if !result.SubsetOfCols(groupBy.GroupingCols) {
43+
if !result.SubsetOfCols(groupBy.Input.Relational().OutputCols) {
4044
result = result.Copy()
41-
result.ProjectCols(groupBy.GroupingCols)
45+
result.ProjectCols(groupBy.Input.Relational().OutputCols)
4246
}
4347

4448
result = result.Intersection(&groupBy.Ordering)

pkg/sql/opt/xform/testdata/physprops/ordering

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3280,3 +3280,49 @@ top-k
32803280
├── cardinality: [0 - 3]
32813281
└── scan t106678
32823282
└── columns: a:1 b:2
3283+
3284+
# Regression test for #137508 - handle unsimplified group-by orderings.
3285+
exec-ddl
3286+
CREATE TABLE table_4 (
3287+
c_ol4_0 REGCLASS,
3288+
col4_1 BOOL NOT NULL,
3289+
col4_2 INT2 NOT NULL,
3290+
col4_3 REGCLASS NOT NULL,
3291+
col4_4 STRING NOT NULL AS (lower(CAST(col4_1 AS STRING))) STORED,
3292+
PRIMARY KEY (col4_3 DESC, col4_2 ASC)
3293+
);
3294+
----
3295+
3296+
opt disable=(SimplifyWindowOrdering,SimplifyGroupByOrdering,PruneRootCols)
3297+
SELECT concat_agg(col4_4 ORDER BY col4_4)
3298+
FROM table_4 WHERE col4_1
3299+
GROUP BY tableoid, col4_4
3300+
ORDER BY tableoid, col4_4;
3301+
----
3302+
sort
3303+
├── columns: concat_agg:8!null [hidden: col4_4:5!null tableoid:7]
3304+
├── key: (7)
3305+
├── fd: ()-->(5), (7)-->(5,8)
3306+
├── ordering: +7 opt(5) [actual: +7]
3307+
└── group-by (hash)
3308+
├── columns: col4_4:5!null tableoid:7 concat_agg:8!null
3309+
├── grouping columns: tableoid:7
3310+
├── internal-ordering: +5
3311+
├── key: (7)
3312+
├── fd: ()-->(5), (7)-->(5,8)
3313+
├── select
3314+
│ ├── columns: col4_1:2!null col4_4:5!null tableoid:7
3315+
│ ├── fd: ()-->(2,5)
3316+
│ ├── scan table_4
3317+
│ │ ├── columns: col4_1:2!null col4_4:5!null tableoid:7
3318+
│ │ ├── computed column expressions
3319+
│ │ │ └── col4_4:5
3320+
│ │ │ └── lower(col4_1:2::STRING)
3321+
│ │ └── fd: (2)-->(5)
3322+
│ └── filters
3323+
│ └── col4_1:2 [outer=(2), fd=()-->(2)]
3324+
└── aggregations
3325+
├── concat-agg [as=concat_agg:8, outer=(5)]
3326+
│ └── col4_4:5
3327+
└── const-agg [as=col4_4:5, outer=(5)]
3328+
└── col4_4:5

0 commit comments

Comments
 (0)