[core][flink][spark][python][vector] Support vector indexes on DV tables - #8930
Conversation
ef31d32 to
5b03c02
Compare
5b03c02 to
5cb2e47
Compare
|
Thanks for working on this. I think this PR needs two changes before merge:
I consider both items part of this PR rather than follow-ups. The |
9ea4129 to
3b1a6ab
Compare
Allow deletion-vector tables to build vector global indexes whose readers can filter deleted row IDs. Opt in both Lumina and every paimon-vector native index factory while keeping other generic index types rejected.\n\nPin Spark and PyPaimon vector reads to the snapshot used for planning. Thread that snapshot through live-row filtering, scalar-index filtering, and raw fallback reads so materialized-deletion compaction cannot mix old index plans with new row-id or DV state.\n\nRefresh global-index deletions on every materialized-compaction commit attempt so indexes committed after preparation are still removed.
3b1a6ab to
6363ee8
Compare
Thanks for the detailed review. Both requested changes are addressed in 6363ee8: |
| /** | ||
| * Whether the read path filters DV-deleted rows, so building over all physical rows is safe. | ||
| */ | ||
| default boolean supportsDeletionVectors() { |
There was a problem hiding this comment.
Remove this, we have support all vector index.
JingsongLi
left a comment
There was a problem hiding this comment.
I found one PyPaimon snapshot-lifecycle correctness issue that should be addressed before merge.
|
|
||
| def read_plan(self, plan): | ||
| # type: (VectorSearchScanPlan) -> GlobalIndexResult | ||
| self._plan_snapshot = plan.snapshot() |
There was a problem hiding this comment.
Storing the plan snapshot on the reader makes the result depend on previous calls. read() remains public and does not clear _plan_snapshot, so reusing a reader after read_plan(plan_at_S1) and then calling read(splits_planned_at_S2) makes the S2 live-row, scalar-index, and raw-fallback paths read from S1. The mutable field is also unsafe for concurrent reads, and the batch path has the same issue. After DV materialization this can return deleted rows or omit valid top-K results. Could we keep the reader stateless and pass the snapshot explicitly for each invocation (for example, _read(splits, snapshot), with read_plan forwarding plan.snapshot()), instead of caching it on self?
JingsongLi
left a comment
There was a problem hiding this comment.
The latest update exposes one additional DV snapshot-consistency issue outside the vector readers.
| + "deleted rows to be indexed.", | ||
| table.name()); | ||
|
|
||
| scanSnapshot = table.snapshotManager().latestSnapshot(); |
There was a problem hiding this comment.
Removing the DV guard here enables every generic index type, including full-text and es-index, but those read paths are not snapshot-consistent yet. FullTextRead.read(Plan) discards the plan context, and DataEvolutionFullTextRead computes live rows from the latest snapshot. If materialized-DV compaction commits between planning at S1 and reading at S2, the S1 index row IDs are intersected with S2 live rows, which can produce missing or empty top-K results. Please keep non-vector indexes gated (or restore an explicit vector-capability check) until full-text plans carry their snapshot and both live-row and raw fallback reads are pinned to it. A plan -> materialized compaction -> read regression test would cover this race.
There was a problem hiding this comment.
Let‘s add support to full-text too.
|
+1 |
Purpose
Allow building vector global indexes on deletion-vector (DV) enabled tables. Generic global-index building previously rejected every index type on DV tables, even though the vector read paths can filter DV-deleted rows and safely build over the physical-row superset.
Changes
GlobalIndexerFactory.supportsDeletionVectors()capability, defaulting tofalse.paimon-vectornative index types:ivf-flat,ivf-pq,ivf-sq,ivf-rq, anddiskann.Related merged fixes
This PR completes snapshot pinning for Spark and PyPaimon and closes the remaining preparation-to-commit race.
Tests