feat(puffin): support deletion-vector-v1 blob read/write by zhaoxuan1994 · Pull Request #777 · apache/iceberg-cpp

zhaoxuan1994 · 2026-06-24T08:22:37Z

feat(puffin): support deletion-vector-v1 blob read/write

Add end-to-end support for the deletion-vector-v1 Puffin blob type on top
of the existing PuffinReader/PuffinWriter and RoaringPositionBitmap.

Encoding (puffin/deletion_vector.{h,cc}):

SerializeDeletionVectorBlob / DeserializeDeletionVectorBlob implement the
DV blob framing: 4-byte big-endian length, 0xD1D33964 magic, the portable
Roaring vector, and a trailing big-endian CRC-32. Reads validate the magic
and checksum.
MakeDeletionVectorBlob builds a spec-compliant Blob: fields set to the
row-position metadata column id, snapshot-id/sequence-number = -1, no
compression, and the required referenced-data-file/cardinality properties.

Writing (data/deletion_vector_writer.{h,cc}):

DeletionVectorWriter accumulates deleted positions per data file, run-length
encodes each bitmap before serializing, writes one blob per data file, and
produces a DataFile per blob carrying content_offset/content_size_in_bytes,
referenced_data_file and record_count for manifest registration.

Reading (data/delete_loader.cc):

Implement DeleteLoader::LoadDV: read the blob bytes referenced by
content_offset/content_size_in_bytes, validate referenced_data_file, the 2GB
limit and cardinality == record_count, then apply positions to the index.
This wires DV deletes into DeleteFilter::ComputeAliveRows.

Puffin writer convenience:

PuffinWriter::Make now fills a default created-by property when the caller
does not provide one, matching the Java writer.

Behavior matches the Java implementation (BaseDVFileWriter / BaseDeleteLoader /
BitmapPositionDeleteIndex): magic, byte order, CRC coverage, blob fields, RLE
and the read-side validations are all aligned.

Tests:

DV blob framing round-trip and error cases (bad magic, corrupted CRC,
truncated blob, size mismatch).
DeletionVectorWriter -> DeleteLoader write/read round trip and guard cases.
delete_loader: load DV, referenced-file filtering, cardinality mismatch,
mixed DV + position-delete loading.
delete_filter: end-to-end ComputeAliveRows filtering with a real Puffin DV
file over Arrow FileIO.
PuffinWriter created-by default and caller-precedence.

Not included (deletion-vector upper layer, follow-up): merging with previously
written DVs (compaction), the bulk delete(PositionDeleteIndex) API, and
per-data-file partition/spec in a single DV file.

Copilot

Pull request overview

Adds end-to-end support for Iceberg Puffin deletion-vector-v1 blobs, including framing/validation, write-side generation of DV Puffin files, and read-side application of DVs through DeleteLoader into DeleteFilter row filtering.

Changes:

Implement deletion-vector-v1 blob framing (length + magic + Roaring portable bytes + CRC32) plus spec-compliant Blob construction helpers.
Add DeletionVectorWriter to emit one DV blob per referenced data file and return manifest-ready DataFile metadata; wire DV loading into DeleteLoader.
Extend Puffin writer metadata defaults (created-by) and add comprehensive DV-focused unit tests + build system wiring.

Reviewed changes

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

Show a summary per file

