fix: ignore zero and NaN sign bits in filters

[jonasdedden]

Fixes #9315. Fixes #9316. Continues #9324 and supersedes #9323.

This builds on the bot PR #9324 and keeps its approach. On top of it, it simplifies the implementation, fixes an index regression the original PR introduced, and folds in the signed-zero fix from #9323 using the same mechanism.

NaN order in filters (#9315)

Arrow compares floats by total order. A NaN with its sign bit set sorts below -inf, so x > 1.0 missed negative NaNs and x < 1.0 returned them. #9324 fixes this by rewriting comparisons: x > c becomes x > c OR x < -inf (the range below -inf holds exactly the negative NaNs), x < c becomes x < c AND x >= -inf, and column-to-column comparisons clear the NaN sign with an internal UDF.

Total order also keeps -0.0 and +0.0 apart. #6236 fixed that for comparisons against a zero literal, but a = b, a < b and array_has(l, 0.0) still compared by encoding (#9316).

Signed zero (#9316)

Instead of adding a second UDF as #9323 does, column-to-column comparisons (and NaN bounds) go through one internal comparison UDF, _lance_compare_floats(a, b, op), which ignores the sign of both zeros and NaNs. It replaces #9324's NaN normalizer and clears the sign bits in registers while comparing, so neither operand is copied. A column comparison costs about 1.2× the unfixed kernel; see the benchmark. array_has(l, v) for a zero or NaN v becomes array_has_any(l, [-v, +v]): one probe, so a volatile haystack is still evaluated once, and LabelList indices still apply. This covers NaN needles as well, which #9323 did not.

Additional fix: BETWEEN lost its single index range

With #9324, x BETWEEN a AND b becomes (x >= a OR x < -inf) AND x <= b AND x >= -inf. The index optimizer can't merge ranges across an OR, so this ran three index searches, one of them for every row above a.

A lower bound already excludes negative NaNs. When an AND chain bounds the same operand from both sides, the added x < -inf and x >= -inf cancel out, so the rewrite now drops both. BETWEEN and x > a AND x < b are one index range again. NOT BETWEEN and one-sided bounds are unchanged. The guards are dropped before the lower bounds are unwrapped, so x BETWEEN -inf AND b keeps x >= -inf, also when the scanner re-optimizes a filter decoded from Substrait.

Additional fix: volatile operands are evaluated once

The range form names its operand twice, so random() < 0.5 drew two values and the two comparisons could disagree. A volatile operand now goes through the comparison UDF instead, like a NaN bound, which evaluates it once. Such operands can't use an index anyway.

Additional fix: Substrait round-trip of the internal UDF

The comparison UDF is part of the optimized filter, but the Substrait decoder resolves functions by name from the caller's SessionState. A column-to-column float filter or a NaN range therefore failed to decode with Unsupported function name. Scalar and aggregate decoding now register the UDF in a copy of the caller's state, and round-trip tests with a plain DataFusion state pin both decoders.

Differences to bot PRs

Additional infos respective to both previous bot PRs

Simplification

• Build both sign encodings of zero and NaN with copysign instead of bit masks
• Replace the per-type NaN normalization UDF with one generic comparison kernel, and use take_function_args for argument checks
• Merge the two literal comparison rewrites into one function and remove unreachable branches
• Build the IN list for equality once
• Zone map: treat a NaN-bearing zone as a candidate for any range without a lower bound, instead of matching only the exact -inf range. In the full query test suite, no other range with a missing lower bound ever reaches the zone map, so pruning is unchanged
• No mem-WAL production change: since feat(mem_wal): answer memtable filters with Lance's scalar expression pass #9804 the memtable plans filters through the same scalar expression pass, so the rewrite applies there unchanged. A test asserts the rows of zero, NaN and range filters through the memtable B-tree.

Review follow-up

The points raised by the bot reviews on #9324 still hold:

• Comparisons against a literal stay indexable, including for computed operands such as JSON paths.
• The UDF does not copy its input at all: it compares in place, which supersedes the unchanged-array fast path.
• The extra index search for one-sided > and >= remains as accepted. Two-sided bounds no longer pay it.
• Volatile operands are evaluated once, the blocker raised on fix: ignore NaN sign bits in filters #9324 after it merged main.

The points raised by the bot reviews on #9323 are covered too:

• The comparison UDF is reconstructible when decoding Substrait, for both scalar and aggregate expressions, with codec round-trip tests using a plain SessionContext state.
• array_has stays a single probe, is stable across repeated optimization, and is tested against LabelList-indexed scans with NULL items, empty lists and NULL lists, plus a Substrait round-trip.

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[jonasdedden]

Commit 37018d1 was introduced as a superceding of #9323 to fix both #9315 and #9316 in one go.

[jonasdedden]

@Xuanwo, @LuciferYang @westonpace this is a somewhat larger PR, but it fixes two issues in one go. Would be great if it received some reviews! :)

[jonasdedden]

Fair point on read cost, so I benchmarked it: 4M f64 rows for filter evaluation, 2M rows in memory for scans, release build, medians. Absolute timings drift between runs, so the first table compares against the unfixed kernel measured in the same process.

Column-to-column x < y, filter evaluation

Version	Cost vs. main
main (wrong on -0.0 / -NaN)	1.0×
Normalize both operands, then compare (first version)	2.8× – 5.2×
Fused comparison UDF, first try	4.1× – 4.5×
+ branch-free sign test	1.4× – 1.7×
+ no validity bitmaps without NULLs, one loop per operand shape	1.2× – 1.5×

The normalizer copied both columns, and it got slower the more -0.0 / -NaN the data held. The fused kernel clears the sign bits in registers while comparing and doesn't allocate anything except the result.

Scans, 2M rows

Filter	main	This PR
x < y, full scan	10.7 ms	10.6 – 11.5 ms
x > 0.5, BTree	0.83 ms	0.91 – 1.10 ms
x >= 0.1 AND x <= 0.2, BTree	0.87 ms	0.88 – 0.93 ms
x > 0.5, no index	6.2 – 6.5 ms	6.4 – 6.8 ms

What remains is on one-sided > / >=: one extra small index search for the negative-NaN range, and two comparisons instead of one without an index. That doubles the filter kernel for those predicates but adds only about 3% to a scan. Two-sided ranges and equality are unchanged.

How to reproduce

Save this as rust/lance/examples/float_filter_bench.rs and run it on this branch and on its merge-base with main:

cargo run --release -p lance --example float_filter_bench

The eval lines print each filter's time relative to Arrow's plain total-order x < y kernel in the same process, which is how the first table is normalized. The scan lines match the second table.

// Minimal benchmark for float filters (lance-format/lance#9628).
// Save as rust/lance/examples/float_filter_bench.rs, then run it on this branch
// and on its merge-base with main:
//   cargo run --release -p lance --example float_filter_bench

use std::{sync::Arc, time::Instant};

use arrow_array::{
    ArrayRef, Float64Array, RecordBatch, RecordBatchIterator, cast::AsArray, types::Float64Type,
};
use lance::{Dataset, index::DatasetIndexExt};
use lance_datafusion::planner::Planner;
use lance_index::{IndexType, scalar::ScalarIndexParams};

/// `n` values uniform in [-1, 1), with 1% `-0.0` and 0.05% `-NaN`.
fn column(n: usize, mut seed: u64) -> ArrayRef {
    Arc::new(Float64Array::from_iter_values((0..n).map(|_| {
        seed ^= seed << 13;
        seed ^= seed >> 7;
        seed ^= seed << 17;
        let p = (seed >> 11) as f64 / (1u64 << 53) as f64;
        match p {
            p if p < 0.0005 => -f64::NAN,
            p if p < 0.0105 => -0.0,
            p => p * 2.0 - 1.0,
        }
    })))
}

fn batch(n: usize) -> RecordBatch {
    RecordBatch::try_from_iter([("x", column(n, 1)), ("y", column(n, 2))]).unwrap()
}

/// Median of 15 runs, in milliseconds.
fn median_ms(mut f: impl FnMut()) -> f64 {
    f();
    let mut ms: Vec<f64> = (0..15)
        .map(|_| {
            let start = Instant::now();
            f();
            start.elapsed().as_secs_f64() * 1e3
        })
        .collect();
    ms.sort_by(f64::total_cmp);
    ms[7]
}

#[tokio::main]
async fn main() {
    // Filter evaluation on one in-memory batch, relative to Arrow's plain
    // total-order `x < y` kernel measured in the same process.
    let data = batch(4_000_000);
    let planner = Planner::new(data.schema());
    let (x, y) = (
        data["x"].as_primitive::<Float64Type>(),
        data["y"].as_primitive::<Float64Type>(),
    );
    let reference = median_ms(|| drop(arrow_ord::cmp::lt(x, y).unwrap()));
    println!("total-order x < y kernel: {reference:.2} ms");
    for filter in ["x < y", "x = y", "x > 0.5", "x >= 0.1 AND x <= 0.2"] {
        let expr = planner.parse_filter(filter).unwrap();
        let physical = planner
            .create_physical_expr(&planner.optimize_expr(expr).unwrap())
            .unwrap();
        let ms = median_ms(|| drop(physical.evaluate(&data).unwrap()));
        println!("eval {filter:24} {ms:6.2} ms  {:.2}x", ms / reference);
    }

    // Scans of an in-memory dataset with a BTree index on `x`.
    let data = batch(2_000_000);
    let reader = RecordBatchIterator::new([Ok(data.clone())], data.schema());
    let mut dataset = Dataset::write(reader, "memory://", None).await.unwrap();
    let params = ScalarIndexParams::for_builtin(IndexType::BTree.try_into().unwrap());
    dataset
        .create_index_builder(&["x"], IndexType::BTree, &params)
        .await
        .unwrap();
    for (filter, use_index) in [
        ("x < y", false),
        ("x > 0.5", true),
        ("x >= 0.1 AND x <= 0.2", true),
        ("x > 0.5", false),
    ] {
        let mut ms = Vec::new();
        for _ in 0..15 {
            let start = Instant::now();
            let mut scanner = dataset.scan();
            scanner.filter(filter).unwrap().use_scalar_index(use_index);
            scanner.count_rows().await.unwrap();
            ms.push(start.elapsed().as_secs_f64() * 1e3);
        }
        ms.sort_by(f64::total_cmp);
        let index = if use_index { "btree" } else { "no index" };
        println!("scan {filter:24} {index:8} {:6.2} ms", ms[7]);
    }
}

[jonasdedden]

Pushed some changes, these include:

Rebased on current main

• Dropped the mem-WAL production change. Since feat(mem_wal): answer memtable filters with Lance's scalar expression pass #9804 the memtable plans filters through the scalar expression pass, so the rewrite applies there without its own code. The PR no longer changes builder.rs outside its tests.

New fixes

• 85efce7 Volatile operands are evaluated once. The range form named its operand twice, so random() < 0.5 drew two values and the two comparisons could disagree. A volatile operand now goes through _lance_compare_floats, like a NaN bound. This is the blocker the gatekeeper raised on fix: ignore NaN sign bits in filters #9324 after it merged main.
• 9eb23ff Faster comparison against a scalar. A bounds check inside the scalar loop stopped most of it from vectorizing and left it branching on each value's sign. Hoisting the check cuts a NaN bound over 4M unsorted f64 rows from 5.6 ms to 3.5 ms. This path covers only NaN bounds and volatile operands; column comparisons were already vectorized.

New tests (d59b0f3), ported from #9324

• Rows of zero, NaN and range filters through the memtable B-tree, with and without the index.
• The comparison UDF decoded through the aggregate Substrait consumer.
• Two-sided Float16 bounds staying a single index range.

[lance-gatekeeper]

⚠️ Gate recommendation: approve with a non-blocking risk.

The follow-up fixes repeated evaluation of volatile operands. Local checks confirm signed-zero/NaN results through the shared memtable planner merged in #9804, narrow index ranges, and scalar/aggregate Substrait decoding. The earlier required-lower-bound fix remains intact; no blocking findings remain.

The accepted remaining cost is one-sided >/>=: an extra negative-NaN index search or comparison. The fused kernel avoids copying its operands. Reported benchmarks cover in-memory f64 workloads; performance-sensitive adopters should measure their own workloads.