[core] introducing row-range-keyed DeletionVectors for DataEvolution tables#8344
[core] introducing row-range-keyed DeletionVectors for DataEvolution tables#8344steFaiz wants to merge 6 commits into
Conversation
| return false; | ||
| } | ||
|
|
||
| return currentDv.isDeleted(rowId); |
There was a problem hiding this comment.
This reader only evaluates the first row-range DV whose end has not passed the current row id. If two row-range keyed DVs overlap, for example [0, 10] and [5, 15], rows 5-10 are never checked against the second DV, so deletions recorded only there will be returned. I don't see the writer/manifest combiner rejecting overlapping RowIdRangeKey values; they only deduplicate exact keys. Please either enforce non-overlapping row-range DVs when writing/combining metadata, or make this reader evaluate all DVs that may contain the row.
There was a problem hiding this comment.
Thanks for your remind! I've added the check in GlobalCombiner and added a test
| ? null | ||
| : IndexFileMetaSerializer.dvMetasToRowArrayData(dvMetas.values()), | ||
| : IndexFileMetaSerializer.metasToRowArrayData( | ||
| dvMetas, DeletionFileKey.Type.FILE_NAME), |
There was a problem hiding this comment.
For row-range keyed DVs this renders the compatibility marker row instead of the real row-id ranges, because metasToRowArrayData(..., FILE_NAME) intentionally fast-fails old readers. That makes table_indexes.dv_ranges misleading for data-evolution tables. Can we expose the row-range DV schema here as well, or render the ranges in a separate column, instead of showing the legacy marker?
There was a problem hiding this comment.
This is very helpful! I've unified FileNameKey and RowRangeKey:
- Reusing the same field
- For FileNames, display the file name
- For RowRanges, display the formated range. e.g. [0, 100]
|
I thought about the design and I think that making row ranged-DV aligned with normal file ranges is much more simple and easy to manage, compared to fully decoupled mode. Just like filename-based implementation.
This is because it's complicated to update DVs if we let DVs can be fully decouple, consider this scenario: // Current DV:
[0, 100] , [101, 200], [230, 300]Update a new DV range [50, 250]
If we don't design sophiscated merge/split mechanism, there might be some DVs' size too big or too small. I think we could:
@JingsongLi Could you please review this proposal and share your thoughts? Any feedback would be greatly appreciated! |
Purpose
The first part of #8322
This is a very BIG pr, I tried to split it into several sub PRs, but found it very hard.
The PR only focuses on core module, leaving Spark/Flink module untouched, mainly includes:
DeletionFileKeyto replace currentfilenameas the unique identifier of each DeletionFileDataEvolutionApplyDvReaderto filter out deleted row ids. This class refers toIndexedSplitRecordReader: First enrich readType by_ROW_ID, then filter rows by RowRange DeletionVectors.SnapshotReaderImpl, assigning overlapping row range DeletionVectors to each DataSplit.Besides, most changes are about changing many
Stringfields toDeletionFileKey.Tests
Unit Tests only.