avantgardnerio commented on code in PR #3286:
URL: https://github.com/apache/arrow-datafusion/pull/3286#discussion_r957601026
##########
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() {
Review Comment:
It's weird that this function takes either a GroupingSet expression, or an
arbitrary expression. I dealt with this a lot in the subquery decorrelation -
it almost feels like dynamic typing. I think the root cause might be that many
of the enum values have fields inside them, rather than a struct - so it's
impossible to de-reference them and pass them between functions.
How would you feel about blanket converting all enum values like
`InSubquery`, `Exists`, etc to point to structs?
##########
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:
Do we need to do other validation? Should I be able to `group by *`?
--
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]
