Dandandan commented on code in PR #20706:
URL: https://github.com/apache/datafusion/pull/20706#discussion_r2888118751
##########
datafusion/optimizer/src/eliminate_group_by_constant.rs:
##########
@@ -91,23 +99,47 @@ impl OptimizerRule for EliminateGroupByConstant {
}
}
-/// Checks if expression is constant, and can be eliminated from group by.
-///
-/// Intended to be used only within this rule, helper function, which heavily
-/// relies on `SimplifyExpressions` result.
-fn is_constant_expression(expr: &Expr) -> bool {
+/// Checks if a GROUP BY expression is redundant (can be removed without
+/// changing grouping semantics). An expression is redundant if it is a
+/// deterministic function of constants and columns already present as bare
+/// column references in the GROUP BY.
Review Comment:
Deterministic function in this code also seems quite precise for this
application?
In the code we check for deterministic functions (those in BinaryExpr), we
cannot detect all functional dependencies...
--
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]
