Skip to content

Commit b407e6a

Browse files
committed
Fix: group items in projection should wrap nullable too.
1 parent 73c771c commit b407e6a

File tree

3 files changed

+30
-10
lines changed

3 files changed

+30
-10
lines changed

src/query/sql/src/executor/physical_plan.rs

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -174,24 +174,29 @@ pub struct AggregateExpand {
174174
pub plan_id: u32,
175175

176176
pub input: Box<PhysicalPlan>,
177-
pub group_bys: Vec<usize>,
177+
pub group_bys: Vec<IndexType>,
178178
pub grouping_id_index: IndexType,
179-
pub grouping_sets: Vec<Vec<usize>>,
179+
pub grouping_sets: Vec<Vec<IndexType>>,
180180
/// Only used for explain
181181
pub stat_info: Option<PlanStatsInfo>,
182182
}
183183

184184
impl AggregateExpand {
185185
pub fn output_schema(&self) -> Result<DataSchemaRef> {
186186
let input_schema = self.input.output_schema()?;
187-
let input_fields = input_schema.fields();
188-
let mut output_fields = Vec::with_capacity(input_fields.len() + 1);
189-
for field in input_fields {
190-
output_fields.push(DataField::new(
191-
field.name(),
192-
field.data_type().wrap_nullable(),
193-
));
187+
let mut output_fields = input_schema.fields().clone();
188+
189+
for group_by in self
190+
.group_bys
191+
.iter()
192+
.filter(|&index| *index != self.grouping_id_index)
193+
{
194+
// All group by columns will wrap nullable.
195+
let i = input_schema.index_of(&group_by.to_string())?;
196+
let f = &mut output_fields[i];
197+
*f = DataField::new(f.name(), f.data_type().wrap_nullable())
194198
}
199+
195200
output_fields.push(DataField::new(
196201
&self.grouping_id_index.to_string(),
197202
DataType::Number(NumberDataType::UInt32),

src/query/sql/src/planner/semantic/grouping_check.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ impl<'a> GroupingChecker<'a> {
4949
.get(&format!("{:?}", scalar))
5050
{
5151
let column = &self.bind_context.aggregate_info.group_items[*index];
52-
let column_binding = if let ScalarExpr::BoundColumnRef(column_ref) = &column.scalar {
52+
let mut column_binding = if let ScalarExpr::BoundColumnRef(column_ref) = &column.scalar
53+
{
5354
column_ref.column.clone()
5455
} else {
5556
ColumnBinding {
@@ -61,6 +62,13 @@ impl<'a> GroupingChecker<'a> {
6162
visibility: Visibility::Visible,
6263
}
6364
};
65+
66+
if let Some(grouping_id) = &self.bind_context.aggregate_info.grouping_id_column {
67+
if grouping_id.index != column_binding.index {
68+
column_binding.data_type = Box::new(column_binding.data_type.wrap_nullable());
69+
}
70+
}
71+
6472
return Ok(BoundColumnRef {
6573
column: column_binding,
6674
}

tests/sqllogictests/suites/duckdb/sql/aggregate/group/group_by_grouping_sets.test

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
query I
2+
select (number % 2) as a from numbers(5) group by grouping sets (a) order by a;
3+
----
4+
0
5+
1
6+
7+
18
statement ok
29
drop table if exists t;
310

0 commit comments

Comments
 (0)