[ABA-21] test(vortex-array): repro for float compare semantics divergence#18
Open
abnobdoss wants to merge 1 commit into
Open
[ABA-21] test(vortex-array): repro for float compare semantics divergence#18abnobdoss wants to merge 1 commit into
abnobdoss wants to merge 1 commit into
Conversation
… not IEEE 754 Add an ignored regression test documenting that vortex-array's float compare path uses Arrow's totalOrder semantics (NaN == NaN → true, -0.0 ≠ +0.0) rather than IEEE 754 / SQL default comparison semantics. Four probes cover the bug surface: - NaN == NaN → false (IEEE 754) but currently true (totalOrder) - +0.0 == -0.0 → true (IEEE 754) but currently false (totalOrder) - NaN < 1.0 → false (both; non-discriminating, pins the Lt contract) - NaN > 1.0 → false (IEEE 754) but currently true (totalOrder) — the SQL-visible failure: WHERE col > 0 silently includes NaN rows The test is left #[ignore]'d because the choice of semantics (totalOrder for dict/sort/pruning vs. IEEE 754 for SQL filters) needs an upstream design decision before a fix can land. See: https://linear.app/abanoubdoss/issue/ABA-21 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Abanoub Doss <abanoub.doss@gmail.com>
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.
Summary
#[ignore]'d testissue_aba21_float_compare_uses_ieee754_semanticstovortex-array/src/scalar_fn/fns/binary/compare.rsdocumenting that vortex-array's float compare path uses Arrow's totalOrder semantics rather than IEEE 754 / SQL default comparison semantics.develop— the test fails when un-ignored.Linear: https://linear.app/abanoubdoss/issue/ABA-21
What the test covers
Four probes against the
arrow_compare_arrayspath (compare.rs lines 174-181 /arrow_ord::cmp::*):NaN == NaNfalsetrue(FAILS)+0.0 == -0.0truefalse(FAILS)NaN < 1.0falsefalse(passes — both agree)NaN > 1.0falsetrue(FAILS —WHERE col > 0silently includes NaN rows)Bug sites on
developvortex-array/src/scalar_fn/fns/binary/compare.rs:174-181—arrow_ord::cmp::{eq, neq, gt, gt_eq, lt, lt_eq}used for primitive floats (totalOrder via Arrow ≥ 57)scalar_cmpat line 195 (partial_cmp) →PValue::partial_cmp→total_compare/is_eqviaNativePTypeOpen question
The intentional choice was made in vortex vortex-data#808 and confirmed in vortex-data#3958 ("totalOrder is what dict/hash/sort needs"). The unresolved question for this PR:
scalar_fncompare path, or should the entirePValuePartialOrd/PartialEqmove to IEEE 754?arrow_ordremain for sort/pruning (where totalOrder is desirable) but theOperator::{Eq, Lt, Gt, …}SQL-filter path switch to IEEE 754? Or is a single semantic across all surfaces acceptable?null(SQL three-valued logic) rather thanfalse?🤖 Generated with Claude Code