feat(index): support zone maps for all data types - #8190
Conversation
|
Important This PR touches the Lance format specification. Substantive changes to the format specification — the If this is a meaningful format change:
|
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The ordered/null-only split is a sound direction, but this revision can produce false-negative pruning in both mixed-version reads and dictionary-float ranges.
A viable revision would version the null-only layout so older readers fall back to scanning while new readers retain version-0 support, and preserve NaN as the dictionary zone maximum (or conservatively include NaN-bearing dictionary zones).
| // Whether this index stores ordered minimum and maximum values. Absent means | ||
| // true for compatibility with version-0 zone maps, which only supported | ||
| // orderable scalar fields. False identifies a null-only zone map. | ||
| optional bool supports_min_max = 3; |
There was a problem hiding this comment.
This changes the meaning of a version-0 zonemap in a way older readers cannot interpret. Their plugin also reports version 0, so retain_supported_indices keeps the index; their generic parser routes list comparisons; and typed-null extrema make a matching zone look empty. Give null-only zonemaps a new index version (and bump the current plugin's supported version) so older readers ignore them and scan. New readers can continue loading version-0 ordered files.
Reproducer run on this head
I added this temporary regression to the existing zonemap test module; it simulates the version-0 reader's absent-field default and asserts the required conservative result:
#[tokio::test]
async fn test_legacy_v0_mode_keeps_matching_fixed_size_list_zone() {
let item = Arc::new(Field::new("item", DataType::Int32, false));
let lists = FixedSizeListArray::new(
item,
2,
Arc::new(Int32Array::from(vec![1, 2, 3, 4])),
None,
);
let target = ScalarValue::try_from_array(&lists, 0).unwrap();
let mut index = train_and_load_array(Arc::new(lists)).await;
Arc::get_mut(&mut index).unwrap().supports_min_max = true;
let legacy_details = prost_types::Any::from_msg(&pbold::ZoneMapIndexDetails {
rows_per_zone: Some(2),
use_seeds: Some(false),
supports_min_max: None,
}).unwrap();
let parser = ZoneMapIndexPlugin
.new_query_parser("lists_idx".to_string(), &legacy_details)
.unwrap();
assert!(parser
.visit_comparison(VALUE_COLUMN_NAME, &target, &Operator::Eq)
.is_some());
assert_eq!(
index.search(&SargableQuery::Equals(target), &NoOpMetricsCollector)
.await.unwrap(),
SearchResult::at_most(0..=1),
);
}cargo test -p lance-index test_legacy_v0_mode_keeps_matching_fixed_size_list_zone -- --nocapture failed with AtMost(empty) instead of AtMost(0..=1).
| | DataType::Decimal256(_, _) => true, | ||
| DataType::Time32(TimeUnit::Second | TimeUnit::Millisecond) => true, | ||
| DataType::Time64(TimeUnit::Microsecond | TimeUnit::Nanosecond) => true, | ||
| DataType::Dictionary(_, value_type) => supports_ordered_extrema(value_type), |
There was a problem hiding this comment.
Dictionary floats are classified as ordered and their NaNs are counted, but ZoneMapProcessor::nan_scalar only promotes primitive float maxima to NaN. A dictionary zone therefore keeps a finite maximum and can be dropped by a high lower bound even though its NaN value compares above that bound. Wrap the NaN maximum in the dictionary scalar type, or conservatively retain NaN-bearing dictionary zones for affected ranges.
Reproducer run on this head
I added this temporary regression to the existing zonemap test module:
#[tokio::test]
async fn test_dictionary_float_range_keeps_nan_zone() {
let values: ArrayRef = Arc::new(DictionaryArray::<Int8Type>::try_new(
Int8Array::from(vec![0, 1]),
Arc::new(Float32Array::from(vec![1.0, f32::NAN])),
).unwrap());
let index = train_and_load_array(values).await;
let target = ScalarValue::Dictionary(
Box::new(DataType::Int8),
Box::new(ScalarValue::Float32(Some(1000.0))),
);
let nan = ScalarValue::Dictionary(
Box::new(DataType::Int8),
Box::new(ScalarValue::Float32(Some(f32::NAN))),
);
assert!(nan > target);
assert_eq!(
index.search(
&SargableQuery::Range(Bound::Excluded(target), Bound::Unbounded),
&NoOpMetricsCollector,
).await.unwrap(),
SearchResult::at_most(0..=1),
);
}cargo test -p lance-index test_dictionary_float_range_keeps_nan_zone -- --nocapture failed with AtMost(empty) instead of AtMost(0..=1).
There was a problem hiding this comment.
✅ Gate recommendation: approve.
This revision resolves both prior correctness blockers: null-only zone maps now use a versioned layout that older readers ignore while preserving ordered version-0 compatibility, and dictionary-float NaN bounds remain conservative. The persisted mode and exact-null completeness are also preserved through load, update, seed, and merge paths.
Summary
Closes #7987
Test Plan
cargo fmt --allcargo check --workspace --tests --benchescargo clippy --all --tests --benches -- -D warningscargo test -p lance-arrow-stats(83 passed; 3 doctests passed)cargo test -p lance-index --no-fail-fast(989 passed, 2 ignored)cargo test -p lance-index --doc(7 passed)cargo test -p lance test_dataset_null_only_zonemap_for_list_column --no-fail-fastcargo test -p lance test_released_zonemap_fixture_preserves_ordered_behavior --no-fail-fastcd python && uv run make lint