File	Description
src/iceberg/test/puffin_reader_writer_test.cc	Adds tests verifying default `created-by` behavior and caller override precedence.
src/iceberg/test/puffin_deletion_vector_test.cc	New tests for DV blob framing (round-trip + corruption cases) and Puffin end-to-end DV read/write.
src/iceberg/test/meson.build	Registers new DV-related test sources/targets in Meson.
src/iceberg/test/deletion_vector_writer_test.cc	New end-to-end tests for `DeletionVectorWriter` ↔ `DeleteLoader` DV round trips and option/state guards.
src/iceberg/test/delete_loader_test.cc	Adds DV load tests (load, skip mismatched referenced file, cardinality mismatch, mixed DV + position deletes).
src/iceberg/test/delete_filter_test.cc	Adds `ComputeAliveRows` DV end-to-end test and updates expected error kinds.
src/iceberg/test/CMakeLists.txt	Registers new DV-related tests in CMake.
src/iceberg/puffin/puffin_writer.cc	Auto-populates `created-by` property when missing.
src/iceberg/puffin/meson.build	Installs new Puffin DV header in Meson.
src/iceberg/puffin/deletion_vector.h	New public API for DV blob serialize/deserialize + spec-compliant `Blob` creation.
src/iceberg/puffin/deletion_vector.cc	Implements DV blob framing, magic/CRC validation, and blob construction.
src/iceberg/meson.build	Adds new DV source files to the Meson build.
src/iceberg/data/meson.build	Installs `deletion_vector_writer.h` in Meson.
src/iceberg/data/deletion_vector_writer.h	New writer API for emitting DV blobs into a Puffin file and returning `DataFile` metadata.
src/iceberg/data/deletion_vector_writer.cc	Implements DV accumulation per referenced file, optimization, Puffin blob emission, and metadata creation.
src/iceberg/data/delete_loader.cc	Implements DV loading path: reads referenced bytes, validates, deserializes, and applies positions to the index.
src/iceberg/CMakeLists.txt	Adds new DV source files to the CMake build.

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

wgtmac · 2026-06-26T01:05:16Z

