bgjackma commented on code in PR #12565:
URL: https://github.com/apache/datafusion/pull/12565#discussion_r1769665420
##########
datafusion/functions-aggregate/src/grouping.rs:
##########
@@ -59,9 +73,55 @@ impl Grouping {
/// Create a new GROUPING aggregate function.
pub fn new() -> Self {
Self {
- signature: Signature::any(1, Volatility::Immutable),
+ signature: Signature::variadic_any(Volatility::Immutable),
}
}
+
+ /// Create an accumulator for GROUPING(grouping_args) in a GROUP BY over
group_exprs
+ /// A special creation function is necessary because GROUPING has unusual
input requirements.
+ pub fn create_grouping_accumulator(
+ &self,
+ grouping_args: &[Arc<dyn PhysicalExpr>],
+ group_exprs: &[(Arc<dyn PhysicalExpr>, String)],
+ ) -> Result<Box<dyn GroupsAccumulator>> {
+ if grouping_args.len() > 32 {
+ return plan_err!(
+ "GROUPING is supported for up to 32 columns. Consider another \
+ GROUPING statement if you need to aggregate over more columns."
+ );
+ }
+ // The PhysicalExprs of grouping_exprs must be Column PhysicalExpr.
Because if
+ // the group by PhysicalExpr in SQL is non-Column PhysicalExpr, then
there is
+ // a ProjectionExec before AggregateExec to convert the non-column
PhysicalExpr
+ // to Column PhysicalExpr.
+ let column_index =
+ |expr: &Arc<dyn PhysicalExpr>| match
expr.as_any().downcast_ref::<Column>() {
+ Some(column) => Ok(column.index()),
+ None => internal_err!("Grouping doesn't support expr: {}",
expr),
+ };
Review Comment:
Can we look for equal PhysicalExprs?
The Postgres docs imply they do ~text comparison but I'm not sure how
accessible that info is at this layer.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
