nathanb9 commented on code in PR #23215:
URL: https://github.com/apache/datafusion/pull/23215#discussion_r3486456364
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -3368,6 +3417,27 @@ mod tests {
assert_eq!(simplify(expr_eq), lit(true));
}
+ #[test]
+ fn test_simplify_nested_associative_expr() {
Review Comment:
small: maybe add negative coverage for float addition and for separated
literals like `(1 + c1) + 2`
##########
datafusion/optimizer/src/simplify_expressions/utils.rs:
##########
@@ -290,6 +294,58 @@ pub fn is_lit(expr: &Expr) -> bool {
matches!(expr, Expr::Literal(_, _))
}
+pub fn has_associative_op(expr: &Expr, info: &SimplifyContext) -> Result<bool>
{
+ let op = match expr {
+ Expr::BinaryExpr(expr) => expr.op,
+ _ => unreachable!(),
+ };
+ let datatype = info.get_data_type(expr)?;
+ // TODO: add other associative ops
+ match op {
+ Operator::Plus => Ok(datatype.is_integer()),
+ Operator::StringConcat => Ok(datatype.is_string() ||
datatype.is_binary()),
+ _ => Ok(false),
+ }
+}
+
+pub fn has_adjacent_literals(expr: &Expr) -> bool {
+ struct AdjacentLiteralVisitor {
+ op: Operator,
+ last_expr_was_literal: bool,
+ }
+
+ impl<'n> TreeNodeVisitor<'n> for AdjacentLiteralVisitor {
+ type Node = Expr;
+
+ fn f_down(&mut self, node: &'n Self::Node) ->
Result<TreeNodeRecursion> {
Review Comment:
`last_expr_was_literal` is not reset when visiting a nonliteral. For
`(lit(1) + col("c1")) + lit(2)`, the rewrite appears to rebuild the same
expression but still results in `Transformed::yes`
Instead you could make `has_adjacent_literals` inspect the flattened operand
sequence for the same operator, and reset adjacency whenever a nonliteral
operand appears.
--
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]
