JingsongLi commented on code in PR #760:
URL: https://github.com/apache/paimon-rust/pull/760#discussion_r3893361063
##########
crates/paimon/src/table/vector_search_builder.rs:
##########
@@ -1194,14 +1216,32 @@ impl<'a> BatchVectorSearchBuilder<'a> {
self
}
- /// Attach a residual scalar predicate applied *after* vector recall on the
- /// primary-key vector path, shared across every query in the batch.
Mirrors
- /// the single [`VectorSearchBuilder::with_filter`]: only the primary-key
- /// vector path (via [`execute_read`](Self::execute_read)) consumes it,
and only
- /// when the table exposes physical rows directly (deletion vectors without
- /// merge-on-read); otherwise the query fails loud.
+ /// Attach one scalar predicate shared by every query in the batch and
applied
+ /// before vector Top-K. See [`VectorSearchBuilder::with_filter`] for the
+ /// primary-key and data-evolution execution semantics.
pub fn with_filter(&mut self, filter: Predicate) -> &mut Self {
self.filter = Some(filter);
+ self.include_row_ids = None;
+ self
+ }
+
+ /// Reuse row IDs from a previously prepared scalar pre-filter.
+ ///
+ /// The builder's table must be [`PreparedVectorSearchFilter::table`] (or
an
+ /// equivalent copy pinned to the same snapshot).
+ pub fn with_include_row_ids(&mut self, include_row_ids: RoaringTreemap) ->
&mut Self {
+ self.include_row_ids = Some(Arc::new(include_row_ids));
+ self.filter = None;
+ self
+ }
+
+ /// Reuse a shared row-ID allow-list without copying its bitmap.
+ pub fn with_shared_include_row_ids(
Review Comment:
[P1] Please keep the prepared allow-list and its pinned table snapshot
atomic in the public API. Today a caller can prepare at S1, let the live table
advance to S2, then create this builder from the original table and pass
`Arc::clone(prepared.include_row_ids())` here. `execute` independently resolves
S2, so the S1 bitmap silently excludes newly matching/updated S2 rows and
returns an incorrect Top-K. The internal DataFusion callers pair
`prepared.table()` correctly, but this zero-copy public setter has no way to
enforce that contract. Could the builder accept `PreparedVectorSearchFilter`
itself and bind its table, or carry/validate the snapshot ID with the bitmap
(leaving raw bitmap setters as an explicitly lower-level API)?
--
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]
