returnString commented on a change in pull request #9704:
URL: https://github.com/apache/arrow/pull/9704#discussion_r596399314
##########
File path: rust/datafusion/src/optimizer/constant_folding.rs
##########
@@ -188,6 +188,97 @@ impl<'a> ExprRewriter for ConstantRewriter<'a> {
right,
},
},
+ Operator::Plus => match (left.as_ref(), right.as_ref()) {
+ (
+ Expr::Literal(ScalarValue::Float64(l)),
+ Expr::Literal(ScalarValue::Float64(r)),
+ ) => match (l, r) {
+ (Some(l), Some(r)) => {
+ Expr::Literal(ScalarValue::Float64(Some(l + r)))
+ }
+ _ => Expr::Literal(ScalarValue::Float64(None)),
+ },
+ (
+ Expr::Literal(ScalarValue::Int64(l)),
+ Expr::Literal(ScalarValue::Int64(r)),
+ ) => match (l, r) {
+ (Some(l), Some(r)) => {
+ Expr::Literal(ScalarValue::Int64(Some(l + r)))
+ }
+ _ => Expr::Literal(ScalarValue::Int64(None)),
+ },
+ (
+ Expr::Literal(ScalarValue::Int32(l)),
+ Expr::Literal(ScalarValue::Int32(r)),
+ ) => match (l, r) {
+ (Some(l), Some(r)) => {
+ Expr::Literal(ScalarValue::Int32(Some(l + r)))
+ }
+ _ => Expr::Literal(ScalarValue::Int64(None)),
Review comment:
Would it be worth adding some kind of macro to simplify the maintenance
of this section? I'm not sure if this is intentionally an Int64 or whether this
is meant to be an Int32, and I wonder if we could have a macro that reduces
these maths ops to just the ScalarValue variant and an operator (slight
awkwardness: can't pass operators to macros).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
