[Feature] Support query deletion vector for delta lake #53766
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![starrocks-cr[bot]](https://avatars.githubusercontent.com/in/404334?s=60&v=4){kind=small}
Youngwb
force-pushed
the
dv_poc
branch
2 times, most recently
from
6c1fde6 to
64ef676
Compare
Signed-off-by: Youngwb <[email protected]>
| @@ -315,6 +315,15 @@ struct TPaimonDeletionFile { | |||
| 3: optional i64 length | |||
| } | |||
|
|
|||
| // refer to https://github.com/delta-io/delta/blob/master/PROTOCOL.md#deletion-vector-descriptor-schema | |||
| struct TDeletionVectorDescriptor { | |||
There was a problem hiding this comment.
Is iceberg dv going to reuse this field?
Signed-off-by: Youngwb <[email protected]>
|
DorianZheng
reviewed
Dec 12, 2024
| uint64_t bitmap_cardinality = roaring64_bitmap_get_cardinality(bitmap); | ||
| std::unique_ptr<uint64_t[]> bitmap_array(new uint64_t[bitmap_cardinality]); | ||
| roaring64_bitmap_to_uint64_array(bitmap, bitmap_array.get()); | ||
| need_skip_rowids->insert(bitmap_array.get(), bitmap_array.get() + bitmap_cardinality); |
There was a problem hiding this comment.
Since eventually need_skip_rowids will be converted to flat bitmap, we should construct the flat bitmap directly from roaring64_bitmap_t.
There was a problem hiding this comment.
Yes, but here we can still need this set, because the group reader need this set to find lowr/upper bound
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 14 / 14 (100.00%) file detail
|
[BE Incremental Coverage Report]✅ pass : 130 / 145 (89.66%) file detail
|
dirtysalt
reviewed
Dec 12, 2024
| offset += MAGIC_NUMBER_LENGTH; | ||
|
|
||
| int64_t serialized_bitmap_length = length - MAGIC_NUMBER_LENGTH; | ||
| std::unique_ptr<char[]> deletion_vector(new char[serialized_bitmap_length]); |
There was a problem hiding this comment.
you can just use std::vector.
unique_ptr<char[]> is little weird.
There was a problem hiding this comment.
OK. I will modify this next PR
|
@Youngwb great job. We can support deletion vector on delta lake finally! |
gengjun-git
approved these changes
Dec 12, 2024
Seaven
approved these changes
Dec 12, 2024
1 task
|
@mergify backport branch-3.4 |
✅ Backports have been created
|
mergify bot
pushed a commit
that referenced
this pull request
Dec 13, 2024
Signed-off-by: Youngwb <[email protected]> (cherry picked from commit 66b78c4)
18 tasks
wanpengfei-git
pushed a commit
that referenced
this pull request
Dec 13, 2024
…) (#53944) Co-authored-by: Youngwb <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why I'm doing:
What I'm doing:
Support query deletion vector for delta lake
Fixes #53767
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: