Skip to content

Commit

Permalink
fix: Final aggregation should not bind to the input of partial aggreg…
Browse files Browse the repository at this point in the history
…ation (apache#155)

This patch adds the check of the index of bound reference. The aggregate expressions of final aggregation are not bound to the input of partial aggregation anymore but sent to native side as unbound expressions.
  • Loading branch information
viirya authored Mar 5, 2024
1 parent 852b8cf commit a131c44
Show file tree
Hide file tree
Showing 4 changed files with 1,187 additions and 1,108 deletions.
13 changes: 10 additions & 3 deletions core/src/execution/datafusion/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use datafusion_physical_expr::{
execution_props::ExecutionProps,
expressions::{
CaseExpr, CastExpr, Count, FirstValue, InListExpr, IsNullExpr, LastValue, Max, Min,
NegativeExpr, NotExpr, Sum,
NegativeExpr, NotExpr, Sum, UnKnownColumn,
},
AggregateExpr, ScalarFunctionExpr,
};
Expand Down Expand Up @@ -201,9 +201,16 @@ impl PhysicalPlanner {
}
ExprStruct::Bound(bound) => {
let idx = bound.index as usize;
let column_name = format!("col_{}", idx);
Ok(Arc::new(Column::new(&column_name, idx)))
if idx >= input_schema.fields().len() {
return Err(ExecutionError::GeneralError(format!(
"Column index {} is out of bound. Schema: {}",
idx, input_schema
)));
}
let field = input_schema.field(idx);
Ok(Arc::new(Column::new(field.name().as_str(), idx)))
}
ExprStruct::Unbound(unbound) => Ok(Arc::new(UnKnownColumn::new(unbound.name.as_str()))),
ExprStruct::IsNotNull(is_notnull) => {
let child = self.create_expr(is_notnull.child.as_ref().unwrap(), input_schema)?;
Ok(Arc::new(IsNotNullExpr::new(child)))
Expand Down
6 changes: 6 additions & 0 deletions core/src/execution/proto/expr.proto
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ message Expr {
BitwiseNot bitwiseNot = 48;
Abs abs = 49;
Subquery subquery = 50;
UnboundReference unbound = 51;
}
}

Expand Down Expand Up @@ -254,6 +255,11 @@ message BoundReference {
DataType datatype = 2;
}

message UnboundReference {
string name = 1;
DataType datatype = 2;
}

message SortOrder {
Expr child = 1;
SortDirection direction = 2;
Expand Down
Loading

0 comments on commit a131c44

Please sign in to comment.