Skip to content

[FLINK-35276][table] Fix incorrect sort order for floating-point negative zero in generated comparators#28383

Open
Au-Miner wants to merge 1 commit into
apache:masterfrom
Au-Miner:FLINK-35276
Open

[FLINK-35276][table] Fix incorrect sort order for floating-point negative zero in generated comparators#28383
Au-Miner wants to merge 1 commit into
apache:masterfrom
Au-Miner:FLINK-35276

Conversation

@Au-Miner

Copy link
Copy Markdown
Contributor

What is the purpose of the change

The generated sort comparator in GenerateUtils.scala uses the ternary expression (a > b ? 1 : a < b ? -1 : 0) for FLOAT and DOUBLE types, which treats 0.0 and -0.0 as equal. This causes SortCodeGeneratorTest.testMultiKeys to fail intermittently when random test data contains negative zero, because the Java reference sort uses Float.compare() / Double.compare() which distinguish +0.0 from -0.0.

Brief change log

  • GenerateUtils.scala: Split FLOAT and DOUBLE out of the integer comparison branch; generate Float.compare() / Double.compare() calls instead of the ternary expression
  • SortCodeGeneratorTest.java: Add testFloatNegativeZeroSortKey() with deterministic 0.0f / -0.0f test data to verify the fix without relying on random seed probability

Verifying this change

This change added a new test SortCodeGeneratorTest#testFloatNegativeZeroSortKey that constructs rows with 0.0f and -0.0f sort keys and asserts the generated comparator produces the same order as the Java reference sort.

The existing testMultiKeys (100 random iterations) also covers this path and will no longer flake.

Does this pull request potentially affect one of the following parts

  • Dependencies (does it add or upgrade a dependency): no
  • The public API, i.e., is any changed class annotated with @Public(Evolving): no
  • The serializers: no
  • The runtime per-record code paths (performance sensitive): yes — sort comparison is on the per-record path, but Float.compare() / Double.compare() are JIT intrinsics with negligible overhead compared to the previous ternary expression
  • Anything that affects deployment or recovery: no
  • The (broadcasting) coordination functionality: no

Documentation

  • Does this pull request introduce a new feature? no
  • If not, how is the change documented? Not applicable.

@flinkbot

flinkbot commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants