andygrove commented on PR #5169: URL: https://github.com/apache/datafusion-comet/pull/5169#issuecomment-5876324428
This is a light fully automated review since there are so many PRs open. `spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala:3458` still says "Integral divide still raises an unconverted Arrow error under ANSI" and links #5072, which this PR closes. I traced `_1 div _2` on the INT columns in that test through the new code. `CometIntegralDivide` casts both operands to `DECIMAL(19, 0)`, so the zero divisor reaches `decimal_integral_div`, which now returns a typed `SparkError::DivideByZero`. That error passes through the hand-built `CheckOverflow` (no `expr_id`) and picks up the `IntegralDivide`'s context in `Cast::evaluate`. So once this merges the comment is wrong, and the test still only checks a message substring, which a `CometNativeException` also satisfies (that is how it passes on main today). Could the ANSI branch call `checkSparkError(res, "DIVIDE_BY_ZERO")` the way the two `/` tests just above it do (lines 3427 and 3442), and drop the comment? `checkSparkError` fails if a `CometNativeException` shows up in the cause chain, so that would pin the int `div` shape alongside the decimal one the new test covers. -- 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]
