Skip to content

[lumina] Validate lumina.encoding.pq.m is positive - #9823

Open
jackylee-ch wants to merge 1 commit into
apache:masterfrom
jackylee-ch:lumina-pqm-positive
Open

jackylee-ch wants to merge 1 commit into
apache:masterfrom
jackylee-ch:lumina-pqm-positive

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

Purpose

capPqM's Javadoc requires pq.m to be > 0 && <= dimension; the body checked only the upper bound, so lumina.encoding.pq.m = 0 reached the native trainer instead of failing here. Javadoc and clamp both arrived in 410e1267b (#7330), so the two bounds were never deliberately different — the upper one keeps clamping because tightening it would break tables that build today.

Open question

If the native trainer reads pq.m = 0 as "auto", such indexes exist, and since toLuminaOptions() is also on the read path (LuminaVectorGlobalIndexReader:253, :377) this would fail their searches too. lumina-jni is external; can anyone confirm?

Tests

LuminaVectorOptionsTest#testEncodingPqMBounds (capPqM had no coverage). Reverting the fix:

Expecting code to raise a throwable.

Written with Claude Code; verification is mine.

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.

1 participant