Core: Add dv_count column to PartitionsTable - #17851
Conversation
| if (file.format() == FileFormat.PUFFIN) { | ||
| this.dvCount += 1; | ||
| } |
There was a problem hiding this comment.
1 Puffin files can multiple DVs right ?
There was a problem hiding this comment.
The intent is to sum up the number of DVs and not the number of Puffin files containing DVs. I believe the current implementation does that.
There was a problem hiding this comment.
i see, that make sense, we should add a test for this scenario to make sure we never double count !
There was a problem hiding this comment.
have written the test and pushed the changes
|
Hey @hemanthboyina , Thanks for the PR! Would you mind, for the record, linking the previous PR and the revert and give some context? |
|
thanks @gaborkaszab , updated the PR description with the context. |
|
I broke this PR, because the Flink version upgrade (to support 1.20, 2.1, 2.2, 2.3 in 1.12.0) takes priority and I have merged it. Please rebase, and sorry for the extra work. |
singhpk234
left a comment
singhpk234
left a comment
There was a problem hiding this comment.
Thanks @pvary, the change mostly LGTM, thanks @hemanthboyina , added a test suggestion
Recommend updating the doc too once this
| if (file.format() == FileFormat.PUFFIN) { | ||
| this.dvCount += 1; | ||
| } |
There was a problem hiding this comment.
i see, that make sense, we should add a test for this scenario to make sure we never double count !
cfe6e25 to
a69b214
Compare
4 participants
This adds a new dv_count column to the partitions metadata table that reports the number of deletion vectors (DVs) per partition.
Background
This is a follow-up to #16125 (adding
dv_count), which was reverted in #17825.The original change reused the existing
position_delete_file_countfield to excludeDVs, which was a behavioral change: position_delete_file_count had previously
counted all position deletes, including DVs. Changing that semantic could break users
relying on the existing behavior, so the PR was reverted.
Per the discussion on the revert, this takes a non-breaking approach:
position-delete files, including DVs.
so dv_count counts DVs rather than physical Puffin files, even when multiple DVs are
packed into a single Puffin file.