amitvijapur commented on code in PR #24409:
URL: https://github.com/apache/datafusion/pull/24409#discussion_r3835770847
##########
datafusion/spark/src/function/math/modulus.rs:
##########
@@ -182,7 +304,7 @@ impl Default for SparkPmod {
impl SparkPmod {
pub fn new() -> Self {
Self {
- signature: Signature::numeric(2, Volatility::Immutable),
+ signature: Signature::user_defined(Volatility::Immutable),
Review Comment:
Done in ff84872d. `one_of` with `Coercible([Decimal, Decimal])` then
`Numeric(2)`, and `pmod_numeric_coercion` is gone with it.
The null result types are decided in `return_type` now. I checked these
against `mod` rather than guessing: `arrow_typeof(mod(NULL, NULL))` is
`Float64` and `arrow_typeof(mod(2.5::decimal(3,1), NULL))` is `Decimal128(3,
1)`, and `pmod` now returns the same.
One knock-on: since the null is no longer coerced away, `spark_pmod` can
receive `DataType::Null` arrays, so it short-circuits to a null array of the
result type instead of failing on `ScalarValue::new_zero`.
`pmod(NULL, 3::int)` does not plan under `Numeric(2)`. `mod` rejects it
identically, and `pmod` did too before this PR when it used
`Signature::numeric(2)`, so I've covered it with a `statement error` test
rather than treating it as a regression to fix here.
--
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]
