Skip to content

fix(index): validate num_pages when loading RTree metadata - #8988

Open
jackylee-ch wants to merge 1 commit into
lance-format:mainfrom
jackylee-ch:fix/rtree-num-pages-validation
Open

fix(index): validate num_pages when loading RTree metadata#8988
jackylee-ch wants to merge 1 commit into
lance-format:mainfrom
jackylee-ch:fix/rtree-num-pages-validation

Conversation

@jackylee-ch

@jackylee-ch jackylee-ch commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

num_pages is read back from index metadata and load validates page_size but never num_pages. A search walks down from num_pages - 1, so a value that disagrees with num_items and page_size does not fail — it answers the wrong rows:

200-item index, page_size 16, correct num_pages = 14

num_pages = "" / "abc" / "-1" / "0" / "9999..."  ->  0 rows
num_pages = "13"                                 ->  8 rows
intact                                           ->  200 rows

Validation runs on the raw metadata, before RTreeMetadata is constructed: its constructor already debug_assert!s the same equality, so validating afterwards would panic in debug builds instead of returning the error. An absent key is derived from num_items and page_size — the format spec computes every page offset from those two — so a conforming index that omits it still loads.

@github-actions github-actions Bot added A-index Vector index, linalg, tokenizer bug Something isn't working labels Sep 4, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-changes Latest Gatekeeper recommendation requests changes. label Sep 4, 2026
num_pages is read back from index metadata and load validates page_size
but never num_pages. A search walks down from num_pages - 1, so a value
that disagrees with num_items and page_size does not fail: zero
underflows and every page range comes back empty, while a low value
truncates the walk. A 200-item index answered 0 rows for an unreadable
value and 8 rows for an off-by-one one.

Validate the raw metadata before constructing RTreeMetadata, because its
constructor already debug_asserts the same equality and would panic
before the error could surface. An absent key is derived from num_items
and page_size, which is how the format spec computes every page offset,
so a conforming index that omits it still loads.
@jackylee-ch
jackylee-ch force-pushed the fix/rtree-num-pages-validation branch from 1b71b21 to e13be63 Compare September 4, 2026 10:32
@lance-gatekeeper lance-gatekeeper Bot removed the K-changes Latest Gatekeeper recommendation requests changes. label Sep 4, 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.

Both prior metadata-loading findings are fixed. The loader now validates a present stored page count before constructing the asserted representation, derives the count when the optional key is absent, and the focused load-path tests cover corrupt, absent, consistent, and degenerate metadata.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 4, 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.

1 participant