[core] Keep anchor normal file when DE read-type pruning leaves only dedicated files - #9966
LuciferYang wants to merge 3 commits into
Conversation
…dedicated files For data-evolution tables without deletion vectors, a projection that references only blob/vector-store columns (e.g. SELECT a blob column added later and only partially backfilled) survives pruneByReadType with the dedicated files alone: the anchor normal file does not write any queried column and, unlike the deletion-vector path, is not added back. Downstream, DataEvolutionSplitRead derives the logical row range from the surviving files; a blob or vector-store file may cover only a sub-range of its group, so every row outside the dedicated files' sub-ranges is silently dropped. Keep the full-range anchor normal file when the kept list would otherwise hold only dedicated files that do not cover the requested rows. Coverage is checked against the effective requested ranges: the row-range pushdown intersected with the group, or the whole group when there is no pushdown. When the dedicated files already cover the requested rows the anchor is unnecessary, so it is not pulled in (a kept normal file also makes it unnecessary, since all normal files of a row-id-range group share the same range). The pruning core is a static method (mirroring evolutionStats) so both behaviors can be pinned by unit tests.
3b830e9 to
ac9d087
Compare
Akash3121
left a comment
Akash3121
left a comment
There was a problem hiding this comment.
LGTM
The fix correctly preserves the normal anchor only when read-type pruning leaves dedicated blob/vector files that do not cover all requested row IDs. I verified the key assumption: DataEvolutionSplitRead.mergeRangesAndSort enforces that overlapping normal files share the same row-ID range, so an already-retained normal file is a sufficient representative. The coverage logic also correctly handles disjoint pushed-down ranges and null-filling outside dedicated-file subranges.
|
Reviewed head Local JDK 8 verification passed: |
| * covering the requested targets means the reader can derive the range from them without the | ||
| * anchor. | ||
| */ | ||
| private static boolean coversRanges(List<ManifestEntry> entries, List<Range> targets) { |
There was a problem hiding this comment.
[P1] This coverage check unions files from different projected columns. With a normal file covering [0,9], b1.blob covering [0,6], and b2.blob covering [3,9], the union covers [0,9], so the anchor is omitted. DataEvolutionSplitRead still creates a separate BlobFileBunch per field, and SELECT b1, b2 fails its first-row-ID alignment check (0 vs 3). I reproduced this with an end-to-end test on this commit. Please retain the anchor unless each field bunch is independently aligned with the requested rows, or keep it whenever pruning leaves only dedicated files.
There was a problem hiding this comment.
Fixed. Coverage is now judged per projected column instead of by unioning files across columns, so a projection whose columns cover disjoint sub-ranges (b1 over [0,6], b2 over [3,9]) keeps the anchor. Added DataEvolutionFileStoreScanTest#testReadTypePruningKeepsAnchorForMultiColumnBlobs.
| ? Collections.singletonList(fullRange) | ||
| : rowRangeIndex.intersectedRanges(fullRange.from, fullRange.to); | ||
| if (!coversRanges(kept, requested)) { | ||
| kept.add(fullRangeAnchor); |
There was a problem hiding this comment.
[P1] Retaining the full-range anchor here does not yet make a partially populated vector column readable. A normal file covering [0,9] and a single vector file covering [0,4], projected as SELECT embedding, produce field bunches of 10 and 5 rows. DataEvolutionVectorReadPlanner returns null for a single vector group, so DataEvolutionSplitRead.createUnionReader throws 'All files in a field merge split should have the same row count.' I reproduced this with an end-to-end test on this commit. Please align/null-fill the vector bunch against the anchor's logical range (or use a range-aware reader), and add a regression test without deletion vectors.
There was a problem hiding this comment.
Fixed. DataEvolutionVectorReadPlanner now plans explicit ranges when a projected vector store covers only a sub-range of its group, so the uncovered rows are NULL-filled through the range-aware path instead of falling to the sequential reader that rejects the mismatched row count. Added VectorStoreTableTest#testPartialUpdateWithMissingVectorRangeWithoutDeletionVectors (no deletion vectors).
…ectors Addresses review on apache#9966: - pruneByReadType judged dedicated-file coverage by unioning files across projected columns, so a multi-column projection whose columns cover disjoint sub-ranges dropped the anchor and misaligned the per-column field bunches. Judge coverage per column instead. - A projected vector store covering only a sub-range of its group made DataEvolutionVectorReadPlanner return null and fall to the sequential reader, which rejects the mismatched row count. Plan explicit ranges so the uncovered rows are NULL-filled. Add regression tests: the multi-column blob case, and a partial vector projection without deletion vectors.
3 participants
Purpose
close #9965
In a data-evolution table,
DataEvolutionFileStoreScan.pruneByReadTypekeeps only the files that write a projected or filter field. When a projection selects a column stored in a dedicated file (a blob or vector-store file), that file may cover only a sub-range of its row-id group, so the kept list can hold dedicated files that do not span the whole group.DataEvolutionSplitReadthen derives the group's logical range from those sub-ranges and silently drops the rows outside them.This keeps the full-range anchor normal file when read-type pruning would otherwise leave a group holding only dedicated files that do not cover the requested rows (the row-range pushdown intersected with the group, or the whole group when there is no pushdown). Coverage is judged per projected column: each column is read as its own field bunch, so one column's files filling another column's gap does not make that column readable. When a kept normal file already gives the group its full range, or the dedicated files cover the requested rows for every projected column, the anchor is not added.
Vector-store columns need one more step. A projected vector that covers only a sub-range made
DataEvolutionVectorReadPlannerreturn a null plan and fall to the sequential union reader, which rejects the mismatched row count. The planner now plans explicit ranges in that case, so the uncovered rows are NULL-filled through the existing range-aware path.Tests
DataEvolutionFileStoreScanTest:testReadTypePruningKeepsAnchorForMultiColumnBlobspins the multi-column case (b1 over [0,6] and b2 over [3,9], whose union spans the group but neither column does alone), alongside the single-column, no-normal-file, and row-range-pushdown cases that keep or skip the anchor by per-column coverage.BlobUpdateTest#testProjectBlobColumnBackfilledForSubRangeKeepsAllRows: a blob column backfilled for only [0,4] of a [0,9] group, projected alone, must return 10 rows with NULL fill outside [0,4].VectorStoreTableTest#testPartialUpdateWithMissingVectorRangeWithoutDeletionVectors: a vector column populated for only part of the group, projected without deletion vectors, must NULL-fill the uncovered rows instead of throwing on the row-count mismatch.API and Format
no
Documentation
no