Skip to content

fix(index): error on a malformed precomputed partitions file - #9342

Open
LuciferYang wants to merge 2 commits into
lance-format:mainfrom
LuciferYang:fix/ivf-rq-precomputed-partitions-errors
Open

LuciferYang wants to merge 2 commits into
lance-format:mainfrom
LuciferYang:fix/ivf-rq-precomputed-partitions-errors

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

Closes #9341

load_precomputed_partitions pulled the row_id and partition columns out of a user-supplied file with .expect and an unchecked as_primitive, so a missing column or a wrong physical type ended the index build in a panic. The user picks that file: create_index(..., precomputed_partition_dataset=<uri>) checks only that the dataset has one fragment and one data file before the builder opens it and scans whatever schema it finds. Both lookups now go through as_primitive_opt and return invalid_input, naming the column, the type expected and what the file has instead — expected a UInt64 'row_id' column, found Int64, or no such column when it is absent.

Nulls get rejected too, and that one is not a panic but a wrong index. Both columns are read through their values buffer, which ignores validity, and the writer that produces these files marks the fields nullable, so a null partition was read as whatever the buffer held at that slot and the row went into an arbitrary partition with nothing reported.

The scope is that one function, which is the create_index path. The sibling loader in rust/lance/src/index/vector/ivf/builder.rs that transform_vectors(partition_ds_uri=...) uses has the same two .expects, and one line further it writes lookup[addr.fragment_id()][addr.row_offset()] with no bounds check, so a file whose row ids belong to a different dataset panics there on an index out of bounds. That needs the bounds check designed alongside the type checks, so it is left for a follow-up rather than half-fixed here.

How was this patch tested?

test_load_precomputed_partitions_rejects_wrong_row_id_type and ..._rejects_wrong_partition_type cover the wrong-type cases, ..._rejects_missing_partition_column covers the branch that replaced the .expect, and ..._rejects_nulls covers a null partition. Each asserts the error variant as well as the message. ..._merges_batches is the happy path over two batches, which also pins that the accumulator merges rather than replaces.

Both halves were checked by mutation: disabling the null guard fails ..._rejects_nulls, and restoring .expect with the unchecked as_primitive fails ..._rejects_wrong_row_id_type inside arrow's cast.rs.

load_precomputed_partitions extracted the row_id and partition columns
with .expect and unchecked downcasts, so a user-supplied file with a
missing column or the wrong physical type panicked the build mid-way.
Return descriptive input errors naming the expected columns and types.

Assisted-by: GLM-5.3
@github-actions github-actions Bot added A-index Vector index, linalg, tokenizer bug Something isn't working labels Sep 17, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate recommendation: approve.

The loader now rejects missing, mistyped, or nullable partition columns before their values enter index construction, while preserving valid multi-batch mappings. This addresses both the panic path and silent null-based misassignment without changing the well-formed file contract.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-index Vector index, linalg, tokenizer bug Something isn't working K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: a malformed precomputed partitions file panics the index build

1 participant