JunRuiLee opened a new pull request, #757:
URL: https://github.com/apache/paimon-rust/pull/757

   Third step of #755: give planning a second entry point, so a primary-key 
vector search can run over bucket splits an engine planned elsewhere instead of 
only over a plan read from this table's index manifest.
   
   **Stacked on #752** — its commit is included here, so the diff shown against 
`main` contains it until that PR merges. Draft for that reason; only the last 
two commits are this PR's own work (`+1036/-13` over #752).
   
   Motivation, the byte format, and how the four steps fit together are in #755.
   
   ## What changes
   
   **`refactor(table): separate PK-vector planning from searching`** — 
behavior-preserving.
   
   `plan_and_search_pk_candidates_batch` resolved the query parameters, read 
the manifest into a plan, and searched that plan in one body, so a caller 
already holding a plan could not reuse the search path. It becomes three pieces:
   
   - `resolve_pk_vector_search_params` — the pre-filter guard and everything 
resolvable from the schema, the options and the queries, before planning.
   - `search_pk_raw_candidates_batch_with_plan` — search a supplied plan, 
returning 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 is never 
searched under limits computed for a wider one.
   - `search_pk_candidates_batch_with_plan` — the raw layer plus the optional 
exact rerank and the merge into one best-first list per query.
   
   The old entry point 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. The three moved blocks (97, 252 and 28 lines) are 
byte-identical, so the commit reads as a move plus new signatures.
   
   **`feat(table): plan a PK-vector search from a decoded bucket split`** — the 
new behavior.
   
   `PkVectorScan::plan_for_bucket_vector_splits` builds a plan from 
`BucketVectorSearchSplit`s. They are authoritative: their payload files, their 
per-file row ranges and the snapshot they pin are used as given and no manifest 
is read. 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. Payload resolution, bucket grouping, current-segment selection and 
exact-fallback eligibility all reuse the manifest route's `plan_from_inputs`, 
so both routes resolve index paths and choose segments identically.
   
   Four inputs are rejected rather than planned around:
   
   - No splits at all — pins no snapshot to report, and a 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 — would search its rows twice. Java emits one 
split per bucket; independently decoded buffers cannot enforce that between 
them.
   - A nested data split carrying its own row ranges — a second authority over 
which physical rows are readable, free to disagree with the per-file ranges the 
bucket form carries. Java's planner builds the nested split without them.
   
   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 must 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 becomes 
an explicit full-file range. An explicitly empty list stays empty and excludes 
its file.
   
   A payload's `deletion_vectors_ranges` is ignored deliberately. Java reserves 
that field for deletion-vector index files, builds a vector payload 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. 
Ignoring matches Java's read semantics; rejecting would be stricter than Java's 
own split reader.
   
   ## Tests
   
   16 new cases. Planning from the Java golden fixture is covered end to end — 
the payload's external 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. The rest cover each rejection, the 
unlisted-versus-empty row-range distinction, payloads for another column or 
index type, snapshot mismatch outranking pruning, an all-pruned plan keeping 
its snapshot, both directory layouts, and the allow-list intersection.
   
   The `Table`-independent core is a free function (`plan_from_bucket_splits`) 
for the same reason `plan_from_inputs` is: it makes planning testable without 
standing up a table, which is how this file already tests the manifest route.
   
   ## Notes for review
   
   - **`plan_for_bucket_vector_splits` has no in-tree caller yet** — the C 
entry point that hands it engine-supplied splits is the next step. It carries 
`#[allow(dead_code)]` with a comment, following the existing convention in this 
crate; the tests drive the free function directly. Happy to fold the caller in 
here instead if you would rather not merge an uncalled entry point.
   - **An ANN payload's own `row_count` is still unused**, here and on the 
manifest route: `IndexFileMeta.row_count` is read but never carried into 
`BucketAnnSegment`, so a zero-row payload is not short-circuited before its 
index file is opened. That is pre-existing and affects both routes, so it is 
left alone rather than changed for one of them.
   - **`deserialize_binary_array_str` is still unhardened.** It validates only 
that each variable-length region is in bounds, so elements may overlap and `n` 
of them can each clone the same body (`~len²/8`). It is reachable from 
`DataFileMeta` row decoding and therefore from a bucket split's nested 
`DataSplit`. Not reachable from untrusted input by anything in this PR, but it 
should be tightened the way #746 tightened the row-array variant before a C 
entry point starts accepting arbitrary split bytes.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to