API, Core: Remove deprecated methods from Snapshot API, backport #573…

…4 (#5875)
linkedin · Sep 29, 2022 · 737474b · 737474b
1 parent 9aa92a5
commit 737474b
Show file tree

Hide file tree

Showing 40 changed files with 509 additions and 394 deletions.
diff --git a/.palantir/revapi.yml b/.palantir/revapi.yml
@@ -1,11 +1,30 @@
 versionOverrides:
-  org.apache.iceberg:iceberg-api:release-base-0.13.0: "0.13.0"
+  org.apache.iceberg:iceberg-api:apache-iceberg-0.14.0: "0.14.0"
 acceptedBreaks:
   apache-iceberg-0.14.0:
     org.apache.iceberg:iceberg-api:
+    - code: "java.class.defaultSerializationChanged"
+      old: "class org.apache.iceberg.PartitionKey"
+      new: "class org.apache.iceberg.PartitionKey"
+      justification: "Serialization across versions is not supported"
     - code: "java.class.removed"
       old: "interface org.apache.iceberg.Rollback"
       justification: "Deprecations for 1.0 release"
+    - code: "java.method.removed"
+      old: "method java.lang.Iterable<org.apache.iceberg.DataFile> org.apache.iceberg.Snapshot::addedFiles()"
+      justification: "Deprecations for 1.0 release"
+    - code: "java.method.removed"
+      old: "method java.util.List<org.apache.iceberg.ManifestFile> org.apache.iceberg.Snapshot::allManifests()"
+      justification: "Deprecations for 1.0 release"
+    - code: "java.method.removed"
+      old: "method java.util.List<org.apache.iceberg.ManifestFile> org.apache.iceberg.Snapshot::dataManifests()"
+      justification: "Deprecations for 1.0 release"
+    - code: "java.method.removed"
+      old: "method java.util.List<org.apache.iceberg.ManifestFile> org.apache.iceberg.Snapshot::deleteManifests()"
+      justification: "Deprecations for 1.0 release"
+    - code: "java.method.removed"
+      old: "method java.lang.Iterable<org.apache.iceberg.DataFile> org.apache.iceberg.Snapshot::deletedFiles()"
+      justification: "Deprecations for 1.0 release"
     - code: "java.method.removed"
       old: "method org.apache.iceberg.OverwriteFiles org.apache.iceberg.OverwriteFiles::validateNoConflictingAppends(org.apache.iceberg.expressions.Expression)"
       justification: "Deprecations for 1.0 release"

diff --git a/api/src/main/java/org/apache/iceberg/Snapshot.java b/api/src/main/java/org/apache/iceberg/Snapshot.java
@@ -64,16 +64,6 @@ public interface Snapshot extends Serializable {
    */
   long timestampMillis();
 
-  /**
-   * Return all {@link ManifestFile} instances for either data or delete manifests in this snapshot.
-   *
-   * @return a list of ManifestFile
-   * @deprecated since 0.14.0, will be removed in 1.0.0; Use {@link Snapshot#allManifests(FileIO)}
-   *     instead.
-   */
-  @Deprecated
-  List<ManifestFile> allManifests();
-
   /**
    * Return all {@link ManifestFile} instances for either data or delete manifests in this snapshot.
    *
@@ -82,16 +72,6 @@ public interface Snapshot extends Serializable {
    */
   List<ManifestFile> allManifests(FileIO io);
 
-  /**
-   * Return a {@link ManifestFile} for each data manifest in this snapshot.
-   *
-   * @return a list of ManifestFile
-   * @deprecated since 0.14.0, will be removed in 1.0.0; Use {@link Snapshot#dataManifests(FileIO)}
-   *     instead.
-   */
-  @Deprecated
-  List<ManifestFile> dataManifests();
-
   /**
    * Return a {@link ManifestFile} for each data manifest in this snapshot.
    *
@@ -100,16 +80,6 @@ public interface Snapshot extends Serializable {
    */
   List<ManifestFile> dataManifests(FileIO io);
 
-  /**
-   * Return a {@link ManifestFile} for each delete manifest in this snapshot.
-   *
-   * @return a list of ManifestFile
-   * @deprecated since 0.14.0, will be removed in 1.0.0; Use {@link
-   *     Snapshot#deleteManifests(FileIO)} instead.
-   */
-  @Deprecated
-  List<ManifestFile> deleteManifests();
-
   /**
    * Return a {@link ManifestFile} for each delete manifest in this snapshot.
    *
@@ -133,19 +103,6 @@ public interface Snapshot extends Serializable {
    */
   Map<String, String> summary();
 
-  /**
-   * Return all data files added to the table in this snapshot.
-   *
-   * <p>The files returned include the following columns: file_path, file_format, partition,
-   * record_count, and file_size_in_bytes. Other columns will be null.
-   *
-   * @return all data files added to the table in this snapshot.
-   * @deprecated since 0.14.0, will be removed in 1.0.0; Use {@link Snapshot#addedDataFiles(FileIO)}
-   *     instead.
-   */
-  @Deprecated
-  Iterable<DataFile> addedFiles();
-
   /**
    * Return all data files added to the table in this snapshot.
    *
@@ -157,19 +114,6 @@ public interface Snapshot extends Serializable {
    */
   Iterable<DataFile> addedDataFiles(FileIO io);
 
-  /**
-   * Return all data files deleted from the table in this snapshot.
-   *
-   * <p>The files returned include the following columns: file_path, file_format, partition,
-   * record_count, and file_size_in_bytes. Other columns will be null.
-   *
-   * @return all data files deleted from the table in this snapshot.
-   * @deprecated since 0.14.0, will be removed in 1.0.0; Use {@link
-   *     Snapshot#removedDataFiles(FileIO)} instead.
-   */
-  @Deprecated
-  Iterable<DataFile> deletedFiles();
-
   /**
    * Return all data files removed from the table in this snapshot.
    *

diff --git a/core/src/main/java/org/apache/iceberg/BaseIncrementalAppendScan.java b/core/src/main/java/org/apache/iceberg/BaseIncrementalAppendScan.java
@@ -174,7 +174,7 @@ private CloseableIterable<FileScanTask> appendFilesFromSnapshots(List<Snapshot>
     Set<Long> snapshotIds = Sets.newHashSet(Iterables.transform(snapshots, Snapshot::snapshotId));
     Set<ManifestFile> manifests =
         FluentIterable.from(snapshots)
-            .transformAndConcat(Snapshot::dataManifests)
+            .transformAndConcat(snapshot -> snapshot.dataManifests(table().io()))
             .filter(manifestFile -> snapshotIds.contains(manifestFile.snapshotId()))
             .toSet();
 

diff --git a/core/src/main/java/org/apache/iceberg/BaseSnapshot.java b/core/src/main/java/org/apache/iceberg/BaseSnapshot.java
@@ -36,12 +36,6 @@
 class BaseSnapshot implements Snapshot {
   private static final long INITIAL_SEQUENCE_NUMBER = 0;
 
-  /**
-   * @deprecated since 0.14.0, will be removed in 1.0.0; {@link FileIO} should be passed to methods
-   *     which require it
-   */
-  @Deprecated private final FileIO io;
-
   private final long snapshotId;
   private final Long parentId;
   private final long sequenceNumber;
@@ -50,6 +44,7 @@ class BaseSnapshot implements Snapshot {
   private final String operation;
   private final Map<String, String> summary;
   private final Integer schemaId;
+  private final String[] v1ManifestLocations;
 
   // lazily initialized
   private transient List<ManifestFile> allManifests = null;
@@ -60,23 +55,7 @@ class BaseSnapshot implements Snapshot {
   private transient List<DeleteFile> addedDeleteFiles = null;
   private transient List<DeleteFile> removedDeleteFiles = null;
 
-  /** For testing only. */
-  BaseSnapshot(FileIO io, long snapshotId, Integer schemaId, String... manifestFiles) {
-    this(
-        io,
-        snapshotId,
-        null,
-        System.currentTimeMillis(),
-        null,
-        null,
-        schemaId,
-        Lists.transform(
-            Arrays.asList(manifestFiles),
-            path -> new GenericManifestFile(io.newInputFile(path), 0)));
-  }
-
   BaseSnapshot(
-      FileIO io,
       long sequenceNumber,
       long snapshotId,
       Long parentId,
@@ -85,7 +64,6 @@ class BaseSnapshot implements Snapshot {
       Map<String, String> summary,
       Integer schemaId,
       String manifestList) {
-    this.io = io;
     this.sequenceNumber = sequenceNumber;
     this.snapshotId = snapshotId;
     this.parentId = parentId;
@@ -94,30 +72,10 @@ class BaseSnapshot implements Snapshot {
     this.summary = summary;
     this.schemaId = schemaId;
     this.manifestListLocation = manifestList;
+    this.v1ManifestLocations = null;
   }
 
   BaseSnapshot(
-      long sequenceNumber,
-      long snapshotId,
-      Long parentId,
-      long timestampMillis,
-      String operation,
-      Map<String, String> summary,
-      Integer schemaId,
-      String manifestList) {
-    this.io = null;
-    this.sequenceNumber = sequenceNumber;
-    this.snapshotId = snapshotId;
-    this.parentId = parentId;
-    this.timestampMillis = timestampMillis;
-    this.operation = operation;
-    this.summary = summary;
-    this.schemaId = schemaId;
-    this.manifestListLocation = manifestList;
-  }
-
-  BaseSnapshot(
-      FileIO io,
       long snapshotId,
       Long parentId,
       long timestampMillis,
@@ -126,18 +84,37 @@ class BaseSnapshot implements Snapshot {
       Integer schemaId,
       List<ManifestFile> dataManifests) {
     this(
-        io,
         INITIAL_SEQUENCE_NUMBER,
         snapshotId,
         parentId,
         timestampMillis,
         operation,
         summary,
         schemaId,
-        null);
+        dataManifests.stream().map(ManifestFile::path).toArray(String[]::new));
     this.allManifests = dataManifests;
   }
 
+  BaseSnapshot(
+      long sequenceNumber,
+      long snapshotId,
+      Long parentId,
+      long timestampMillis,
+      String operation,
+      Map<String, String> summary,
+      Integer schemaId,
+      String[] v1ManifestLocations) {
+    this.sequenceNumber = sequenceNumber;
+    this.snapshotId = snapshotId;
+    this.parentId = parentId;
+    this.timestampMillis = timestampMillis;
+    this.operation = operation;
+    this.summary = summary;
+    this.schemaId = schemaId;
+    this.manifestListLocation = null;
+    this.v1ManifestLocations = v1ManifestLocations;
+  }
+
   @Override
   public long sequenceNumber() {
     return sequenceNumber;
@@ -178,6 +155,14 @@ private void cacheManifests(FileIO fileIO) {
       throw new IllegalArgumentException("Cannot cache changes: FileIO is null");
     }
 
+    if (allManifests == null && v1ManifestLocations != null) {
+      // if we have a collection of manifest locations, then we need to load them here
+      allManifests =
+          Lists.transform(
+              Arrays.asList(v1ManifestLocations),
+              location -> new GenericManifestFile(fileIO.newInputFile(location), 0));
+    }
+
     if (allManifests == null) {
       // if manifests isn't set, then the snapshotFile is set and should be read to get the list
       this.allManifests = ManifestLists.read(fileIO.newInputFile(manifestListLocation));
@@ -203,19 +188,6 @@ public List<ManifestFile> allManifests(FileIO fileIO) {
     return allManifests;
   }
 
-  /**
-   * @deprecated since 0.14.0, will be removed in 1.0.0; Use {@link Snapshot#allManifests(FileIO)}
-   *     instead.
-   */
-  @Override
-  @Deprecated
-  public List<ManifestFile> allManifests() {
-    if (allManifests == null) {
-      cacheManifests(io);
-    }
-    return allManifests;
-  }
-
   @Override
   public List<ManifestFile> dataManifests(FileIO fileIO) {
     if (dataManifests == null) {
@@ -224,19 +196,6 @@ public List<ManifestFile> dataManifests(FileIO fileIO) {
     return dataManifests;
   }
 
-  /**
-   * @deprecated since 0.14.0, will be removed in 1.0.0; Use {@link Snapshot#dataManifests(FileIO)}
-   *     instead.
-   */
-  @Override
-  @Deprecated
-  public List<ManifestFile> dataManifests() {
-    if (dataManifests == null) {
-      cacheManifests(io);
-    }
-    return dataManifests;
-  }
-
   @Override
   public List<ManifestFile> deleteManifests(FileIO fileIO) {
     if (deleteManifests == null) {
@@ -245,19 +204,6 @@ public List<ManifestFile> deleteManifests(FileIO fileIO) {
     return deleteManifests;
   }
 
-  /**
-   * @deprecated since 0.14.0, will be removed in 1.0.0; Use {@link
-   *     Snapshot#deleteManifests(FileIO)} instead.
-   */
-  @Override
-  @Deprecated
-  public List<ManifestFile> deleteManifests() {
-    if (deleteManifests == null) {
-      cacheManifests(io);
-    }
-    return deleteManifests;
-  }
-
   @Override
   public List<DataFile> addedDataFiles(FileIO fileIO) {
     if (addedDataFiles == null) {
@@ -266,19 +212,6 @@ public List<DataFile> addedDataFiles(FileIO fileIO) {
     return addedDataFiles;
   }
 
-  /**
-   * @deprecated since 0.14.0, will be removed in 1.0.0; Use {@link Snapshot#addedDataFiles(FileIO)}
-   *     instead.
-   */
-  @Override
-  @Deprecated
-  public List<DataFile> addedFiles() {
-    if (addedDataFiles == null) {
-      cacheDataFileChanges(io);
-    }
-    return addedDataFiles;
-  }
-
   @Override
   public List<DataFile> removedDataFiles(FileIO fileIO) {
     if (removedDataFiles == null) {
@@ -287,19 +220,6 @@ public List<DataFile> removedDataFiles(FileIO fileIO) {
     return removedDataFiles;
   }
 
-  /**
-   * @deprecated since 0.14.0, will be removed in 1.0.0; Use {@link
-   *     Snapshot#removedDataFiles(FileIO)} instead.
-   */
-  @Override
-  @Deprecated
-  public List<DataFile> deletedFiles() {
-    if (removedDataFiles == null) {
-      cacheDataFileChanges(io);
-    }
-    return removedDataFiles;
-  }
-
   @Override
   public Iterable<DeleteFile> addedDeleteFiles(FileIO fileIO) {
     if (addedDeleteFiles == null) {