API, CORE, Flink, Spark: Deprecate Snapshot Change Methods by RussellSpitzer · Pull Request #15241 · apache/iceberg

RussellSpitzer · 2026-02-05T18:45:17Z

Context

This is part of the write metadata with columnar formats change. When we start writing parquet manifests, calls to ManifestReader() without passing through the partitionSpecByID will error out. The reason for the error is that we have no way of reading the metadata in the new file APIs and we have decided we aren't going add it in the future.

Snapshot.addedFiles and it's friends are some of the main users of this ManifestRead(path, IO) path (that aren't test code) so we need to remove those methods and switch our usage to a version which passes through partitionSpecByID. Otherwise switching to parquet manifests will cause issues throughout the codebase.

See #13769

This PR

We currently offer several methods for getting files changed in a snapshot but they rely on the assumption that you can read the partition_spec from the manifest metadata. In advance of the move to Parquet Manifest, we'll be no longer able to rely on this part of the manifest read code.

In this PR we deprecate those existing methods and create a new utility class which can do the same thing as the old Snapshot methods. The new utility class does not assume that the manifest read code can actually read the partition_spec info and instead takes it as an arguement.

Production Code Changes

Core

CherryPickOperation
MicroBatches

Flink

TableChange

Spark

MicroBatchStream

Test Changes

Unfortunately there are also a huge number of test usages of these methods, the majority of this commit is cleaning those up.

As a disclaimer, I did use Cursor and Claude code when writing this PR, It did the majority of the test refactoring although I have checked them all as well.

We currently offer several methods for getting files changed in a snapshot but they rely on the assumption that you can read the partition_spec from the manifest metadata. In advance of the move to Parquet Manifest, we'll be no longer able to rely on this part of the manifest read code. In this PR we deprecate those existing methods and create a new utility class which can do the same thing as the old Snapshot methods. The new utility class does not assume that the manifest read code can actually read the partition_spec info and instead takes it as an arguement. In production code there are only a small number of actual uses 1. CherryPickOperation 2. MicroBatches Within our other modules we also had a few usages Flink 1. TableChange Spark 2. MicroBatchStream Unfortuantely there are also a huge number of test usages of these methods, the majority of this commit is cleaning those up.

RussellSpitzer · 2026-02-05T18:47:03Z

api/src/main/java/org/apache/iceberg/Snapshot.java

