docs: drop the dead rows_per_range create index option - #835
Open
jackylee-ch wants to merge 1 commit into
Open
jackylee-ch wants to merge 1 commit into
jackylee-ch wants to merge 1 commit into
Conversation
rows_per_range is documented as a Long with "Default is 1000000", but nothing reads it. In main it appears only in AddIndexExec's SparkOnlyOptions strip set and that class's javadoc, so it never reaches the Lance index backend either; IndexUtilsTest asserts it is stripped from the JSON params. The range build takes its partition count from the fragment count instead. Setting it has no effect and no warning. The strip set is left alone so existing statements that pass it keep working.
github-actions
Bot
added
the
documentation
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The Lance 8 migration in #612 replaced global value ranges with disjoint fragment-covered segments. This docs-only correction now matches the live fragment-ID partitioning contract while leaving existing SQL that passes the ignored rows_per_range option compatible.
lance-gatekeeper
Bot
added
the
K-approved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
rows_per_rangeis documented as aLongwith "Default is 1000000", but nothing reads it. Inmainit appears only inAddIndexExec'sSparkOnlyOptionsstrip set (:801) and that class's javadoc (:70), so it never reaches the Lance index backend either —IndexUtilsTestasserts it is stripped from the JSON params. The range build takes its partition count from the fragment count (Math.max(1, numFragments)), so setting the option has no effect and no warning.Contrast within the same command:
zone_sizeis read atAddIndexExec.scala:333, androws_per_zoneis absent from the strip set so it passes through to Lance. Onlyrows_per_rangeis inert.Replaces the row with a sentence on what actually determines the range partitioning. The strip set is left alone so statements that already pass the option keep working.