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


##########
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:
   @leangjonathan I think the stepwise can be implemented on the upgrade itself 
rather than in our test framework. So if a user does Update 1 -> 3 on the back 
end we can do 1 -> 2 -> 3 so we necessarily would go through all the steps. I 
think we are ok just doing
   
   N -> Latest and N -> N+1
   
   The N+1 just for helping us track down stepwise pieces that are broken. 
Although technically if all the n -> n+1 upgrades work the n -> latest should 
also work if we implement it correctly.



-- 
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