From 5bd467fd26ff53e1cda2dd67b959570a386145e4 Mon Sep 17 00:00:00 2001 From: Anoop Johnson Date: Wed, 2 Sep 2026 12:43:31 -0700 Subject: [PATCH 1/5] API, Core: Add ManifestFile adapter for v4 tracked files Reintroduces the ManifestFile adapter closed in #16867, which stalled because ManifestFile had no way to represent a manifest deletion vector. The V4 scan task planner (#17541) plans over manifests through the ManifestFile API, so this adds ManifestFile.deletionVector() and has the adapter expose the tracked file's manifest DV. --- .../java/org/apache/iceberg/ManifestFile.java | 9 +- .../apache/iceberg/TrackedFileAdapters.java | 129 +++++++++- .../iceberg/TestTrackedFileAdapters.java | 239 ++++++++++++++++++ 3 files changed, 373 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/ManifestFile.java b/api/src/main/java/org/apache/iceberg/ManifestFile.java index 2f732aef427f..52c90fabc706 100644 --- a/api/src/main/java/org/apache/iceberg/ManifestFile.java +++ b/api/src/main/java/org/apache/iceberg/ManifestFile.java @@ -126,7 +126,7 @@ static Schema schema() { /** Returns length of the manifest file. */ long length(); - /** Returns iD of the {@link PartitionSpec} used to write the manifest file. */ + /** Returns ID of the {@link PartitionSpec} used to write the manifest file. */ int partitionSpecId(); /** Returns the content stored in the manifest; either DATA or DELETES. */ @@ -138,7 +138,7 @@ static Schema schema() { /** Returns the lowest data sequence number of any live file in the manifest. */ long minSequenceNumber(); - /** Returns iD of the snapshot that added the manifest file to table metadata. */ + /** Returns ID of the snapshot that added the manifest file to table metadata. */ Long snapshotId(); /** @@ -210,6 +210,11 @@ default Long firstRowId() { return null; } + /** Returns the manifest deletion vector, or null if absent. */ + default ByteBuffer deletionVector() { + return null; + } + /** * Copies this {@link ManifestFile manifest file}. Readers can reuse manifest file instances; use * this method to make defensive copies. diff --git a/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java b/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java index 154abb9f7478..ea2d0f37fc53 100644 --- a/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java +++ b/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java @@ -24,7 +24,7 @@ import java.util.Set; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; -/** Adapts {@link TrackedFile} entries to the {@link DataFile} and {@link DeleteFile} APIs. */ +/** Adapts {@link TrackedFile} entries to their read APIs, for example {@link DataFile}. */ class TrackedFileAdapters { private TrackedFileAdapters() {} @@ -53,7 +53,16 @@ static DeleteFile asEqualityDeleteFile(TrackedFile file, Map> implements ContentFile { private final TrackedFile file; @@ -405,6 +414,122 @@ public DeleteFile copyWithStats(Set requestedColumnIds) { } } + /** Adapts a TrackedFile to {@link ManifestFile}. */ + private static class TrackedManifestFile implements ManifestFile { + private final TrackedFile file; + + private TrackedManifestFile(TrackedFile file) { + Tracking tracking = file.tracking(); + Preconditions.checkArgument( + tracking.dataSequenceNumber() != null, "Invalid data sequence number: null"); + Preconditions.checkArgument( + tracking.dataSequenceNumber().equals(tracking.fileSequenceNumber()), + "Manifest data and file sequence numbers must be equal, got %s and %s", + tracking.dataSequenceNumber(), + tracking.fileSequenceNumber()); + this.file = file; + } + + @Override + public String path() { + return file.location(); + } + + @Override + public long length() { + return file.fileSizeInBytes(); + } + + @Override + public int partitionSpecId() { + throw new UnsupportedOperationException( + "v4 manifests are not bound to a single partition spec"); + } + + @Override + public ManifestContent content() { + switch (file.contentType()) { + case DATA_MANIFEST: + return ManifestContent.DATA; + case DELETE_MANIFEST: + return ManifestContent.DELETES; + default: + throw new IllegalStateException( + "Unsupported content type for manifests: " + file.contentType()); + } + } + + @Override + public long sequenceNumber() { + return file.tracking().dataSequenceNumber(); + } + + @Override + public long minSequenceNumber() { + return file.manifestInfo().minSequenceNumber(); + } + + @Override + public Long snapshotId() { + return file.tracking().snapshotId(); + } + + @Override + public Integer addedFilesCount() { + return file.manifestInfo().addedFilesCount(); + } + + @Override + public Long addedRowsCount() { + return file.manifestInfo().addedRowsCount(); + } + + @Override + public Integer existingFilesCount() { + return file.manifestInfo().existingFilesCount(); + } + + @Override + public Long existingRowsCount() { + return file.manifestInfo().existingRowsCount(); + } + + @Override + public Integer deletedFilesCount() { + return file.manifestInfo().deletedFilesCount(); + } + + @Override + public Long deletedRowsCount() { + return file.manifestInfo().deletedRowsCount(); + } + + @Override + public List partitions() { + return null; + } + + @Override + public ByteBuffer keyMetadata() { + return file.keyMetadata(); + } + + @Override + public Long firstRowId() { + return file.tracking().firstRowId(); + } + + @Override + public ByteBuffer deletionVector() { + return file.manifestInfo().dv(); + } + + @Override + public ManifestFile copy() { + return new TrackedManifestFile(file.copy()); + } + } + private static PartitionSpec resolveSpec( TrackedFile file, Map specsById) { Integer specId = file.specId(); diff --git a/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java b/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java index c9a45928ad70..d059479e64bc 100644 --- a/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java +++ b/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java @@ -38,9 +38,12 @@ class TestTrackedFileAdapters { private static final String MANIFEST_LOCATION = "s3://bucket/table/manifest.parquet"; private static final String DATA_FILE_LOCATION = "s3://bucket/data/file.parquet"; private static final String DV_LOCATION = "s3://bucket/puffin/dv-file.bin"; + private static final String MANIFEST_FILE_LOCATION = "s3://bucket/table/manifest-1.parquet"; + private static final long MANIFEST_FILE_SIZE = 2048L; // Tracking values that the delegation tests validate. private static final long MANIFEST_POS = 3L; + private static final long SNAPSHOT_ID = 42L; private static final long DATA_SEQUENCE_NUMBER = 10L; private static final long FILE_SEQUENCE_NUMBER = 11L; private static final long FIRST_ROW_ID = 1000L; @@ -83,6 +86,35 @@ class TestTrackedFileAdapters { CONTENT_STATS.setStats(2, SCORE_STATS); } + private static final Tracking MANIFEST_TRACKING = + new TrackingStruct( + EntryStatus.ADDED, + SNAPSHOT_ID, + // data and file sequence numbers must be equal + DATA_SEQUENCE_NUMBER, + DATA_SEQUENCE_NUMBER, + /* dvSnapshotId= */ null, + FIRST_ROW_ID, + /* deletedPositions= */ null, + /* replacedPositions= */ null); + + private static final byte[] MANIFEST_DV = new byte[] {1, 2, 3}; + + private static final ManifestInfo MANIFEST_INFO = + ManifestInfoStruct.builder() + .addedFilesCount(3) + .existingFilesCount(5) + .deletedFilesCount(2) + .replacedFilesCount(0) + .addedRowsCount(300L) + .existingRowsCount(500L) + .deletedRowsCount(200L) + .replacedRowsCount(0L) + .minSequenceNumber(7L) + .dv(ByteBuffer.wrap(MANIFEST_DV)) + .dvCardinality(4L) + .build(); + @Test void dataFileAdapterDelegation() { TrackingStruct tracking = @@ -334,6 +366,213 @@ void dvDeleteFileAdapterRejectsNullDeletionVector() { .hasMessage("Cannot create DV delete file: no deletion vector"); } + @ParameterizedTest + @EnumSource( + value = FileContent.class, + names = {"DATA_MANIFEST", "DELETE_MANIFEST"}) + void manifestFileAdapterDelegation(FileContent contentType) { + ByteBuffer keyMetadata = ByteBuffer.wrap(new byte[] {7, 8, 9}); + TrackedFile file = + new TrackedFileStruct( + MANIFEST_TRACKING, + contentType, + FORMAT_VERSION_V4, + MANIFEST_FILE_LOCATION, + FileFormat.PARQUET, + 0L, + MANIFEST_FILE_SIZE, + null, + null, + null, + null, + null, + MANIFEST_INFO, + keyMetadata, + null, + null); + + ManifestFile manifest = TrackedFileAdapters.asManifestFile(file); + + ManifestContent expectedContent = + contentType == FileContent.DATA_MANIFEST ? ManifestContent.DATA : ManifestContent.DELETES; + assertThat(manifest.path()).isEqualTo(MANIFEST_FILE_LOCATION); + assertThat(manifest.length()).isEqualTo(MANIFEST_FILE_SIZE); + assertThat(manifest.content()).isEqualTo(expectedContent); + assertThat(manifest.sequenceNumber()).isEqualTo(DATA_SEQUENCE_NUMBER); + assertThat(manifest.minSequenceNumber()).isEqualTo(MANIFEST_INFO.minSequenceNumber()); + assertThat(manifest.snapshotId()).isEqualTo(SNAPSHOT_ID); + assertThat(manifest.addedFilesCount()).isEqualTo(MANIFEST_INFO.addedFilesCount()); + assertThat(manifest.addedRowsCount()).isEqualTo(MANIFEST_INFO.addedRowsCount()); + assertThat(manifest.existingFilesCount()).isEqualTo(MANIFEST_INFO.existingFilesCount()); + assertThat(manifest.existingRowsCount()).isEqualTo(MANIFEST_INFO.existingRowsCount()); + assertThat(manifest.deletedFilesCount()).isEqualTo(MANIFEST_INFO.deletedFilesCount()); + assertThat(manifest.deletedRowsCount()).isEqualTo(MANIFEST_INFO.deletedRowsCount()); + assertThat(manifest.firstRowId()).isEqualTo(FIRST_ROW_ID); + assertThat(manifest.keyMetadata()).isEqualTo(keyMetadata); + assertThat(manifest.deletionVector()).isEqualTo(ByteBuffer.wrap(MANIFEST_DV)); + assertThat(manifest.partitions()).isNull(); + } + + @Test + void manifestFileAdapterPartitionSpecIdUnsupported() { + TrackedFile file = + new TrackedFileStruct( + MANIFEST_TRACKING, + FileContent.DATA_MANIFEST, + FORMAT_VERSION_V4, + MANIFEST_FILE_LOCATION, + FileFormat.PARQUET, + 0L, + MANIFEST_FILE_SIZE, + null, + null, + null, + null, + null, + MANIFEST_INFO, + null, + null, + null); + + ManifestFile manifest = TrackedFileAdapters.asManifestFile(file); + + assertThatThrownBy(manifest::partitionSpecId) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("v4 manifests are not bound to a single partition spec"); + } + + @Test + void manifestFileAdapterCopy() { + ByteBuffer keyMetadata = ByteBuffer.wrap(new byte[] {7, 8, 9}); + TrackedFile file = + new TrackedFileStruct( + MANIFEST_TRACKING, + FileContent.DATA_MANIFEST, + FORMAT_VERSION_V4, + MANIFEST_FILE_LOCATION, + FileFormat.PARQUET, + 0L, + MANIFEST_FILE_SIZE, + null, + null, + null, + null, + null, + MANIFEST_INFO, + keyMetadata, + null, + null); + + ManifestFile original = TrackedFileAdapters.asManifestFile(file); + ManifestFile copy = original.copy(); + + assertThat(copy.path()).isEqualTo(original.path()); + assertThat(copy.length()).isEqualTo(original.length()); + assertThat(copy.content()).isEqualTo(original.content()); + assertThat(copy.sequenceNumber()).isEqualTo(original.sequenceNumber()); + assertThat(copy.minSequenceNumber()).isEqualTo(original.minSequenceNumber()); + assertThat(copy.snapshotId()).isEqualTo(original.snapshotId()); + assertThat(copy.addedFilesCount()).isEqualTo(original.addedFilesCount()); + assertThat(copy.addedRowsCount()).isEqualTo(original.addedRowsCount()); + assertThat(copy.existingFilesCount()).isEqualTo(original.existingFilesCount()); + assertThat(copy.existingRowsCount()).isEqualTo(original.existingRowsCount()); + assertThat(copy.deletedFilesCount()).isEqualTo(original.deletedFilesCount()); + assertThat(copy.deletedRowsCount()).isEqualTo(original.deletedRowsCount()); + assertThat(copy.firstRowId()).isEqualTo(original.firstRowId()); + assertThat(copy.partitions()).isNull(); + assertThat(copy.keyMetadata()).isEqualTo(original.keyMetadata()); + assertThat(copy.keyMetadata().array()).isNotSameAs(original.keyMetadata().array()); + assertThat(copy.deletionVector()).isEqualTo(original.deletionVector()); + assertThat(copy.deletionVector().array()).isNotSameAs(original.deletionVector().array()); + } + + @ParameterizedTest + @EnumSource( + value = FileContent.class, + mode = EnumSource.Mode.EXCLUDE, + names = {"DATA_MANIFEST", "DELETE_MANIFEST"}) + void manifestFileAdapterRejectsNonManifestContent(FileContent contentType) { + TrackedFileStruct file = dummyTrackedFile(contentType); + + assertThatThrownBy(() -> TrackedFileAdapters.asManifestFile(file)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid content type for ManifestFile: %s", contentType); + } + + @Test + void manifestFileAdapterRejectsNullDataSequenceNumber() { + TrackingStruct tracking = + new TrackingStruct( + EntryStatus.ADDED, + SNAPSHOT_ID, + null, + FILE_SEQUENCE_NUMBER, + null, + FIRST_ROW_ID, + null, + null); + TrackedFile file = + new TrackedFileStruct( + tracking, + FileContent.DATA_MANIFEST, + FORMAT_VERSION_V4, + MANIFEST_FILE_LOCATION, + FileFormat.PARQUET, + 0L, + MANIFEST_FILE_SIZE, + null, + null, + null, + null, + null, + MANIFEST_INFO, + null, + null, + null); + + assertThatThrownBy(() -> TrackedFileAdapters.asManifestFile(file)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid data sequence number: null"); + } + + @Test + void manifestFileAdapterRejectsUnequalSequenceNumbers() { + TrackingStruct tracking = + new TrackingStruct( + EntryStatus.ADDED, + SNAPSHOT_ID, + DATA_SEQUENCE_NUMBER, + FILE_SEQUENCE_NUMBER, + null, + FIRST_ROW_ID, + null, + null); + TrackedFile file = + new TrackedFileStruct( + tracking, + FileContent.DATA_MANIFEST, + FORMAT_VERSION_V4, + MANIFEST_FILE_LOCATION, + FileFormat.PARQUET, + 0L, + MANIFEST_FILE_SIZE, + null, + null, + null, + null, + null, + MANIFEST_INFO, + null, + null, + null); + + assertThatThrownBy(() -> TrackedFileAdapters.asManifestFile(file)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Manifest data and file sequence numbers must be equal, got %s and %s", + DATA_SEQUENCE_NUMBER, FILE_SEQUENCE_NUMBER); + } + @Test void nullContentStatsReturnsNullStats() { TrackedFileStruct file = dummyTrackedFile(FileContent.DATA); From 050f90b2f85f8ca29cd956b1282e060ea4bd7302 Mon Sep 17 00:00:00 2001 From: Anoop Johnson Date: Thu, 3 Sep 2026 09:53:49 -0700 Subject: [PATCH 2/5] PR feedback --- .../apache/iceberg/TrackedFileAdapters.java | 8 - .../iceberg/TestTrackedFileAdapters.java | 139 +++--------------- 2 files changed, 18 insertions(+), 129 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java b/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java index ea2d0f37fc53..29087eb44f6a 100644 --- a/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java +++ b/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java @@ -419,14 +419,6 @@ private static class TrackedManifestFile implements ManifestFile { private final TrackedFile file; private TrackedManifestFile(TrackedFile file) { - Tracking tracking = file.tracking(); - Preconditions.checkArgument( - tracking.dataSequenceNumber() != null, "Invalid data sequence number: null"); - Preconditions.checkArgument( - tracking.dataSequenceNumber().equals(tracking.fileSequenceNumber()), - "Manifest data and file sequence numbers must be equal, got %s and %s", - tracking.dataSequenceNumber(), - tracking.fileSequenceNumber()); this.file = file; } diff --git a/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java b/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java index d059479e64bc..f6909106d72f 100644 --- a/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java +++ b/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java @@ -31,6 +31,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; +import org.mockito.Mockito; class TestTrackedFileAdapters { @@ -38,7 +39,6 @@ class TestTrackedFileAdapters { private static final String MANIFEST_LOCATION = "s3://bucket/table/manifest.parquet"; private static final String DATA_FILE_LOCATION = "s3://bucket/data/file.parquet"; private static final String DV_LOCATION = "s3://bucket/puffin/dv-file.bin"; - private static final String MANIFEST_FILE_LOCATION = "s3://bucket/table/manifest-1.parquet"; private static final long MANIFEST_FILE_SIZE = 2048L; // Tracking values that the delegation tests validate. @@ -100,6 +100,8 @@ class TestTrackedFileAdapters { private static final byte[] MANIFEST_DV = new byte[] {1, 2, 3}; + private static final ByteBuffer MANIFEST_KEY_METADATA = ByteBuffer.wrap(new byte[] {7, 8, 9}); + private static final ManifestInfo MANIFEST_INFO = ManifestInfoStruct.builder() .addedFilesCount(3) @@ -371,13 +373,12 @@ void dvDeleteFileAdapterRejectsNullDeletionVector() { value = FileContent.class, names = {"DATA_MANIFEST", "DELETE_MANIFEST"}) void manifestFileAdapterDelegation(FileContent contentType) { - ByteBuffer keyMetadata = ByteBuffer.wrap(new byte[] {7, 8, 9}); TrackedFile file = new TrackedFileStruct( MANIFEST_TRACKING, contentType, FORMAT_VERSION_V4, - MANIFEST_FILE_LOCATION, + MANIFEST_LOCATION, FileFormat.PARQUET, 0L, MANIFEST_FILE_SIZE, @@ -387,7 +388,7 @@ void manifestFileAdapterDelegation(FileContent contentType) { null, null, MANIFEST_INFO, - keyMetadata, + MANIFEST_KEY_METADATA, null, null); @@ -395,7 +396,7 @@ void manifestFileAdapterDelegation(FileContent contentType) { ManifestContent expectedContent = contentType == FileContent.DATA_MANIFEST ? ManifestContent.DATA : ManifestContent.DELETES; - assertThat(manifest.path()).isEqualTo(MANIFEST_FILE_LOCATION); + assertThat(manifest.path()).isEqualTo(MANIFEST_LOCATION); assertThat(manifest.length()).isEqualTo(MANIFEST_FILE_SIZE); assertThat(manifest.content()).isEqualTo(expectedContent); assertThat(manifest.sequenceNumber()).isEqualTo(DATA_SEQUENCE_NUMBER); @@ -408,7 +409,7 @@ void manifestFileAdapterDelegation(FileContent contentType) { assertThat(manifest.deletedFilesCount()).isEqualTo(MANIFEST_INFO.deletedFilesCount()); assertThat(manifest.deletedRowsCount()).isEqualTo(MANIFEST_INFO.deletedRowsCount()); assertThat(manifest.firstRowId()).isEqualTo(FIRST_ROW_ID); - assertThat(manifest.keyMetadata()).isEqualTo(keyMetadata); + assertThat(manifest.keyMetadata()).isEqualTo(MANIFEST_KEY_METADATA); assertThat(manifest.deletionVector()).isEqualTo(ByteBuffer.wrap(MANIFEST_DV)); assertThat(manifest.partitions()).isNull(); } @@ -420,7 +421,7 @@ void manifestFileAdapterPartitionSpecIdUnsupported() { MANIFEST_TRACKING, FileContent.DATA_MANIFEST, FORMAT_VERSION_V4, - MANIFEST_FILE_LOCATION, + MANIFEST_LOCATION, FileFormat.PARQUET, 0L, MANIFEST_FILE_SIZE, @@ -443,47 +444,17 @@ void manifestFileAdapterPartitionSpecIdUnsupported() { @Test void manifestFileAdapterCopy() { - ByteBuffer keyMetadata = ByteBuffer.wrap(new byte[] {7, 8, 9}); - TrackedFile file = - new TrackedFileStruct( - MANIFEST_TRACKING, - FileContent.DATA_MANIFEST, - FORMAT_VERSION_V4, - MANIFEST_FILE_LOCATION, - FileFormat.PARQUET, - 0L, - MANIFEST_FILE_SIZE, - null, - null, - null, - null, - null, - MANIFEST_INFO, - keyMetadata, - null, - null); + TrackedFile file = Mockito.mock(TrackedFile.class); + TrackedFile fileCopy = Mockito.mock(TrackedFile.class); + Mockito.when(file.contentType()).thenReturn(FileContent.DATA_MANIFEST); + Mockito.when(file.copy()).thenReturn(fileCopy); + Mockito.when(fileCopy.location()).thenReturn(MANIFEST_LOCATION); + + ManifestFile copy = TrackedFileAdapters.asManifestFile(file).copy(); - ManifestFile original = TrackedFileAdapters.asManifestFile(file); - ManifestFile copy = original.copy(); - - assertThat(copy.path()).isEqualTo(original.path()); - assertThat(copy.length()).isEqualTo(original.length()); - assertThat(copy.content()).isEqualTo(original.content()); - assertThat(copy.sequenceNumber()).isEqualTo(original.sequenceNumber()); - assertThat(copy.minSequenceNumber()).isEqualTo(original.minSequenceNumber()); - assertThat(copy.snapshotId()).isEqualTo(original.snapshotId()); - assertThat(copy.addedFilesCount()).isEqualTo(original.addedFilesCount()); - assertThat(copy.addedRowsCount()).isEqualTo(original.addedRowsCount()); - assertThat(copy.existingFilesCount()).isEqualTo(original.existingFilesCount()); - assertThat(copy.existingRowsCount()).isEqualTo(original.existingRowsCount()); - assertThat(copy.deletedFilesCount()).isEqualTo(original.deletedFilesCount()); - assertThat(copy.deletedRowsCount()).isEqualTo(original.deletedRowsCount()); - assertThat(copy.firstRowId()).isEqualTo(original.firstRowId()); - assertThat(copy.partitions()).isNull(); - assertThat(copy.keyMetadata()).isEqualTo(original.keyMetadata()); - assertThat(copy.keyMetadata().array()).isNotSameAs(original.keyMetadata().array()); - assertThat(copy.deletionVector()).isEqualTo(original.deletionVector()); - assertThat(copy.deletionVector().array()).isNotSameAs(original.deletionVector().array()); + // copy() delegates to the tracked file's copy(), which deep-copies the nested structs. + Mockito.verify(file).copy(); + assertThat(copy.path()).isEqualTo(MANIFEST_LOCATION); } @ParameterizedTest @@ -499,80 +470,6 @@ void manifestFileAdapterRejectsNonManifestContent(FileContent contentType) { .hasMessage("Invalid content type for ManifestFile: %s", contentType); } - @Test - void manifestFileAdapterRejectsNullDataSequenceNumber() { - TrackingStruct tracking = - new TrackingStruct( - EntryStatus.ADDED, - SNAPSHOT_ID, - null, - FILE_SEQUENCE_NUMBER, - null, - FIRST_ROW_ID, - null, - null); - TrackedFile file = - new TrackedFileStruct( - tracking, - FileContent.DATA_MANIFEST, - FORMAT_VERSION_V4, - MANIFEST_FILE_LOCATION, - FileFormat.PARQUET, - 0L, - MANIFEST_FILE_SIZE, - null, - null, - null, - null, - null, - MANIFEST_INFO, - null, - null, - null); - - assertThatThrownBy(() -> TrackedFileAdapters.asManifestFile(file)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid data sequence number: null"); - } - - @Test - void manifestFileAdapterRejectsUnequalSequenceNumbers() { - TrackingStruct tracking = - new TrackingStruct( - EntryStatus.ADDED, - SNAPSHOT_ID, - DATA_SEQUENCE_NUMBER, - FILE_SEQUENCE_NUMBER, - null, - FIRST_ROW_ID, - null, - null); - TrackedFile file = - new TrackedFileStruct( - tracking, - FileContent.DATA_MANIFEST, - FORMAT_VERSION_V4, - MANIFEST_FILE_LOCATION, - FileFormat.PARQUET, - 0L, - MANIFEST_FILE_SIZE, - null, - null, - null, - null, - null, - MANIFEST_INFO, - null, - null, - null); - - assertThatThrownBy(() -> TrackedFileAdapters.asManifestFile(file)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage( - "Manifest data and file sequence numbers must be equal, got %s and %s", - DATA_SEQUENCE_NUMBER, FILE_SEQUENCE_NUMBER); - } - @Test void nullContentStatsReturnsNullStats() { TrackedFileStruct file = dummyTrackedFile(FileContent.DATA); From cf93c17805d2d036b57b7d282f18fcb9fe1f1530 Mon Sep 17 00:00:00 2001 From: Anoop Johnson Date: Fri, 4 Sep 2026 13:01:31 -0700 Subject: [PATCH 3/5] Fix PR feedback --- .../java/org/apache/iceberg/ManifestFile.java | 2 +- .../apache/iceberg/TrackedFileAdapters.java | 4 +- .../iceberg/TestTrackedFileAdapters.java | 49 +++++-------------- 3 files changed, 15 insertions(+), 40 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/ManifestFile.java b/api/src/main/java/org/apache/iceberg/ManifestFile.java index 52c90fabc706..ef450f7f9ee0 100644 --- a/api/src/main/java/org/apache/iceberg/ManifestFile.java +++ b/api/src/main/java/org/apache/iceberg/ManifestFile.java @@ -211,7 +211,7 @@ default Long firstRowId() { } /** Returns the manifest deletion vector, or null if absent. */ - default ByteBuffer deletionVector() { + default ByteBuffer manifestDeletionVector() { return null; } diff --git a/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java b/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java index 29087eb44f6a..f9df5eb3eb79 100644 --- a/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java +++ b/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java @@ -446,7 +446,7 @@ public ManifestContent content() { case DELETE_MANIFEST: return ManifestContent.DELETES; default: - throw new IllegalStateException( + throw new UnsupportedOperationException( "Unsupported content type for manifests: " + file.contentType()); } } @@ -512,7 +512,7 @@ public Long firstRowId() { } @Override - public ByteBuffer deletionVector() { + public ByteBuffer manifestDeletionVector() { return file.manifestInfo().dv(); } diff --git a/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java b/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java index f6909106d72f..80025ac0a92d 100644 --- a/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java +++ b/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java @@ -93,10 +93,10 @@ class TestTrackedFileAdapters { // data and file sequence numbers must be equal DATA_SEQUENCE_NUMBER, DATA_SEQUENCE_NUMBER, - /* dvSnapshotId= */ null, + null, // dvSnapshotId FIRST_ROW_ID, - /* deletedPositions= */ null, - /* replacedPositions= */ null); + null, // deletedPositions + null); // replacedPositions private static final byte[] MANIFEST_DV = new byte[] {1, 2, 3}; @@ -380,17 +380,17 @@ void manifestFileAdapterDelegation(FileContent contentType) { FORMAT_VERSION_V4, MANIFEST_LOCATION, FileFormat.PARQUET, - 0L, + 10L, // recordCount MANIFEST_FILE_SIZE, - null, - null, - null, - null, - null, + null, // specId + null, // partition + null, // contentStats + null, // sortOrderId + null, // deletionVector MANIFEST_INFO, MANIFEST_KEY_METADATA, - null, - null); + null, // splitOffsets + null); // equalityIds ManifestFile manifest = TrackedFileAdapters.asManifestFile(file); @@ -410,33 +410,8 @@ void manifestFileAdapterDelegation(FileContent contentType) { assertThat(manifest.deletedRowsCount()).isEqualTo(MANIFEST_INFO.deletedRowsCount()); assertThat(manifest.firstRowId()).isEqualTo(FIRST_ROW_ID); assertThat(manifest.keyMetadata()).isEqualTo(MANIFEST_KEY_METADATA); - assertThat(manifest.deletionVector()).isEqualTo(ByteBuffer.wrap(MANIFEST_DV)); + assertThat(manifest.manifestDeletionVector()).isEqualTo(ByteBuffer.wrap(MANIFEST_DV)); assertThat(manifest.partitions()).isNull(); - } - - @Test - void manifestFileAdapterPartitionSpecIdUnsupported() { - TrackedFile file = - new TrackedFileStruct( - MANIFEST_TRACKING, - FileContent.DATA_MANIFEST, - FORMAT_VERSION_V4, - MANIFEST_LOCATION, - FileFormat.PARQUET, - 0L, - MANIFEST_FILE_SIZE, - null, - null, - null, - null, - null, - MANIFEST_INFO, - null, - null, - null); - - ManifestFile manifest = TrackedFileAdapters.asManifestFile(file); - assertThatThrownBy(manifest::partitionSpecId) .isInstanceOf(UnsupportedOperationException.class) .hasMessage("v4 manifests are not bound to a single partition spec"); From f707d7b2face4c5d6eba0af6c7ff78b406a937cb Mon Sep 17 00:00:00 2001 From: Anoop Johnson Date: Fri, 4 Sep 2026 16:07:58 -0700 Subject: [PATCH 4/5] Introduce ManifestBitmap interface --- .../org/apache/iceberg/ManifestBitmap.java | 33 +++++++++++++++++++ .../java/org/apache/iceberg/ManifestFile.java | 2 +- .../apache/iceberg/TrackedFileAdapters.java | 24 ++++++++++++-- .../iceberg/TestTrackedFileAdapters.java | 5 ++- 4 files changed, 58 insertions(+), 6 deletions(-) create mode 100644 api/src/main/java/org/apache/iceberg/ManifestBitmap.java diff --git a/api/src/main/java/org/apache/iceberg/ManifestBitmap.java b/api/src/main/java/org/apache/iceberg/ManifestBitmap.java new file mode 100644 index 000000000000..f85bd2ae6b12 --- /dev/null +++ b/api/src/main/java/org/apache/iceberg/ManifestBitmap.java @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg; + +import java.nio.ByteBuffer; + +/** A small bitmap that is serialized inline in a metadata file. */ +public interface ManifestBitmap { + /** Number of bits set in this bitmap. */ + int cardinality(); + + /** Return whether the bit at {@code position} is set. */ + boolean isSet(int position); + + /** Return the serialized bitmap as a {@link ByteBuffer}. */ + ByteBuffer buffer(); +} diff --git a/api/src/main/java/org/apache/iceberg/ManifestFile.java b/api/src/main/java/org/apache/iceberg/ManifestFile.java index ef450f7f9ee0..b831c2059190 100644 --- a/api/src/main/java/org/apache/iceberg/ManifestFile.java +++ b/api/src/main/java/org/apache/iceberg/ManifestFile.java @@ -211,7 +211,7 @@ default Long firstRowId() { } /** Returns the manifest deletion vector, or null if absent. */ - default ByteBuffer manifestDeletionVector() { + default ManifestBitmap manifestDeletionVector() { return null; } diff --git a/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java b/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java index 210590f3d3b2..98cf78088702 100644 --- a/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java +++ b/core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java @@ -522,8 +522,28 @@ public Long firstRowId() { } @Override - public ByteBuffer manifestDeletionVector() { - return file.manifestInfo().dv(); + public ManifestBitmap manifestDeletionVector() { + ByteBuffer dv = file.manifestInfo().dv(); + if (dv == null) { + return null; + } + + return new ManifestBitmap() { + @Override + public int cardinality() { + throw new UnsupportedOperationException("Bitmap decoding has not been implemented"); + } + + @Override + public boolean isSet(int position) { + throw new UnsupportedOperationException("Bitmap decoding has not been implemented"); + } + + @Override + public ByteBuffer buffer() { + return dv; + } + }; } @Override diff --git a/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java b/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java index f1203fd9f545..646407fae356 100644 --- a/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java +++ b/core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java @@ -98,9 +98,8 @@ class TestTrackedFileAdapters { new TrackingStruct( EntryStatus.ADDED, SNAPSHOT_ID, - // data and file sequence numbers must be equal - DATA_SEQUENCE_NUMBER, DATA_SEQUENCE_NUMBER, + FILE_SEQUENCE_NUMBER, null, // dvSnapshotId FIRST_ROW_ID, null, // deletedPositions @@ -426,7 +425,7 @@ void manifestFileAdapterDelegation(FileContent contentType) { assertThat(manifest.deletedRowsCount()).isEqualTo(MANIFEST_INFO.deletedRowsCount()); assertThat(manifest.firstRowId()).isEqualTo(FIRST_ROW_ID); assertThat(manifest.keyMetadata()).isEqualTo(MANIFEST_KEY_METADATA); - assertThat(manifest.manifestDeletionVector()).isEqualTo(ByteBuffer.wrap(MANIFEST_DV)); + assertThat(manifest.manifestDeletionVector().buffer()).isEqualTo(ByteBuffer.wrap(MANIFEST_DV)); assertThat(manifest.partitions()).isNull(); assertThatThrownBy(manifest::partitionSpecId) .isInstanceOf(UnsupportedOperationException.class) From 133f3c544f0eb549fb278ecb8f3808ac8de9d5e7 Mon Sep 17 00:00:00 2001 From: Anoop Johnson Date: Fri, 4 Sep 2026 16:20:53 -0700 Subject: [PATCH 5/5] tweak comment --- api/src/main/java/org/apache/iceberg/ManifestBitmap.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/iceberg/ManifestBitmap.java b/api/src/main/java/org/apache/iceberg/ManifestBitmap.java index f85bd2ae6b12..a6a41af41479 100644 --- a/api/src/main/java/org/apache/iceberg/ManifestBitmap.java +++ b/api/src/main/java/org/apache/iceberg/ManifestBitmap.java @@ -20,7 +20,7 @@ import java.nio.ByteBuffer; -/** A small bitmap that is serialized inline in a metadata file. */ +/** A bitmap that is serialized inline in a metadata file. */ public interface ManifestBitmap { /** Number of bits set in this bitmap. */ int cardinality();