Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Iceberg row group fix new #10505

Closed

Conversation

yingsu00
Copy link
Collaborator

This PR contains two fixes:
1.
Fixes #9856
When the base data file and positional delete files contains multiple
unaligned RowGroups, some of the bits at the end of
IcebergSplitReader::deleteBitmap_ could be mistakenly skipped, causing
wrong result. This commit fixes it by introducing an offset into this
deleteBitmap_ and shift the unused bits to the beginning for each batch.

When a base file batch doesn't have matching delete positions, the delete
bitmap size was set incorrectly. For such a batch, the delete bitmap size should
be set as 0.

This commit makes it possible to create base data files and delete fiels
with un-aligned Rowgroup boundaries. It also added several new test cases
and improved some variable and function namings.
When the base data file and positional delete files contains multiple
unaligned RowGroups, some of the bits at the end of
IcebergSplitReader::deleteBitmap_ could be mistakenly skipped, causing
wrong result. This commit fixes it by introducing an offset into this
deleteBitmap_ and shift the unused bits to the beginning for each batch.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 18, 2024
Copy link

netlify bot commented Jul 18, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 832348e
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/669980a60a39b500070c620e

When a base file batch doesn't have matching delete positions, the delete
bitmap size was set incorrectly. For such a batch, the delete bitmap size should
be set as 0.

Co-authored-by: Ying Su <[email protected]>
@yingsu00 yingsu00 requested a review from Yuhta July 18, 2024 20:53
@yingsu00 yingsu00 marked this pull request as ready for review July 18, 2024 20:53
@Yuhta Yuhta added the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Jul 19, 2024
@facebook-github-bot
Copy link
Contributor

@kevinwilfong has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@kevinwilfong merged this pull request in 7089078.

Copy link

Conbench analyzed the 1 benchmark run on commit 7089078f.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong result while Iceberg table read with Positional Delete
3 participants