amogh-jahagirdar commented on code in PR #10861:
URL: https://github.com/apache/iceberg/pull/10861#discussion_r1707253771
##########
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:
@nastra Check out the thread
https://github.com/apache/iceberg/pull/10861#discussion_r1703406710, I had a
similar question earlier but the rationale for the internal stepwise upgrade is
to avoid having multiple upgrade paths from a table metadata builder
perspective.
So the the rationale for performing the stepwise upgrade at least from a
client side perspective in the reference implementation (the change here):
The stepwise approach has the advantage of keeping testing a bit simpler
(don't need to test 1->3 direct and 1->2->3, there's only a single path from 1
to 3) . It also seems better from a TableMetadata API perspective to ensure
that any potential issues on the upgrade path have a consistent failure point.
So if there's an issue upgrading from 2->3 in the 1->2->3 path, it's easy to
distinguish that compared to 1->3 somehow just working (as an example).
I think in the end for the server side we can't really require a single
upgrade path because the client can send any list of upgrade options, nor do I
think we want to spec that out since we probably want to enable servers to make
their choice? If they want to do the stepwise internal upgrade themselves since
they consider it more safe or if they want to direct upgrade they can just do
that.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
