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