hadrian-reppas commented on code in PR #23215:
URL: https://github.com/apache/datafusion/pull/23215#discussion_r3486891446
##########
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:
I kept the `AdjacentLiteralVisitor` and changed it so that it updates
`last_expr_was_literal` correctly. `is_associative_with_adjacent_literals`
needs to take the expression by reference because we call it from the big match
statement in the `Simplifier`. I'm not sure I can construct the flattened
operand sequence with just a reference to the expression. I suppose I could
construct a `Vec<&Expr>`, but I'm not sure how much better that would be.
Unless I could somehow write one function that does `&Expr -> Vec<&Expr>` and
`Expr -> Vec<Expr>`.
--
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]
