leangjonathan commented on code in PR #10861:
URL: https://github.com/apache/iceberg/pull/10861#discussion_r1705965721


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1004,24 +1004,28 @@ private Builder setInitialFormatVersion(int 
newFormatVersion) {
       return this;
     }
 
-    public Builder upgradeFormatVersion(int newFormatVersion) {
+    public Builder upgradeFormatVersion(int targetFormatVersion) {
       Preconditions.checkArgument(
-          newFormatVersion <= SUPPORTED_TABLE_FORMAT_VERSION,
+          targetFormatVersion <= SUPPORTED_TABLE_FORMAT_VERSION,
           "Cannot upgrade table to unsupported format version: v%s (supported: 
v%s)",
-          newFormatVersion,
+          targetFormatVersion,
           SUPPORTED_TABLE_FORMAT_VERSION);
       Preconditions.checkArgument(
-          newFormatVersion >= formatVersion,
+          targetFormatVersion >= formatVersion,
           "Cannot downgrade v%s table to v%s",
           formatVersion,
-          newFormatVersion);
+          targetFormatVersion);
 
-      if (newFormatVersion == formatVersion) {
+      if (targetFormatVersion == formatVersion) {
         return this;
       }
 
-      this.formatVersion = newFormatVersion;
-      changes.add(new MetadataUpdate.UpgradeFormatVersion(newFormatVersion));
+      // register incremental version changes separately to ensure all upgrade 
requirements are met
+      for (int version = formatVersion + 1; version <= targetFormatVersion; 
version++) {
+        changes.add(new MetadataUpdate.UpgradeFormatVersion(version));

Review Comment:
   +1
   
   I was poking around the HTTP client, REST (de)serializers, and request 
parser logic. There is no content enforcement JSON inputs for deserialization . 
Specifically, the implementation of `UpdateTableRequestParser::fromJson` does 
not include any moderation of contents. Though I suppose deserialization and 
parsing wouldn't be the right place to enforce this so we come back to 
@amogh-jahagirdar's concern about the REST spec and getting out of order or 
missing versions in the upgrade list.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to