Skip to content

[core] Fix lookup returning rows deleted by deletion vectors - #10813

Open
Stephen0421 wants to merge 1 commit into
apache:masterfrom
Stephen0421:fix-lookup-deletion-vector
Open

Stephen0421 wants to merge 1 commit into
apache:masterfrom
Stephen0421:fix-lookup-deletion-vector

Conversation

@Stephen0421

Copy link
Copy Markdown
Contributor

Purpose

LocalTableQuery builds lookup files without applying deletion vectors, and checks only the row kind of the cached record. After compaction drops the delete record and marks the old position with a deletion vector, Flink partial lookup and the query service return the deleted row.

Deletion-vector tables that use the deduplicate merge engine and have no sequence field now look up from level 0, so an uncompacted delete is visible. The cached entry stores the row position. If that position is contained in the bucket's current deletion vector, lookup returns null and does not continue into older levels. The vector is not applied while building the lookup file, because dropping the key there would fall through to a stale value.

Other merge engines and tables with a sequence field keep their previous start level. A level-0 record is not yet merged, so returning it would skip partial-update, aggregation, and sequence-field behavior. Those tables still honor deletion vectors on the files they do read.

Each refreshFiles reloads the bucket deletion-vector index from the latest snapshot. Streaming plans do not carry deletion files, and compaction can mark a file that is absent from the current split. The parsed vectors are reused while the index files are unchanged, and the cached file list is recorded only after the read succeeds. Deletion-vector tables do not use remote lookup files, because those files do not store the row position.

Tests

  • LocalTableQueryDeletionVectorTest
    • An uncompacted level-0 delete is not returned, and a later update of that key is returned.
    • After compaction that only writes a deletion vector, the deleted key stays invisible while another key in the same file remains visible.
    • A partial-update level-0 record does not replace untouched columns with null.
    • An older sequence-field value does not overwrite the compacted value.
  • PrimaryKeySimpleTableTest#testTableQueryForNormal and #testTableQueryForLookup for the existing non-deletion-vector lookup path.
  • mvn -pl paimon-core spotless:check


private List<IndexManifestEntry> currentDeletionVectorEntries() {
Snapshot latest = snapshotManager.latestSnapshot();
long snapshotId = latest == null ? -1L : latest.id();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Keep deletion vectors aligned with the streamed file update

Loading deletion vectors from the latest table snapshot can make them newer than the beforeFiles / dataFiles being applied. If snapshot N adds a deletion vector for an old data file and snapshot N+1 rewrites that file and removes the vector, a lagging query operator processing N reads N+1's now-empty vector state while its lookup levels still contain the old file. The deleted row is then returned until the N+1 file update arrives. This is observable in QueryExecutorOperator, which applies updates individually while KvQueryServer serves concurrent requests. Please propagate the update's snapshot/index state with the file event, or otherwise update files and deletion vectors from the same snapshot.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. refreshFiles now takes the snapshot id of the file event, and deletion vectors are loaded from that snapshot instead of latestSnapshot().

QueryExecutorOperator reads the snapshot id from field 0 of the FileMonitorTable row and passes it through. A lagging executor applying snapshot N therefore still sees N's deletion vectors, even if N+1 has already rewritten the file and dropped the vector. The deleted row is not returned.

If that snapshot has expired, refresh throws OutOfRangeException instead of serving the files with no deletion vectors. FileStoreLookupFunction already catches this and reopens from a live snapshot.

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deleted-row lookup problem has clear end-to-end value. The 3 new DV tests and 2 existing non-DV lookup tests pass locally with normal build checks. An additional real-table regression exposes a newly introduced snapshot mismatch: PR head returns null, while its parent returns the expected S1 value for the same held file plan.

}

private List<IndexManifestEntry> currentDeletionVectorEntries() {
Snapshot latest = snapshotManager.latestSnapshot();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Pair deletion vectors with the snapshot of the supplied files

latestSnapshot() can be newer than the files being installed. I reproduced this using real writes/compaction: at S1, compact 20 keys including (1, 10) and retain its planned data files; write (1, 99) and partially compact S2; then call refreshFiles with the held S1 files. This code reads S2's vector, masks key 1's position in S1, and lookup returns null (key 2 remains visible). The same test against the parent implementation returns 10. Key 1 exists in both S1 and S2, so this is neither a valid stale value nor the current value.

This interleaving is reachable when QueryFileMonitor has planned/enqueued an S1 event and S2 commits before QueryExecutorOperator processes it; the query server continues serving lookups between events. A miss can silently drop a lookup-join match. Please carry the event/split snapshot ID (or matching DV metadata) through refreshFiles and load the corresponding deletion vectors, so file levels and visibility are installed from the same snapshot. Add a regression with a held file plan and an intervening update/compaction.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The event/split snapshot id is now passed into refreshFiles, and deletion vectors are loaded from that snapshot together with the files.

testDeletionVectorUsesTheFileSnapshot holds the S1 file plan, writes (1, 99), partially compacts S2, then refreshes with the held S1 files. Key 1 stays 10 and key 2 stays 20. The S2 vector is no longer applied to the S1 file.

PrimaryKeyPartialLookupTable passes split.snapshotId() for both DataSplit and IncrementalSplit. A negative id means the files are not committed yet, and no deletion vector is applied. If the snapshot file has expired, refresh throws OutOfRangeException rather than looking up without its deletion vectors.

Lookup ignored deletion vectors after compaction dropped the delete record, so partial lookup and the query service returned deleted rows.

Deletion vectors are now loaded from the snapshot that produced the installed files. A newer commit can no longer hide or reveal rows in a file plan that has not been applied yet. If that snapshot has expired, refresh fails with OutOfRangeException instead of serving the files without their vectors.
@Stephen0421
Stephen0421 force-pushed the fix-lookup-deletion-vector branch from 74b0738 to 15606ce Compare October 27, 2026 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants