OpenAPI: Deprecate snapshot-id of SetStatisticsUpdate (apache#12010)

* Deprecate snapshot-id of SetStatisticsUpdate * update rest spec py * Update api/src/main/java/org/apache/iceberg/UpdateStatistics.java Co-authored-by: Eduard Tudenhoefner <etudenhoefner@gmail.com> * Fix typo, remove revapi change * Address comments * spotless * extend docstring --------- Co-authored-by: Eduard Tudenhoefner <etudenhoefner@gmail.com>
swapna267 · Jan 27, 2025 · b7de28e · b7de28e
1 parent e370888
commit b7de28e
Show file tree

Hide file tree

Showing 8 changed files with 64 additions and 11 deletions.
diff --git a/api/src/main/java/org/apache/iceberg/UpdateStatistics.java b/api/src/main/java/org/apache/iceberg/UpdateStatistics.java
@@ -27,9 +27,22 @@ public interface UpdateStatistics extends PendingUpdate<List<StatisticsFile>> {
    * the snapshot if any exists.
    *
    * @return this for method chaining
+   * @deprecated since 1.8.0, will be removed 1.9.0 or 2.0.0, use {@link
+   *     #setStatistics(StatisticsFile)}.
    */
+  @Deprecated
   UpdateStatistics setStatistics(long snapshotId, StatisticsFile statisticsFile);
 
+  /**
+   * Set the table's statistics file for given snapshot, replacing the previous statistics file for
+   * the snapshot if any exists. The snapshot id of the statistics file will be used.
+   *
+   * @return this for method chaining
+   */
+  default UpdateStatistics setStatistics(StatisticsFile statisticsFile) {
+    throw new UnsupportedOperationException("Setting statistics is not supported");
+  }
+
   /**
    * Remove the table's statistics file for given snapshot.
    *

diff --git a/core/src/main/java/org/apache/iceberg/MetadataUpdate.java b/core/src/main/java/org/apache/iceberg/MetadataUpdate.java
@@ -231,16 +231,25 @@ public void applyTo(TableMetadata.Builder metadataBuilder) {
   }
 
   class SetStatistics implements MetadataUpdate {
-    private final long snapshotId;
     private final StatisticsFile statisticsFile;
 
+    /**
+     * Set statistics for a snapshot.
+     *
+     * @deprecated since 1.8.0, will be removed in 1.9.0 or 2.0.0, use
+     *     SetStatistics(statisticsFile).
+     */
+    @Deprecated
     public SetStatistics(long snapshotId, StatisticsFile statisticsFile) {
-      this.snapshotId = snapshotId;
+      this.statisticsFile = statisticsFile;
+    }
+
+    public SetStatistics(StatisticsFile statisticsFile) {
       this.statisticsFile = statisticsFile;
     }
 
     public long snapshotId() {
-      return snapshotId;
+      return statisticsFile.snapshotId();
     }
 
     public StatisticsFile statisticsFile() {
@@ -249,7 +258,7 @@ public StatisticsFile statisticsFile() {
 
     @Override
     public void applyTo(TableMetadata.Builder metadataBuilder) {
-      metadataBuilder.setStatistics(snapshotId, statisticsFile);
+      metadataBuilder.setStatistics(statisticsFile);
     }
   }
 

diff --git a/core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java b/core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java
@@ -512,10 +512,9 @@ private static MetadataUpdate readSetDefaultSortOrder(JsonNode node) {
   }
 
   private static MetadataUpdate readSetStatistics(JsonNode node) {
-    long snapshotId = JsonUtil.getLong(SNAPSHOT_ID, node);
     JsonNode statisticsFileNode = JsonUtil.get(STATISTICS, node);
     StatisticsFile statisticsFile = StatisticsFileParser.fromJson(statisticsFileNode);
-    return new MetadataUpdate.SetStatistics(snapshotId, statisticsFile);
+    return new MetadataUpdate.SetStatistics(statisticsFile);
   }
 
   private static MetadataUpdate readRemoveStatistics(JsonNode node) {

diff --git a/core/src/main/java/org/apache/iceberg/SetStatistics.java b/core/src/main/java/org/apache/iceberg/SetStatistics.java
@@ -32,10 +32,23 @@ public SetStatistics(TableOperations ops) {
     this.ops = ops;
   }
 
+  /**
+   * Set the statistics file for a snapshot.
+   *
+   * @deprecated since 1.8.0, will be removed in 1.9.0 or 2.0.0, use {@link
+   *     #setStatistics(StatisticsFile)}.
+   */
+  @Deprecated
   @Override
   public UpdateStatistics setStatistics(long snapshotId, StatisticsFile statisticsFile) {
     Preconditions.checkArgument(snapshotId == statisticsFile.snapshotId());
-    statisticsToSet.put(snapshotId, Optional.of(statisticsFile));
+    statisticsToSet.put(statisticsFile.snapshotId(), Optional.of(statisticsFile));
+    return this;
+  }
+
+  @Override
+  public UpdateStatistics setStatistics(StatisticsFile statisticsFile) {
+    statisticsToSet.put(statisticsFile.snapshotId(), Optional.of(statisticsFile));
     return this;
   }
 

diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java
@@ -1319,6 +1319,12 @@ private Builder resetMainBranch() {
       return this;
     }
 
+    /**
+     * Set a statistics file for a snapshot.
+     *
+     * @deprecated since 1.8.0, will be removed 1.9.0 or 2.0.0, use setStatistics(statisticsFile).
+     */
+    @Deprecated
     public Builder setStatistics(long snapshotId, StatisticsFile statisticsFile) {
       Preconditions.checkNotNull(statisticsFile, "statisticsFile is null");
       Preconditions.checkArgument(
@@ -1327,7 +1333,14 @@ public Builder setStatistics(long snapshotId, StatisticsFile statisticsFile) {
           snapshotId,
           statisticsFile.snapshotId());
       statisticsFiles.put(statisticsFile.snapshotId(), ImmutableList.of(statisticsFile));
-      changes.add(new MetadataUpdate.SetStatistics(snapshotId, statisticsFile));
+      changes.add(new MetadataUpdate.SetStatistics(statisticsFile));
+      return this;
+    }
+
+    public Builder setStatistics(StatisticsFile statisticsFile) {
+      Preconditions.checkNotNull(statisticsFile, "statisticsFile is null");
+      statisticsFiles.put(statisticsFile.snapshotId(), ImmutableList.of(statisticsFile));
+      changes.add(new MetadataUpdate.SetStatistics(statisticsFile));
       return this;
     }
 

diff --git a/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java b/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java
@@ -783,7 +783,6 @@ public void testSetStatistics() {
     long snapshotId = 1940541653261589030L;
     MetadataUpdate expected =
         new MetadataUpdate.SetStatistics(
-            snapshotId,
             new GenericStatisticsFile(
                 snapshotId,
                 "s3://bucket/warehouse/stats.puffin",

diff --git a/open-api/rest-catalog-open-api.py b/open-api/rest-catalog-open-api.py
@@ -1028,7 +1028,11 @@ class Term(BaseModel):
 
 class SetStatisticsUpdate(BaseUpdate):
     action: str = Field('set-statistics', const=True)
-    snapshot_id: int = Field(..., alias='snapshot-id')
+    snapshot_id: Optional[int] = Field(
+        None,
+        alias='snapshot-id',
+        description='This optional field is **DEPRECATED for REMOVAL** since it contains redundant information. Clients should use the `statistics.snapshot-id` field instead.',
+    )
     statistics: StatisticsFile
 
 

diff --git a/open-api/rest-catalog-open-api.yaml b/open-api/rest-catalog-open-api.yaml
@@ -2886,7 +2886,6 @@ components:
       allOf:
         - $ref: '#/components/schemas/BaseUpdate'
       required:
-        - snapshot-id
         - statistics
       properties:
         action:
@@ -2895,6 +2894,10 @@ components:
         snapshot-id:
           type: integer
           format: int64
+          deprecated: true
+          description:
+            This optional field is **DEPRECATED for REMOVAL** since it contains redundant information.
+            Clients should use the `statistics.snapshot-id` field instead.
         statistics:
           $ref: '#/components/schemas/StatisticsFile'