Skip to content

fix(lumina): reproduce Java's table, index, query option order - #908

Merged
JingsongLi merged 1 commit into
apache:mainfrom
jackylee-ch:fix/lumina-searcher-option-merge-order
Sep 22, 2026
Merged

JingsongLi merged 1 commit into
apache:mainfrom
jackylee-ch:fix/lumina-searcher-option-merge-order

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

ensure_loaded started from index_meta.options() and then wrote the table's stripped lumina.* options over it, so a table option beat the value the index was built with.

The build side rewrites some of these keys deliberately: validate_and_cap_pq_m caps encoding.pq.m at the column's dimension and persists the capped value. So 'lumina.encoding.pq.m' = '64' on a 32-dimension column built the index with m=32 and opened the searcher with m=64.

Java's order is table, then indexMeta.options(), then per-query (LuminaVectorGlobalIndexReader:377-378, :253-255). Rust had no query layer left at the reader — both read paths fold the per-query map into the table options first — so inverting the merge alone would demote per-query options too. search_options_for_query puts them back on top.

A table-level diskann.search.beam_width no longer wins: build_lumina_options records it in the metadata, as Java's writer does. A query option still overrides it.

`ensure_loaded` started from `index_meta.options()` and then wrote the table's
stripped `lumina.*` options over it, so a table option beat the value the index
was actually built with.

The build side rewrites some of these keys on purpose: `validate_and_cap_pq_m`
caps `encoding.pq.m` at the column's dimension and the capped value is what is
persisted in the index metadata. With the merge inverted, `'lumina.encoding.pq.m'
= '64'` on a 32-dimension column built the index with m=32 and then opened the
searcher with m=64. The same inversion let a stale `lumina.distance.metric`
reconfigure the searcher while scores kept being converted with the index's
metric, so results were ranked by one metric and scored by another.

Java merges table options first, then `indexMeta.options()`, then the per-query
options -- `LuminaVectorGlobalIndexReader:377-378` for the searcher and
`:253-255` for each search. Rust had no query layer left at the reader: both read
paths fold the per-query map into the table options before constructing it
(`de_vector_read`, `pk_vector_read`), so simply inverting the merge would have
demoted per-query options too. `search_options_for_query` puts them back on top
at each search, which is also the only layer that can still tune
`diskann.search.beam_width` and `search.parallel_number`: `build_lumina_options`
seeds every `ALL_OPTIONS_DEFAULTS` key into the metadata, so a Rust-built index
always records their build-time values and a table option no longer overrides
them -- as in Java, whose writer persists the same map.

Divergence reaches the reader through the build procedure's `options =>`
argument, which is applied over the table options, or through an `ALTER` after
the index is built.

@JingsongLi JingsongLi 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.

Reviewed head 30530ad. Requirement fit: supported; implementation: no blocking findings in the option-precedence change. The existing Rust reader did overlay table/query options on index_meta.options() at searcher construction, so a build-time-capped encoding.pq.m could be replaced by the raw configured value. The new base merge keeps persisted index geometry authoritative, while per-query options are applied for both single and batch searches. I checked the official Java LuminaVectorGlobalIndexReader: its ensureLoaded merges table then index metadata, and buildSearchOptions layers query options last. Locally both new lumina::reader precedence tests passed. One compatibility nuance is now explicit: changing a table-level search option after index creation no longer overrides the value persisted in a Rust-built index; use a per-query option to tune a current search.

@JingsongLi
JingsongLi merged commit 5013b01 into apache:main Sep 22, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants