alamb commented on code in PR #3286:
URL: https://github.com/apache/arrow-datafusion/pull/3286#discussion_r959613756
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1314,6 +1314,32 @@ pub struct Aggregate {
}
impl Aggregate {
+ pub fn try_new(
+ input: Arc<LogicalPlan>,
+ group_expr: Vec<Expr>,
+ aggr_expr: Vec<Expr>,
+ schema: DFSchemaRef,
+ ) -> datafusion_common::Result<Self> {
+ if group_expr.is_empty() && aggr_expr.is_empty() {
+ return Err(DataFusionError::Plan(
+ "Aggregate requires at least one grouping or aggregate
expression"
+ .to_string(),
+ ));
+ }
+ let group_expr_count = grouping_set_expr_count(&group_expr)?;
+ if schema.fields().len() != group_expr_count + aggr_expr.len() {
+ return Err(DataFusionError::Plan(
+ "Aggregate schema has wrong number of fields".to_string(),
+ ));
Review Comment:
```suggestion
return Err(DataFusionError::Plan(
format!("Aggregate schema has wrong number of fields.
Expected {} got {}",
schema.fields().len(), group_expr_count + aggr_expr.len()
)
));
```
##########
datafusion/expr/src/utils.rs:
##########
@@ -45,6 +45,22 @@ pub fn exprlist_to_columns(expr: &[Expr], accum: &mut
HashSet<Column>) -> Result
Ok(())
}
+/// Count the number of distinct exprs in a list of group by expressions. If
the
+/// first element is a `GroupingSet` expression then it must be the only expr.
+pub fn grouping_set_expr_count(group_expr: &[Expr]) -> Result<usize> {
+ if let Some(Expr::GroupingSet(grouping_set)) = group_expr.first() {
+ if group_expr.len() > 1 {
+ return Err(DataFusionError::Plan(
+ "Invalid group by expressions, GroupingSet must be the only
expression"
+ .to_string(),
+ ));
+ }
+ Ok(grouping_set.distinct_expr().len())
+ } else {
+ Ok(group_expr.len())
Review Comment:
FWIW the sqlparser won't allow `group by *`
```
❯ select count(*) from foo group by *; 🤔 Invalid statement: sql parser
error: Expected an expression:, found: *
```
--
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]