+    };
+    std::vector<Entry> entries;
+    entries.reserve(bitmaps_.size());
+    for (auto& [referenced_data_file, bitmap] : bitmaps_) {


This should merge existing deletes before writing a DV for a path. The spec requires at most one DV per data file, and Java BaseDVFileWriter loads previous DVs/position deletes and reports rewritten delete files. As-is, adding another DV can leave stale delete files in table state.

wgtmac · 2026-06-26T01:05:16Z

+  /// Partition spec the referenced data files belong to (optional).
+  std::shared_ptr<PartitionSpec> spec;
+  /// Partition the referenced data files belong to.
+  PartitionValues partition;


Please store the partition spec and partition per referenced data file, not per writer. Java DVFileWriter passes them on each delete call, and the spec allows one Puffin file to hold DVs for files from different partitions. With this API, one writer can emit the wrong delete-file metadata.

wgtmac · 2026-06-26T01:05:16Z

+  // Identify the writer in the footer unless the caller already set it. Only
+  // format the default value when it is actually needed.
+  const std::string created_by_key(StandardPuffinProperties::kCreatedBy);
+  if (!properties.contains(created_by_key)) {


Please keep this default in the DV writer path instead of the generic PuffinWriter. Java does not add created-by in Puffin.write(...).build(); BaseDVFileWriter sets it when creating DV files. This changes footer metadata for all Puffin writers.

wgtmac · 2026-06-26T01:12:12Z

+  /// \brief Mark a row position as deleted for the given data file.
+  ///
+  /// Positions are accumulated per data file and serialized on Close().
+  Status Delete(std::string_view referenced_data_file, int64_t pos);


Please make this match Java DVFileWriter more closely. Delete should take the data file spec and partition, and there should be an overload that accepts a PositionDeleteIndex. Otherwise the writer cannot represent per-file metadata or bulk deletes.

wgtmac · 2026-06-26T01:12:12Z

+
+  /// \brief Metadata for the DataFiles produced (one per referenced data file).
+  /// \note Must be called after Close().
+  Result<FileWriter::WriteResult> Metadata();


Please return a delete-specific result here, not FileWriter::WriteResult. Java returns DeleteWriteResult with delete files, referenced data files, and rewritten delete files. Row delta needs those sets for conflict checks and cleanup.

wgtmac · 2026-06-26T01:12:12Z

+///
+/// The returned bytes include the length prefix, magic sequence, the Roaring
+/// "portable" serialization of the bitmap, and the trailing CRC-32 checksum.
+ICEBERG_DATA_EXPORT Result<std::vector<uint8_t>> SerializeDeletionVectorBlob(


Please avoid exposing DV blob serialization as a public Puffin API. Java keeps this behind PositionDeleteIndex and BitmapPositionDeleteIndex. Exposing RoaringPositionBitmap here makes the bitmap layout part of the public surface.

wgtmac · 2026-06-26T01:12:12Z

+ private:
+  DeletionVectorWriterOptions options_;
+  // Ordered by referenced data file path for deterministic blob layout.
+  std::map<std::string, RoaringPositionBitmap> bitmaps_;


Please model the private state like Java Deletes: path, positions, spec, and partition. A map from path to bitmap loses per-file metadata, so the wrong manifest metadata can be produced.

wgtmac · 2026-06-26T01:12:31Z

+  // Identify the writer in the footer unless the caller already set it. Only
+  // format the default value when it is actually needed.
+  const std::string created_by_key(StandardPuffinProperties::kCreatedBy);
+  if (!properties.contains(created_by_key)) {


Please do not set created-by in the generic PuffinWriter. Java only sets it in BaseDVFileWriter when writing DVs. Keeping it here changes footer metadata for every Puffin writer.

wgtmac · 2026-06-26T01:12:32Z

+
+/// \brief Constants describing the `deletion-vector-v1` blob framing.
+///
+/// The serialized blob has the following layout (see the Puffin spec):


This is too much spec text for a public header. Please keep the header brief and point to the Puffin spec. The byte layout is better covered in the implementation or tests.

wgtmac · 2026-06-26T01:12:32Z

+    entries.reserve(bitmaps_.size());
+    for (auto& [referenced_data_file, bitmap] : bitmaps_) {
+      // Run-length encode before serializing for space efficiency, matching the
+      // Java DV writer.


Please avoid comments that justify the code by saying it matches Java. A short local comment is enough, or the test can cover the parity. This reads like review explanation.

wgtmac · 2026-06-26T01:12:32Z

+/// content_offset/content_size_in_bytes and referenced_data_file required to
+/// register the deletion vector in a manifest.
+///
+/// \note All referenced data files are assumed to belong to the single


Please remove this note once the API is fixed. It documents a limitation that Java does not have and that the spec does not require.

wgtmac · 2026-06-26T01:15:47Z

+
+Result<std::vector<uint8_t>> SerializeDeletionVectorBlob(
+    const RoaringPositionBitmap& bitmap) {
+  ICEBERG_ASSIGN_OR_RAISE(auto serialized, bitmap.Serialize());


If this helper stays public, please run-length encode before serializing here, not only in DeletionVectorWriter. Java PositionDeleteIndex.serialize() does it on every serialization path. Direct calls to this helper currently skip that step.

Copilot AI review requested due to automatic review settings June 24, 2026 08:22

Copilot started reviewing on behalf of zhaoxuan1994 June 24, 2026 08:23 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

Comment thread src/iceberg/puffin/puffin_writer.cc Outdated

Comment thread src/iceberg/data/deletion_vector_writer.cc

zhaoxuan1994 force-pushed the feat/puffin-deletion-vector-v1 branch 4 times, most recently from da65ba6 to d3f8f42 Compare June 24, 2026 11:11

manuzhang mentioned this pull request Jun 25, 2026

Track Iceberg v3 Feature Support (Roadmap for 0.4.0) #637

Open

23 tasks

wgtmac reviewed Jun 26, 2026

View reviewed changes

feat(puffin): support deletion-vector-v1 blob read/write

24a9c20

zhaoxuan1994 force-pushed the feat/puffin-deletion-vector-v1 branch from d3f8f42 to 24a9c20 Compare June 26, 2026 03:20

Uh oh!

Conversation

zhaoxuan1994 commented Jun 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants