alamb commented on code in PR #3739:
URL: https://github.com/apache/arrow-datafusion/pull/3739#discussion_r989392376
##########
datafusion/optimizer/src/projection_push_down.rs:
##########
@@ -332,16 +330,6 @@ fn optimize_plan(
}
})?;
- let new_schema = DFSchema::new_with_metadata(
- schema
- .fields()
- .iter()
- .filter(|x|
new_required_columns.contains(&x.qualified_column()))
- .cloned()
- .collect(),
- schema.metadata().clone(),
- )?;
Review Comment:
yes I agree having the caller have to specify (correctly) the aggregate
schema is a recipe for disaster
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1334,10 +1337,28 @@ pub struct Aggregate {
}
impl Aggregate {
+ /// Create a new aggregate operator.
pub fn try_new(
input: Arc<LogicalPlan>,
group_expr: Vec<Expr>,
aggr_expr: Vec<Expr>,
+ ) -> datafusion_common::Result<Self> {
+ let grouping_expr: Vec<Expr> =
grouping_set_to_exprlist(group_expr.as_slice())?;
+ let all_expr = grouping_expr.iter().chain(aggr_expr.iter());
+ validate_unique_names("Aggregations", all_expr.clone())?;
+ let schema = DFSchema::new_with_metadata(
+ exprlist_to_fields(all_expr, &input)?,
+ input.schema().metadata().clone(),
+ )?;
+ Self::try_new_with_schema(input, group_expr, aggr_expr,
Arc::new(schema))
+ }
+
+ /// Create a new aggregate operator using the provided schema to avoid the
overhead of
+ /// building the schema again when the schema is already known.
+ pub fn try_new_with_schema(
Review Comment:
I recommend making this code non pub as to use it requires a lot of
knowledge about how to use it.
If we have to make it pub to call from other crates, I recommend putting a
big warning on it telling others not to use it as it is intended for internal
use
```suggestion
fn try_new_with_schema(
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
