wlhjason commented on code in PR #20597:
URL: https://github.com/apache/datafusion/pull/20597#discussion_r3293527650
##########
datafusion/substrait/src/logical_plan/producer/expr/scalar_function.rs:
##########
@@ -264,57 +297,21 @@ pub fn from_between(
low,
high,
} = between;
- if *negated {
- // `expr NOT BETWEEN low AND high` can be translated into (expr < low
OR high < expr)
- let substrait_expr = producer.handle_expr(expr.as_ref(), schema)?;
- let substrait_low = producer.handle_expr(low.as_ref(), schema)?;
- let substrait_high = producer.handle_expr(high.as_ref(), schema)?;
-
- let l_expr = make_binary_op_scalar_func(
- producer,
- &substrait_expr,
- &substrait_low,
- Operator::Lt,
- );
- let r_expr = make_binary_op_scalar_func(
- producer,
- &substrait_high,
- &substrait_expr,
- Operator::Lt,
- );
- Ok(make_binary_op_scalar_func(
- producer,
- &l_expr,
- &r_expr,
- Operator::Or,
- ))
+ let expr = if *negated {
+ // `expr NOT BETWEEN low AND high` can be translated into (expr < low
OR high < expr)
+ Expr::or(
+ Expr::lt(*expr.clone(), *low.clone()),
+ Expr::lt(*high.clone(), *expr.clone()),
+ )
Review Comment:
This is actually just a refactor of the existing logic to transform
`BETWEEN` into a Boolean expression!
The previous implementation used 6 direct calls to
`make_binary_op_scalar_func`. Rather than adding a data type and nullability to
every one of these, I thought this was a good chance to vastly simplify the
implementation.
--
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]
