Skip to content

feat: MergingSnapshotUpdate merges multiple DVs#708

Merged
wgtmac merged 2 commits into
apache:mainfrom
HuaHuaY:impl_merge_dv
Jul 16, 2026
Merged

feat: MergingSnapshotUpdate merges multiple DVs#708
wgtmac merged 2 commits into
apache:mainfrom
HuaHuaY:impl_merge_dv

Conversation

@HuaHuaY

@HuaHuaY HuaHuaY commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements support for merging multiple deletion vectors (DVs) that reference the same data file by rewriting them into merged Puffin DV blobs, enabling format v3 workflows that previously returned NotImplemented.

Changes:

  • Add DV-merge pipeline in MergingSnapshotUpdate that unions multiple DV bitmaps per referenced data file and writes merged DV blobs into a Puffin container.
  • Introduce TransactionContext::NewDataLocation(...) to generate table-consistent data-file output locations for merged DV Puffin files.
  • Promote Puffin + RoaringPositionBitmap components from iceberg_data into the core iceberg library (exports + build wiring) and add a test covering DV merging.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/iceberg/update/merging_snapshot_update.h Adds DV merge APIs and tracking for merged DV outputs.
src/iceberg/update/merging_snapshot_update.cc Implements DV merging via Puffin reader/writer and bitmap union; adds cleanup handling for merged DV outputs.
src/iceberg/transaction.h Adds TransactionContext::NewDataLocation API.
src/iceberg/transaction.cc Implements NewDataLocation via LocationProvider.
src/iceberg/test/merging_snapshot_update_test.cc Adds an integration-style test validating duplicate DV merging behavior and correctness.
src/iceberg/puffin/puffin_writer.h Moves export macro to core ICEBERG_EXPORT.
src/iceberg/puffin/puffin_reader.h Moves export macro to core ICEBERG_EXPORT.
src/iceberg/puffin/puffin_format.h Moves export macro to core ICEBERG_EXPORT.
src/iceberg/puffin/json_serde_internal.h Moves export macro to core ICEBERG_EXPORT.
src/iceberg/puffin/file_metadata.h Moves export macro to core ICEBERG_EXPORT.
src/iceberg/deletes/roaring_position_bitmap.h Moves export macro to core ICEBERG_EXPORT.
src/iceberg/meson.build Moves Puffin + bitmap sources/deps into iceberg lib; adjusts croaring linkage.
src/iceberg/CMakeLists.txt Moves Puffin + bitmap sources into iceberg lib; adjusts croaring linkage propagation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/iceberg/update/merging_snapshot_update.cc Outdated
Comment thread src/iceberg/update/merging_snapshot_update.cc Outdated
@HuaHuaY
HuaHuaY force-pushed the impl_merge_dv branch 2 times, most recently from 284b077 to f25cbe1 Compare June 22, 2026 08:10
Comment thread src/iceberg/puffin/puffin_writer.cc Outdated
Comment thread src/iceberg/meson.build Outdated
Comment thread src/iceberg/update/merging_snapshot_update.cc Outdated
Comment thread src/iceberg/update/merging_snapshot_update.cc Outdated
@wgtmac

wgtmac commented Jun 24, 2026

Copy link
Copy Markdown
Member

@zhaoxuan1994 is working on adding puffin reader/writer for v3 DVs. Let's wait for it and then rebase on it later.

@wgtmac
wgtmac force-pushed the impl_merge_dv branch 2 times, most recently from b480fc0 to 97bdb7c Compare July 15, 2026 16:42
@wgtmac

wgtmac commented Jul 16, 2026

Copy link
Copy Markdown
Member

I've made several changes to this PR:

  • Kept puffin and dv code in iceberg-data library to make the core library IO-less still.
  • Refactored DV logic based on a recent commit which added DV writer and reader support.
  • Removed misc-include-cleaner from clang-tidy since I found it is useless but adds a lot of noise by blocking every ci until we add a lot of lengthy include statements.
  • Removed ci workflow for sigv4 with meson. Recently vcpkg update introduced a breaking change of aws sdk. I think this workflow is fragile but there is no active Meson expert. Considering that Meson is still partially supported, I don't want to waste time and burn tokens to fix it. So I directly removed this workflow and may eventually remove all Meson code if unmaintained for a long time (perhaps a year?).

@wgtmac wgtmac changed the title feat: implement merge multiple DVs feat: MergingSnapshotUpdate merges multiple DVs Jul 16, 2026
Comment thread .clang-tidy
clang-analyzer-*,
google-*,
modernize-*,
misc-include-cleaner,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#820 has been reverted via this change because it does not add too much value but a lot of noise. cc @yadavay-amzn @zhjwpku

@wgtmac
wgtmac merged commit c5a5b5c into apache:main Jul 16, 2026
21 checks passed
@HuaHuaY
HuaHuaY deleted the impl_merge_dv branch July 16, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants