LuciferYang commented on code in PR #13002:
URL: https://github.com/apache/gluten/pull/13002#discussion_r4012228500
##########
gluten-substrait/src/main/scala/org/apache/gluten/utils/DecimalArithmeticUtil.scala:
##########
@@ -62,6 +62,9 @@ object DecimalArithmeticUtil {
resultPrecision = intDig + decDig
resultScale = decDig
}
+ // Remainder and Pmod land here: isDecimalArithmetic admits them but no
result type is
+ // derived above. On the transformCheckOverflow path this throw is what
makes decimal % and
+ // pmod fall back.
Review Comment:
Yes, this is a pre-existing limitation rather than something this PR
introduces — the throw has been there since the gluten-substrait split, and
this change only preserves it (the PR is a no-op removal of the
PromotePrecision machinery).
On fixing it via `resultDecimalType`: that calculation is necessary but not
sufficient. Spark's `Pmod.resultDecimalType` follows the same Remainder rule,
so the Gluten side would be small. But Velox's sparksql library registers
decimal arithmetic for add/subtract/multiply/divide only
(`velox/functions/sparksql/DecimalArithmetic.cpp`); `remainder`/`pmod` are
registered for integral and floating types only, not decimal. So if we dropped
the throw and emitted a decimal `modulus`/`pmod`, Velox has no decimal kernel
to resolve it, and it would not offload — the explicit throw is the cleaner
fallback point. Supporting it would need the decimal `remainder`/`pmod` kernels
added in Velox first, then the type calc here.
That is a real enhancement rather than part of this cleanup, so I'd rather
keep it out of this PR and open a follow-up. Happy to file one.
--
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]
