viirya commented on code in PR #7379:
URL: https://github.com/apache/datafusion-comet/pull/7379#discussion_r4437583137
##########
spark/src/main/scala/org/apache/comet/serde/arithmetic.scala:
##########
@@ -190,7 +190,7 @@ object CometAdd extends CometExpressionSerde[Add] with
MathBase {
// proto stays shallow and doesn't overflow protobuf's recursion limit
when re-parsed.
val operands = flattenAssociative(
expr,
- { case _: Add => true; case _ => false },
+ { case a: Add => a.evalMode == expr.evalMode; case _ => false },
Review Comment:
The scaladoc on `createBalancedMathExpr` says a single
`returnType`/`evalMode` is correct for every inner node because casts act as
flatten boundaries. That now also relies on this check, so could it mention
that a nested `Add`/`Multiply` in a different eval mode is a flatten boundary
too? That makes it less likely to be dropped in a later refactor.
--
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]
