[LiuYC3213687107]

Which issue does this PR close?

• Closes Wrong results: ORDER BY <double>, ... LIMIT n drops -0.0 rows when 0.0 arrives in an earlier batch (TopK dynamic filter) #26618.

Rationale for this change

ORDER BY <double>, ... LIMIT n can drop -0.0 rows that belong in the result when the input arrives in more than one batch.

The TopK heap orders -0.0 before 0.0 (IEEE 754 totalOrder, same as the full sort), but since #22835 comparisons treat -0.0 = 0.0. With a heap boundary of (0.0, 1) the dynamic filter becomes x < 0.0 OR (x = 0.0 AND y < 1), which rejects (-0.0, 5) even though the heap would rank it ahead of the boundary.

What changes are included in this PR?

In TopK::build_filter_expression, when the threshold of a sort key is a floating-point zero (either sign):

• use a non-strict bound for that key (x <= 0.0 for ASC, x >= 0.0 for DESC), so both zeros pass the filter, and
• stop adding predicates for later sort keys, since the equality on this key no longer identifies the heap's tie group.

The dynamic filter is only a pruning hint, so making it more conservative for this one value cannot change results; the heap still decides the final order. This mirrors how the function already stops at the first nested-type key.

What is the testing strategy for this PR?

Added sqllogictest cases to topk.slt that reproduce the issue with two batches, for both ASC and DESC. Without the fix the LIMIT 2 queries return (0, 7), (0, 1) instead of (-0, 5), (-0, 7). (sqllogictest prints -0.0 as 0, so the y column distinguishes the rows.)

Are there any user-facing changes?

No API changes. ORDER BY ... LIMIT queries over float columns containing both 0.0 and -0.0 now return correct results.

AI assistance

This change was prepared with Claude Code (Anthropic): the fix in datafusion/physical-plan/src/topk/mod.rs, the tests in topk.slt, and this description. I reviewed and verified the change myself.

🤖 Generated with Claude Code

[mkleen]

The tests expect an order we plan to change. The expected rows depend on -0.0 sorting before 0.0. Under #26138, -0.0 and 0.0 will count as equal when sorting, so y decides the order. Then ORDER BY x, y LIMIT 2 should return (0,1), (-0,5) rather than (-0,5), (-0,7), and these tests will fail. That's fine for now, since the bug only happens with today's order. Could you add a comment above the tests so the next person knows why they changed? Something like:

# The expected rows assume -0.0 sorts before 0.0 (the current order).
# They will change when -0.0 and 0.0 are treated as equal when sorting,
# see https://github.com/apache/datafusion/issues/26138

[mkleen]

Thanks for working on this, and the fix makes sense. There is now a common place for this kind of check: datafusion_common::float_semantics, added in #26595. Putting the zero check there would keep all of this logic together.

[mkleen]

Thanks for working on this, and the fix makes sense. There is now a common place for this kind of check: datafusion_common::float_semantics, added in #26595. Putting the zero check there would keep all of this logic together.

[LiuYC3213687107]

Thanks for the review! Done in 881738e: moved the zero check into datafusion_common::float_semantics as is_float_zero (with a unit test), and added the comment above the tests pointing to #26138.

[mkleen]

@LiuYC3213687107 Thanks. While having a second look at this I think this would fit better to move to ScalarValue::is_float_zero similiar to #26727

[LiuYC3213687107]

Makes sense, thanks! Moved it to ScalarValue::is_float_zero next to normalize in da40aea, with the unit test in scalar/mod.rs.

[mkleen]

This looks good to me. I think the fix makes sense to merge as a correctness fix now.
The underlying problem is that sorting puts -0.0 before 0.0 while equality treats them as the same value. #26138 is suppose to fix this by merging -0.0 and 0.0 together. This should be marked temporary and linked to #26138.