feat(table): plan a PK-vector search from a decoded bucket split - #757
Open
JunRuiLee wants to merge 4 commits into
Open
feat(table): plan a PK-vector search from a decoded bucket split#757JunRuiLee wants to merge 4 commits into
JunRuiLee wants to merge 4 commits into
Conversation
JunRuiLee
force-pushed
the
feat/pk-vector-plan-from-bucket-split
branch
from
August 28, 2026 07:47
2ef30b3 to
aaac4b9
Compare
`plan_and_search_pk_candidates_batch` resolved the query parameters, read the index manifest into a plan, and searched that plan in one body, so a caller that already holds a plan could not reuse the search path. Split it into three pieces with no behavior change: - `resolve_pk_vector_search_params` — the query-level parameters and the pre-filter guard: everything resolvable from the schema, the options and the queries alone, before planning. - `search_pk_raw_candidates_batch_with_plan` — search a supplied plan and return each query's raw indexed and exact candidate lists. Plan-dependent concurrency (segment count, batch-index parallelism, range-read bound) is derived from the plan actually being searched, so a narrowed plan can never be searched under limits computed for a wider one. - `search_pk_candidates_batch_with_plan` — the raw layer plus the optional exact rerank of the approximate candidates and the merge into one best-first list per query. `plan_and_search_pk_candidates_batch` keeps its signature and becomes a wrapper over the three. The empty-plan short circuit moves into the raw layer, still ahead of backend resolution, so a table with no searchable data does not error on an unrecognized index type.
A `BucketVectorSearchSplit` already carries everything a search needs for one bucket: the payload files, the rows each data file allows, and the snapshot the whole plan is pinned to. Planning could only be driven the other way round, by reading this table's index manifest, so a search could not be run over splits an engine planned elsewhere. `PkVectorScan::plan_for_bucket_vector_splits` builds a plan from such splits instead. The splits are authoritative -- no manifest is read -- and only the partition conjuncts of the scan's filter are re-applied, since a caller may narrow the query further than the planner that produced the splits. Bucket grouping, current-segment selection and exact-fallback eligibility reuse the manifest route's `plan_from_inputs`, so both routes pick segments the same way. A payload's index file is resolved where Java put it: its `_EXTERNAL_PATH` when it records one, and otherwise the bucket directory the split serialized when the table sets `index-file-in-data-file-dir`, or the table `index/` directory. Java records an external path only for an index stored outside the table -- `PkVectorAnnSegmentFile` writes `null` unless its path factory is external -- so an ordinary bucket-local payload carries none, and looking for it under `index/` would not find it. The manifest route keeps its existing `index/` assumption; the option is read only for splits an engine supplied. Four inputs are rejected rather than planned around: - No splits at all, which pins no snapshot to report, and the plan's snapshot id has to stay authoritative even when nothing is searchable. - Splits pinning different snapshots, checked before partition pruning so an inconsistent input cannot hide behind an empty plan. - Two splits for one bucket, which would search its rows twice. Java emits one split per bucket, but independently decoded buffers cannot enforce that. - A nested data split carrying its own row ranges, which would be a second authority over which physical rows are readable, free to disagree with the per-file ranges the bucket form carries. Row ranges become a per-split allow-list of physical positions on the plan, and the search intersects it with the residual predicate's allow-list: both sides list what is permitted, so a position needs to survive both. The normalization is where the two formats disagree -- Java records ranges only for the files its own pre-filter narrowed and omits the rest, while the search kernel reads a missing entry as "no rows allowed" -- so an omitted file is turned into an explicit full-file range. An empty list stays empty and excludes its file. A payload's `deletion_vectors_ranges` is ignored on purpose. Java reserves that field for deletion-vector index files, builds vector payloads through the overload that leaves it null, and takes a read's deletion vectors from the bucket's data split, so a value there describes something the payload is not. Planning from the Java golden fixture is covered end to end: the external payload path wins over both directory layouts, the five-billion-byte size survives, and a six-row file listed as rows 0-1 and 4-5 plans to exactly those positions.
A producer that restricts the readable rows of only some data files leaves the rest unrestricted, and an adapter has to say so explicitly, as an allow-list covering the whole file. Building live row ids then walked that list one position at a time, costing an insert per row of the file, where the same statement made by omitting the residual entirely takes a single range insert. Recognize the whole-file shape and insert one range instead. A list whose length equals the file's row count and whose maximum is the last position can only be the full set, so the check also subsumes the per-position bound check it replaces. Deletion vectors still apply: the shortcut only replaces how positions enter the live set, not what happens to them afterwards.
- `plan_for_bucket_vector_splits` carried its whole doc block twice. - `into_parts` claimed the index metadata moves rather than being cloned, which `plan_from_inputs` does not honor: it clones `_INDEX_META` out of the `GlobalIndexMeta` it is handed. Narrowed the claim to what the method itself does. - `manifest_planning_leaves_positions_unrestricted` could not observe what its name claims: it drives `plan_from_inputs`, which returns splits and not a `PkVectorScanPlan`, so `physical_row_ranges_by_split` is out of its reach and the only assertion left was a split count that `builds_one_split_per_bucket_with_data` already makes.
JunRuiLee
force-pushed
the
feat/pk-vector-plan-from-bucket-split
branch
from
August 31, 2026 09:57
aaac4b9 to
90433b9
Compare
JunRuiLee
marked this pull request as ready for review
August 31, 2026 09:59
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.
Third step of #755: run a primary-key vector search over bucket splits an engine planned elsewhere, not only over a plan read from this table's index manifest.
Independent of #752 — the diff is against
mainalone.PkVectorScan::plan_for_bucket_vector_splitstakes the splits as authoritative: their payloads, row ranges and pinned snapshot are used as given, no manifest is read, and only the partition conjuncts are re-applied, since a caller may narrow further than the planner that produced them. Bucket grouping and segment selection reuse the manifest route.Three decisions worth checking:
_EXTERNAL_PATHwins; otherwise the bucket directory the split serialized whenindex-file-in-data-file-diris set, else<table>/index. Java records an external path only for an index stored outside the table —PkVectorAnnSegmentFilewritesnullotherwise — so an ordinary bucket-local payload carries none. This duplicates only the small path decision fix(table): resolve index files by external path and bucket layout #752 centralizes; whichever lands second should consolidate this call site, and the option accessor added here is byte-identical to fix(table): resolve index files by external path and bucket layout #752's.A payload's
deletion_vectors_rangesis ignored: Java reserves that field for deletion-vector index files and takes a read's deletion vectors from the data split.Deliberately not here:
plan_for_bucket_vector_splitshas no in-tree caller yet — the C entry point is the next step, and it carries#[allow(dead_code)]meanwhile. Happy to fold the caller in if you would rather not merge an uncalled entry point.