[SPARK-59093][PS] Fix incorrect results and arithmetic errors in floor division - #58397
Open
Spenserrrr wants to merge 2 commits into
Open
[SPARK-59093][PS] Fix incorrect results and arithmetic errors in floor division#58397Spenserrrr wants to merge 2 commits into
Spenserrrr wants to merge 2 commits into
Conversation
`_floor_divide_floating`, `_floor_divide_integral` and `_floor_divide_func` were added to `numpy_compat.py` for the `np.floor_divide` mapping, but the `//` operator in `data_type_ops/num_ops.py` needs the same expression. `num_ops` is the core operator implementation and `numpy_compat` is the NumPy compatibility layer on top of it, so importing the helpers from there would invert the layering. `pyspark.pandas.utils` already hosts shared column expressions such as the `compare_null_first` family, and it imports no pandas-on-Spark module other than `_typing` and `typedef.typehints`, so both callers can reach it without a cycle. This is a pure move: the functions are unchanged, `numpy_compat` keeps its mapping entry and imports the helper, and the existing test imports it from its new home.
…r division `IntegralOps` and `FractionalOps` computed `//` as `F.floor(a / b)` for both the forward and the reflected operator. That is wrong in three ways and fails outright in a fourth: - the division rounds before the floor, so `ps.Series([1.0]) // 0.1` returned 10.0 where pandas returns 9.0; - integral operands are cast to double first, so `-(2**53 + 1) // 2` returned -4503599627370496.0 instead of -4503599627370497; - an infinite divisor returned 0.0 for every dividend, where pandas derives the quotient from the remainder and returns -1.0 for opposite signs; - `F.floor` returns a bigint, so an infinite or out-of-range quotient raised ARITHMETIC_OVERFLOW under ANSI mode and saturated to Long.MAX_VALUE without it, for example `ps.Series([np.inf]) // 2.0` and `1e300 // 1e-300`. The reflected operator additionally raised DIVIDE_BY_ZERO for `0 // ps.Series([0])`, because its zero-divisor branch divided an infinity by the dividend. SPARK-58581 fixed the same arithmetic in the `np.floor_divide` mapping, which is unreachable: `np.floor_divide` is dispatched to this dunder operation before the mapping registry is consulted. The four methods now reuse that expression, so both paths agree and the zero-divisor answers come from literals rather than a division, which makes the result the same under ANSI mode and without it. A quotient above 2**53 is still rounded, since the result type has to stay double for the infinities a zero divisor produces.
Spenserrrr
marked this pull request as ready for review
August 28, 2026 23:24
Contributor
Author
|
Hi @zhengruifeng! This is the follow-up I mentioned in #58306. That PR fixed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
IntegralOpsandFractionalOpsnow compute//and its reflected form with_floor_divide_func, the expression added in SPARK-58581. The first commit moves that helper fromnumpy_compat.pytopyspark.pandas.utilsso the core operator does not import the NumPy compatibility layer.Why are the changes needed?
F.floor(a / b)rounds before flooring, casts integral operands to double, and returns a bigint, so//gives wrong values (ps.Series([1.0]) // 0.1returns 10.0, pandas 9.0) and raisesARITHMETIC_OVERFLOWwhen the quotient is infinite. SPARK-58581 fixed this arithmetic in thenp.floor_dividemapping, but that entry is unreachable, so the operator users actually call was left unchanged.Does this PR introduce any user-facing change?
Yes.
//matches pandas where it previously did not:ps.Series([1.0, 10.0]) // 0.1returns[9.0, 99.0]instead of[10.0, 100.0]. A quotient above2**53is still rounded, since the result type has to stay double.How was this patch tested?
Rows added to the existing
test_floordivandtest_rfloordiv, each confirmed to fail without the change, plus the existing pandas-on-Spark suites with ANSI mode on and off.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)