@@ -112,7 +112,11 @@ public interface Snapshot extends Serializable {
   *
   * @param io a {@link FileIO} instance used for reading files from storage
   * @return all data files added to the table in this snapshot.


The Main Deprecations are Here

RussellSpitzer · 2026-02-05T18:48:20Z

core/src/main/java/org/apache/iceberg/CherryPickOperation.java

      // Pick modifications from the snapshot
-      for (DataFile addedFile : cherrypickSnapshot.addedDataFiles(io)) {
+      SnapshotFileChanges changes =
+          SnapshotFileChanges.builder(cherrypickSnapshot, io, specsById).build();


We technically don't have to use the cached utility here, but we use it in the other usage within this file so I thought it was a bit clearer this way.

This looks reasonable to me. Cached or not is the internal impl of SnapshotFileChanges

RussellSpitzer · 2026-02-05T18:49:06Z

core/src/main/java/org/apache/iceberg/CherryPickOperation.java

      failMissingDeletePaths();

-      // copy adds from the picked snapshot
+      // copy adds and deletes from the picked snapshot


Suggested change

// copy adds and deletes from the picked snapshot

// copy adds from the picked snapshot

RussellSpitzer · 2026-02-05T18:51:44Z

core/src/main/java/org/apache/iceberg/SnapshotFileChanges.java

@@ -0,0 +1,276 @@
+/*


This is the actual Utility we are switching to

With V4 metadata change, the change detection can be more complicated with manfest DVs. I like this direction of moving the change detection out of the Snapshot class, which can just focus on core data structures.

This could be a good foundation for the change detection in the V4 adaptive tree.

In V4, if we are going to colocate DV (deleted old and added new) and data file, it might make sense to expose a combined result. Otherwise, the associations get split first and then need to be joined again.

RussellSpitzer · 2026-02-05T18:57:00Z

core/src/main/java/org/apache/iceberg/SnapshotFileChanges.java

+   * @param specsById a map of partition spec IDs to partition specs
+   * @return a new Builder
+   */
+  public static Builder builder(


I think we may want to in the future extend this to take multiple snapshots so ti may make sense to break the api into a "specs,io" and "snapshot" seperately, but not now

RussellSpitzer · 2026-02-05T18:58:41Z

core/src/main/java/org/apache/iceberg/SnapshotFileChanges.java

+  }
+
+  private void cacheDataFileChanges() {
+    List<ManifestFile> changedManifests =


I changed the logic from the BaseSnapshot implementation to optionally use an ExecutorService. I want to save actually using that for a followup PR but I think it was probably a mistake that we had this single threaded before.

RussellSpitzer · 2026-02-05T19:00:42Z

core/src/main/java/org/apache/iceberg/SnapshotFileChanges.java

+  }
+
+  private void cacheDeleteFileChanges() {
+    List<ManifestFile> changedManifests =


Similar to above this differs from the BaseSnapshot impl by using an optional executor service

RussellSpitzer · 2026-02-05T19:02:14Z

core/src/main/java/org/apache/iceberg/SnapshotFileChanges.java

+  }
+
+  public static class Builder {
+    private final Snapshot snapshot;


I am attempting to hid some of the constructor details from the Changes Class with this builder, but that may be premature.

RussellSpitzer · 2026-02-05T19:03:35Z

core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java


    return metadata.snapshot(ref.snapshotId());
  }
+


A set of static methods which allow for getting just one element of the Changes class without actually constructing it. We can drop these as well but I think they make the test code refactor a bit smaller.

RussellSpitzer · 2026-02-05T19:19:09Z

core/src/main/java/org/apache/iceberg/MicroBatches.java

@@ -31,6 +31,7 @@
 import org.apache.iceberg.relocated.com.google.common.collect.Iterables;


Another production code change here

RussellSpitzer · 2026-02-05T19:21:07Z

core/src/test/java/org/apache/iceberg/util/TestSnapshotUtil.java

    assertThat(table.schema().asStruct()).isEqualTo(expected.asStruct());
    assertThat(SnapshotUtil.schemaFor(table, tag).asStruct()).isEqualTo(initialSchema.asStruct());
  }
+


Actual Tests for the Utility

RussellSpitzer · 2026-02-05T19:26:38Z

flink/v1.20/flink/src/main/java/org/apache/iceberg/flink/maintenance/operator/TableChange.java


-  TableChange(Snapshot snapshot, FileIO io) {
-    this(snapshot.addedDataFiles(io), snapshot.addedDeleteFiles(io));
+  TableChange(Snapshot snapshot, Table table) {


This is a bit of a messier change, The old version called a (this) but we can't do that with the new Caching object since we can't init and access it twice.

So the logic for doing the actual counts had to be moved into this method so I could take advantage of the cache and not change performance.

this code is for the old Flink source, which is deprecated since 1.7.0. It is fine if we copy the code a little bit

Claude Opus 4.5 suggestion (after some massaging)

TableChange(Snapshot snapshot, Table table) { this(SnapshotFileChanges.builder(snapshot, table.io(), table.specs()).build()); } private TableChange(SnapshotFileChanges changes) { this(changes.addedDataFiles(), changes.addedDeleteFiles()); }

@stevenzwu: This is live code for Flink table maintenance

Ah good call, That's a lot cleaner I like it

@pvary you are right. When I saw MonitorSource, I thought it was for the old deprecated FlinkSource

RussellSpitzer · 2026-02-05T19:27:07Z

spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java

    // If snapshotSummary doesn't have SnapshotSummary.ADDED_FILES_PROP,
    // iterate through addedFiles iterator to find addedFilesCount.
    return addedFilesCount == -1
-        ? Iterables.size(snapshot.addedDataFiles(table.io()))


This is the only Spark Usage, pretty straight forward fix

RussellSpitzer · 2026-02-05T19:34:24Z

If it's easier for reviewers, I can also split this into "deprecate and utility" and then do all the production and test updates in a follow up PR. I just bundled this all up so that we would be able to avoid having a build that has active deprecation warnings.

stevenzwu · 2026-02-06T00:14:54Z

core/src/main/java/org/apache/iceberg/SnapshotFileChanges.java

@@ -0,0 +1,276 @@
+/*


With V4 metadata change, the change detection can be more complicated with manfest DVs. I like this direction of moving the change detection out of the Snapshot class, which can just focus on core data structures.

This could be a good foundation for the change detection in the V4 adaptive tree.

In V4, if we are going to colocate DV (deleted old and added new) and data file, it might make sense to expose a combined result. Otherwise, the associations get split first and then need to be joined again.

stevenzwu · 2026-02-06T00:19:50Z

core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java

+   * @param snapshot the snapshot to detect changes for
+   * @return all data files added to the table in this snapshot
+   */
+  public static Iterable<DataFile> addedDataFiles(Table table, Snapshot snapshot) {


I am wondering if it is necessary to have this overload of Table arg. maybe just have the version that takes (Snapshot, FileIO, Map<Integer, PartitionSpec>). I wouldn't mind if the unit test change add an extra arg.

+1

It is very easy to call the others if table is available.

Fair enough, I can remove it

stevenzwu · 2026-02-06T00:21:59Z

core/src/main/java/org/apache/iceberg/CherryPickOperation.java

      // Pick modifications from the snapshot
-      for (DataFile addedFile : cherrypickSnapshot.addedDataFiles(io)) {
+      SnapshotFileChanges changes =
+          SnapshotFileChanges.builder(cherrypickSnapshot, io, specsById).build();


This looks reasonable to me. Cached or not is the internal impl of SnapshotFileChanges

stevenzwu · 2026-02-06T00:38:51Z

core/src/main/java/org/apache/iceberg/SnapshotFileChanges.java

+ * query multiple file change types for the same snapshot. By default, manifests are read
+ * sequentially. Use {@link Builder#executeWith(ExecutorService)} to enable parallel reading.
+ */
+public class SnapshotFileChanges {


can we keep this class as package private to start with? SnapshotUtil is in a diff package which prevents this.

Not sure if ChangelogUtil is a better place. Or maybe add a new ChangeDetectionUtil?

stevenzwu · 2026-02-06T00:45:02Z

core/src/test/java/org/apache/iceberg/util/TestSnapshotUtil.java

+    Snapshot snapshotWithAddedFile = table.currentSnapshot();
+
+    // Test the utility method with table
+    Iterable<DataFile> filesFromTableUtil =


nit: the variable name is a bit misleading. it is more like filesFromTableParam, because there is actually a class called TableUtil. I also made another comment that we may not need to have the overload that takes a Table arg.

core/src/test/java/org/apache/iceberg/util/TestSnapshotUtil.java

stevenzwu · 2026-02-06T00:58:14Z

flink/v1.20/flink/src/main/java/org/apache/iceberg/flink/maintenance/operator/TableChange.java


-  TableChange(Snapshot snapshot, FileIO io) {
-    this(snapshot.addedDataFiles(io), snapshot.addedDeleteFiles(io));
+  TableChange(Snapshot snapshot, Table table) {


this code is for the old Flink source, which is deprecated since 1.7.0. It is fine if we copy the code a little bit

pvary · 2026-02-06T09:53:17Z

core/src/main/java/org/apache/iceberg/SnapshotFileChanges.java

+            Iterables.filter(
+                Iterables.filter(
+                    snapshot.allManifests(io),
+                    manifest -> manifest.content() == ManifestContent.DATA),
+                manifest -> Objects.equals(manifest.snapshotId(), snapshot.snapshotId())));


nit: why two filters? Could we combine them?

Just being lazy and mimicking the BaseSnapshot code, Let me tighten that up

pvary · 2026-02-06T09:53:49Z

core/src/main/java/org/apache/iceberg/SnapshotFileChanges.java

+    }
+    return addedDataFiles;


We need a prompt for the AIs to add newlines 🤖

We can do a "agents.md" i know a bunch of other projects have done this

Although if we really want this to not be an issue we need to encode it in our style rules

pvary · 2026-02-06T09:56:36Z

core/src/main/java/org/apache/iceberg/SnapshotFileChanges.java

+  }
+
+  /** Returns all data files removed from the table in this snapshot. */
+  public Iterable<DataFile> removedDataFiles() {


Do we want Iterable or Iterator?
Iterator could be a bit more flexible, if we end up in a situation where the data don't fit into memory, but we might end up duplicating data.

Minimally we should return immutable results

Currently i'm just mimicing the old signature, but your right it make be nice to change to interator although our current implementation is just iterable. I'm wondering for "iterator" invocations if we should change the implementation of the helper methods I added to SnapshotUtil since this class is basically for caching.

RussellSpitzer · 2026-02-06T15:38:43Z

core/src/test/java/org/apache/iceberg/util/TestSnapshotUtil.java

+    assertThat(secondCallResult).hasSize(1);
+
+    // Both calls should return the same reference (cached)
+    assertThat(firstCallResult).isSameAs(secondCallResult);


@stevenzwu Caching test here

wondering if it is clearer to move all the test here (including the SnapshotUtil static methods testing) to a new TestSnapshotFileChanges class

github-actions bot added API spark core flink INFRA labels Feb 5, 2026

RussellSpitzer commented Feb 5, 2026

View reviewed changes

Spotless, Self Review Thoughts

a6bd8c3

RussellSpitzer commented Feb 5, 2026

View reviewed changes

Remove gitignore changes

53d0ea6

github-actions bot removed the INFRA label Feb 5, 2026

Spotless Apply on Everything

a4ab20c

Fix JavaDocs

848a19b

RussellSpitzer requested review from nastra, pvary and stevenzwu and removed request for pvary and stevenzwu February 5, 2026 19:42

RussellSpitzer requested review from pvary and stevenzwu February 5, 2026 19:43

stevenzwu reviewed Feb 6, 2026

View reviewed changes

pvary reviewed Feb 6, 2026

View reviewed changes

RussellSpitzer commented Feb 6, 2026

View reviewed changes

	// copy adds and deletes from the picked snapshot
	// copy adds from the picked snapshot

		@@ -31,6 +31,7 @@
		import org.apache.iceberg.relocated.com.google.common.collect.Iterables;

Conversation

RussellSpitzer commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

This PR

Production Code Changes

Core

Flink

Spark

Test Changes

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!

RussellSpitzer Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

RussellSpitzer commented Feb 5, 2026

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!

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!

RussellSpitzer commented Feb 5, 2026 •

edited

Loading

RussellSpitzer Feb 5, 2026 •

edited

Loading

stevenzwu Feb 6, 2026 •

edited

Loading