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


##########
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:
   I've read the conversation you mentioned in 
https://github.com/apache/iceberg/pull/10861#discussion_r1703406710 but I still 
don't see a good enough reason to internally do a 1 -> 2 -> 3 upgrade path when 
a user wants to perform 1 -> 3. 
   Are we potentially foreseeing issues during such an upgrade that would 
justify having 1 -> 2 -> 3? The way I see it is that we're just adding an 
additional metadata update that is being applied immediately, without any other 
stuff interfering with it, since all of the format version upgrades are in 
sequence.
   I'm ok either way, but I'd just like to understand what potential problems 
this might be preventing.
   



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