JunRuiLee commented on PR #516:
URL: https://github.com/apache/paimon-rust/pull/516#issuecomment-4956218123

   Thanks for the review, both points are addressed in the latest commit.
   
   **1. Inactive source files.** You're right — this was an interop gap. The PR 
originally mirrored the read kernel at `a50a36ff8`, which still rejected any 
ANN source that wasn't an active file; current Java master 
(`PrimaryKeyVectorBucketSearch` / `PkVectorAnnSegmentSearcher`) instead skips 
inactive sources and searches only the still-active portions. I've adopted the 
master behavior:
   
   - `build_live_row_ids` now takes the active-source set and masks out the 
ordinal ranges of inactive sources (they contribute no live ids; deletion 
vectors are applied only to active sources). It returns `None` only when all 
sources are active and no deletion vector is relevant.
   - `bucket_search` skips ANN sources missing from the active set; an active 
source with a mismatched row count is still a hard error.
   - `map_ann_results` additionally rejects a hit that resolves to an inactive 
source, mirroring the Java `checkArgument` (defensive — the live-row mask 
should already exclude it).
   - The active-source set is threaded through `PkVectorAnnSearcher::search` 
and `VindexAnnSearcher`.
   
   I replaced `test_rejects_ann_source_missing_or_mismatched_active_file` 
(which solidified the old reject) with a row-count-mismatch reject test plus 
skip-inactive coverage, and added tests for inactive ordinal-range masking and 
inactive-hit rejection.
   
   **2. Bucket Top-K complexity.** Replaced the `Vec` + `max_by` linear scan 
with a `BinaryHeap<WorstFirst>` keeping the BEST_FIRST-worst on top, so the 
merge is `O(candidate_count log K)`, matching the Java `PriorityQueue` and the 
existing `exact.rs` heap. Added a bounded-heap tie-break test (equal distances 
across files, eviction ordered by `(data_file_name, row_position)`).
   
   I've kept the larger current-master features (`GlobalIndexSearchMode` / FAST 
mode, the indexed/exact `Result` split) out of this PR to keep it scoped to 
these two comments; they'll come in a later PR.
   


-- 
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