Spenserrrr opened a new pull request, #58306: URL: https://github.com/apache/spark/pull/58306
### What changes were proposed in this pull request? `_floor_divide_func` computed `floor(c1 / c2)`, which diverges from NumPy and pandas in two ways. This PR fixes both. - **Floating operands.** Flooring the quotient is wrong when the division rounds up across an integer. `1.0 / 0.1` is not representable and rounds to exactly `10.0`, so its floor is 10 while NumPy, pandas and Python all return 9. A remainder is exact in IEEE arithmetic, so the quotient is now derived from it, as NumPy's `npy_divmod` does, with the same sign correction and snap-to-nearest. - **Integral operands.** They were cast to double first, which drops the low bits above 2^53 (Spark's `/` always divides as double), so `-9007199254740993 // 2` returned `-4503599627370496` rather than `-4503599627370497`. They now divide in integer space and only the result is cast. A second commit guards the one quotient a long cannot hold, `-2**63 // -1`, where Spark's integer division raises `ARITHMETIC_OVERFLOW`. The mapping's original pandas UDF delegated to NumPy and returned NumPy's wrapped value there, so the guard restores that result. ### Why are the changes needed? `1.0 // 0.1` returning 10 is inconsistent with Spark's own `%`: `double % double` gives `0.09999999999999995`, and `a == b*q + r` with `r` in `[0, b)` holds for `q = 9` but implies a negative remainder for `q = 10`. pandas, NumPy and Python all return 9. Above 2^53 the integral path lost exact values that `int64` can represent and the mapping's earlier UDF preserved. ### Does this PR introduce _any_ user-facing change? No. `np.floor_divide` is dispatched to the pandas-on-Spark `floordiv` dunder operation before this registry is consulted (`maybe_dispatch_ufunc_to_dunder_op` runs first and `floor_divide` is aliased to `floordiv`), so this mapping is not reached today, as the comment on the entry notes. Worth flagging for reviewers: the same two defects are live in the path that *is* reached, `data_type_ops/num_ops.py` (`IntegralOps.floordiv`, `FractionalOps.floordiv` and both `rfloordiv`), which use `F.floor(lc / rc)`. There, `ps.Series([1.0]) // 0.1` returns `10.0` against pandas' `9.0`, and `psser // 2` is off by one above 2^53. That is deliberately not fixed here: it is a user-facing behaviour change on a long-standing path and deserves its own JIRA and a maintainer's call on whether to make it. ### How was this patch tested? `test_floor_divide_func` gains two frames, compared exactly rather than with `almost=True` (a relative tolerance accepts an off-by-one at these magnitudes): a frame with divisors binary cannot represent (`1.0 // 0.1`, `2.0 // 0.2`, ...), and a frame of integral operands above 2^53. The overflow commit adds a `-2**63` row. Each new row was checked to be load-bearing by reverting `numpy_compat.py` alone and confirming the test fails. `pyspark.pandas.tests.test_numpy_compat` and `pyspark.pandas.tests.connect.test_parity_numpy_compat` both pass. The remainder-based expression was also compared against `np.floor_divide` over 4000 random float pairs (0 mismatches, versus 35 for the previous formula) and the claims in the new comments were each checked against a live session rather than reasoned. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) -- 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]
