[core] Support batch primary-key vector search - #8666
Conversation
leaves12138
left a comment
There was a problem hiding this comment.
Blocking issue: routing configured primary-key vector columns through PrimaryKeyBatchVectorRead breaks Spark lateral vector search. The new reader returns PrimaryKeyVectorResult, which is addressed by physical file positions, but LateralVectorSearchExec still treats every batch result as a global row-ID result. Both createRowIdToMatches and createBatchGlobalIndexResult call result.results() (PaimonStrategy.scala:527 and :541), while PrimaryKeyScoredResult.results() intentionally throws UnsupportedOperationException. The metadata-only path has the same assumption at PaimonStrategy.scala:485.
I reproduced this with a primary-key table configured with pk-vector.index.columns and a LATERAL (SELECT ... FROM vector_search(..., q.embedding, 1)) query. It fails with Primary-key scored results use physical file positions, not global row ids.
Please either add physical-position-aware batch consumption in Spark (including per-query score association and combined physical splits) or prevent this routing for Spark until that support exists. A Spark regression test should cover both regular projected rows and metadata-only output. The focused Core suite itself passes 60/60.
|
I reproduced a blocking Spark regression introduced by the new primary-key batch route. For a primary-key table configured with SELECT q.id, r.id
FROM T AS q,
LATERAL (
SELECT id
FROM vector_search('T', 'embedding', q.embedding, 1)
) AS r
Please add physical-position-aware Spark batch consumption (including per-query scores and combined physical splits), or avoid routing this Spark path to the primary-key batch reader until it is supported. Please also add Spark regression coverage for both regular row projections and metadata-only projections. The focused Core suite passes 60/60; the issue is specifically at the Spark batch-result consumption boundary. |
05e7005 to
a98f2c8
Compare
leaves12138
left a comment
There was a problem hiding this comment.
Re-reviewed after the rebase. The previous blocking issue is resolved: the Spark lateral path now handles physical-position batch results without calling results(), while preserving per-query scores and snapshot-consistent physical splits.
I also ran the focused core vector test suites (28 tests, all passed). PrimaryKeyVectorSearchTest still has one failure in distributed primary-key vector search applies residual filter before top k, but I reproduced the identical failure on base commit 5f700f4affa01b122187b41b537cc76855d4d113 (with only the base's one-line async return compilation fix), so it is not introduced by this PR.
LGTM.
What changed
Why
BatchVectorSearchBuilderImpl previously always selected the data-evolution search path. As a result, batch callers could not use bucket-local primary-key vector indexes even though single-query search supported them.
Impact
Configured primary-key vector columns now support native batch search without changing the existing data-evolution path or public APIs.
Validation