Repository navigation
Conversation
Arrow's total order places a negative NaN below every other value and a positive NaN above, and DataFusion compares floats that way, so `x < 0` matches a `-NaN` row and `x > 100` a `+NaN` row. Zone statistics record how many NaNs a zone holds but not their sign, and the reader pruned ranges from min/max alone: a zone with a negative NaN and ordinary values looked like `[5, +NaN]` and lost its `-NaN` rows to `x < 0`, a zone of only NaNs had null bounds and was pruned for `x > 100`, and a NaN literal as a bound could prune ordinary rows. The reader now treats nulls, NaNs and ordinary values as separate groups and keeps a zone when any of them can match. Any zone with `nan_count > 0` stays a candidate for a range query; a NaN literal as a bound lies beyond the ordinary values on the side its sign names; equality with an ordinary value still uses the bounds, since a NaN cannot satisfy it. Rows of kept zones are filtered as before, so results are exact and the cost is less pruning for zones that hold NaNs. The writer records `max = +NaN` only when the zone holds a positive NaN, the zone's largest value; a zone whose NaNs are all negative keeps its ordinary maximum. `lance-arrow-stats` reports `negative_nan_count` next to `nan_count` so the writer can tell the two apart. Index and seed schemas are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…type
Use a NaN literal of each column's own type (`arrow_cast('NaN',
'Float16')` and so on) so the planner does not cast the column and the
filter stays eligible for the index, assert through the explain plan that
representative ordinary and NaN predicates are planned as a
ScalarIndexQuery on every column, and check that the typed literals
carry the intended sign by counting the rows they match.
Describe the `+NaN` max as a marker for a positive NaN rather than the
zone's largest value, in the code comments and in the zone map format
doc, and note there that range queries keep every zone with NaNs. Drop a
redundant local in the seed reader.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Zone statistics gain a nullable UInt32 `negative_nan_count` column in the index file and the write seed: how many of the zone's `nan_count` NaNs carry the sign bit. Null means the sign split is unknown, which is how every index and seed written before this column reads back; non-float columns write 0, and a present column of another type or a count above `nan_count` is an error. Old zones are not rewritten and merge as they are. With the signs known, the reader prunes zones with NaNs by range again: a zone is kept when its nulls, ordinary values, negative NaNs or positive NaNs can match. Unknown counts keep both signs possible, and a NaN literal as a bound never rules out NaNs of its own sign, since the counts do not record payloads. `max` keeps the canonical `+NaN` marker for a positive NaN; a zone whose NaNs are all negative keeps its ordinary maximum. A dataset written by the previous writer is checked in as a fixture, with its generator, to prove that old indices and old seeds load with unknown counts, that old and new zones merge, and that queries with the index return the rows a scan returns in every case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
github-actions
Bot
added
A-python
|
Important Format specification voteThis PR modifies the Lance format specification, so it requires 3 binding +1 votes from PMC members (excluding the proposer), at least one of them on the latest commit, and a minimum 72-hour voting period, weekends excluded, before it can merge. Vote by approving this PR (+1) or requesting changes (−1, a veto). See the voting process. Approvals carry over across pushes, so a rebase or a typo fix does not send everyone back to re-vote. Whoever approves the latest commit is vouching that nothing substantive has changed since the earlier approvals; if something has, ask for fresh votes. Status: ❌ Blocked — 0 of 3 required approvals
Updated automatically by the format-spec vote gate, which re-checks every 15 minutes — just voted? Re-check now (press Run workflow; leave the input blank to re-check every open format PR). A PMC member may apply the |
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
Land a backward-compatible version of #9877 first, keep this PR to the signed-NaN format proposal, and move its implementation to a follow-up. The proposed contract must retain the any-NaN max marker so released readers keep sound global bounds.
Please mark this PR with the breaking-change label.
| max, and a positive NaN is the largest value in the zone under Arrow's total | ||
| ordering, so the bound is still correct. The ordinary maximum is hidden behind | ||
| it; a reader that needs it must scan. A zone whose NaNs are all negative keeps | ||
| its ordinary maximum. |
There was a problem hiding this comment.
Removing the +NaN marker for negative-only zones makes released readers report a range that excludes live values. v13.0.0’s value_range_over detects NaNs through max, without checking nan_count or the new column. For [1.0, -NaN], this writer persists min = max = 1.0, and the released method returns [1.0, 1.0]. A mixed-version or rolled-back reader using column_value_range for pruning can silently lose matching rows; the previous writer returned None for this zone.
Preserve max = +NaN whenever nan_count > 0 in both index and seed statistics. New readers can still use the sign counts; if finer pruning requires the ordinary maximum, store it separately instead of changing the marker’s meaning.
Reproducer
Added to the existing scalar::zonemap::tests module at this head. The helper is v13.0.0’s folding method copied verbatim, with only its name changed. The test trains, writes, and loads the current index and checks the old marker as a control.
impl ZoneMapIndex {
fn gate_released_value_range_over<'a>(
segments: impl IntoIterator<Item = &'a Self>,
) -> Option<(ScalarValue, ScalarValue)> {
let mut min: Option<&ScalarValue> = None;
let mut max: Option<&ScalarValue> = None;
for seg in segments.into_iter() {
// Nested types have no meaningful ordering
if seg.data_type.is_nested() {
return None;
}
for zone in seg.zones.iter() {
// Legacy Decimal zones can contain comparable values even though their
// extrema were never written, so skipping them would produce a subset.
if Self::zone_has_missing_extrema(zone) && Self::zone_has_comparable_values(zone) {
return None;
}
if Self::scalar_is_nan(&zone.max) {
return None;
}
if Self::scalar_is_finite_bound(&zone.min)
&& min.is_none_or(|cur| zone.min.partial_cmp(cur).is_some_and(|o| o.is_lt()))
{
min = Some(&zone.min);
}
if Self::scalar_is_finite_bound(&zone.max)
&& max.is_none_or(|cur| zone.max.partial_cmp(cur).is_some_and(|o| o.is_gt()))
{
max = Some(&zone.max);
}
}
}
Some((min?.clone(), max?.clone()))
}
}
#[tokio::test]
async fn gate_released_range_preserves_unknown_for_negative_nans() {
let mut index = train_and_load::<Float32Type>(vec![vec![Some(1.0), Some(-f32::NAN)]]).await;
assert_eq!(index.value_range(), None, "current reader remains safe");
assert_eq!(index.zones[0].min, ScalarValue::Float32(Some(1.0)));
assert_eq!(index.zones[0].max, ScalarValue::Float32(Some(1.0)));
assert_eq!(index.zones[0].nan_count, 1);
assert_eq!(index.zones[0].negative_nan_count, Some(1));
assert!(ScalarValue::Float32(Some(-f32::NAN)) < index.zones[0].min);
let saved_max = index.zones[0].max.clone();
Arc::get_mut(&mut index).unwrap().zones[0].max = ScalarValue::Float32(Some(f32::NAN));
assert_eq!(ZoneMapIndex::gate_released_value_range_over([index.as_ref()]), None,
"the released writer's +NaN marker made this unknown");
Arc::get_mut(&mut index).unwrap().zones[0].max = saved_max;
assert_eq!(ZoneMapIndex::gate_released_value_range_over([index.as_ref()]), None,
"released readers must not claim a range that excludes a live negative NaN");
}cargo test --profile ci -p lance-index --lib gate_released_range_preserves_unknown_for_negative_nansThe final assertion fails: expected None, observed Some((Float32(1), Float32(1))). This executes the released folding algorithm over the current writer’s loaded index, rather than a complete old-version binary.
| | `null_count` | UInt32 | false | Number of null values in the zone | | ||
| | `nan_count` | UInt32 | false | Number of NaN values (for float types) | | ||
| | `nan_count` | UInt32 | false | Number of NaN values of either sign (float types; 0 otherwise) | | ||
| | `negative_nan_count` | UInt32 | true | Number of the `nan_count` NaNs with the sign bit set (float types; 0 otherwise). Null when unknown | |
There was a problem hiding this comment.
This format proposal also contains the sign-aware reader, writer, and regression tests. The repository’s format proposal requirements explicitly require the specification plus only library edits needed to compile, with behavior implemented in follow-up PRs. That keeps the durable contract independently reviewable for the format vote.
Keep the statistics definition and evolution rules here, and move the sign-aware behavior and its tests into a follow-up based on the accepted contract. The conservative correctness fix is already separated in #9877.
lance-gatekeeper
Bot
added
the
K-changes
This branch has not been deployed
1 participant
ENT-2893. Stacked on #9877: its two commits come first here, and only the last commit is this change. This changes the on-disk statistics and needs a format vote.
#9877 fixes the lost rows by keeping every zone with a NaN for range queries, because the statistics do not say whether a zone's NaNs sort below or above the ordinary values. This PR records that, so such zones can be pruned by range again.
Statistics
Zone statistics gain
negative_nan_count, the number of thenan_countNaNs that carry the sign bit, so positive NaNs arenan_count - negative_nan_count. It is a nullable UInt32 column in both the index file and the write seed, written afternan_count. Null means unknown, not zero: every index and seed written before this column reads back that way. Non-float columns write 0. A present column of another type, or a count abovenan_count, is an error. Old zones are not rewritten; they merge into new indices as they are and stay unknown.min,maxandnan_countare written as in #9877:maxcarries the canonical+NaNas a marker when the zone holds a positive NaN (not the zone's largest NaN, since payloads order NaNs among themselves; it is the only way older readers learn the zone holds NaNs), and a zone whose NaNs are all negative keeps its ordinary maximum.docs/src/format/index/scalar/zonemap.mddocuments how nulls, ordinary values and the two NaN signs are accounted and which rules exist for backward compatibility.Pruning
A zone is kept if any group can match: nulls for
IS NULL; ordinary values when[min, max]intersects the range; negative NaNs when the range is open below or starts at a negative NaN; positive NaNs when it is open above or ends at a positive NaN. Unknown counts keep both signs possible, which is the #9877 behaviour. A NaN literal as a bound never rules out NaNs of its own sign, because same-sign NaNs still order by payload and the counts do not record payloads.DatasetStatistics::column_value_rangekeeps returningNonefor any NaN-bearing column.Tests
test_data/v14.0.0-beta.10/zonemap_signed_nanis a dataset written by the writer before this column (mainatfbec02448): three float columns whose zones each hold one NaN/null shape, a seeded ZoneMap index per column, and an appended fragment carrying old seeds. Its generator is checked in next to it.<,<=,>,>=,=,IN,BETWEEN,IS NULL,IS NOT NULL, with ordinary and NaN literals of both signs) returns the same rows with the index as without it, the old seeds harvest into an index that still marks those zones unknown, and a fragment written now merges into it with known counts.max, and every predicate again matches a scan before and after the seeds are harvested.lance-indexunit tests cover each NaN/null shape against every bound kind, including different NaN payloads, assert that no zone holding a matching row is pruned, and check the specific prune/keep decisions, seed round-trips (old seeds read back unknown) and the count validation.🤖 Generated with Claude Code