rahil-c commented on code in PR #13735:
URL: https://github.com/apache/hudi/pull/13735#discussion_r2366288759
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -99,43 +96,49 @@ public static boolean needsDowngrade(HoodieTableVersion
fromTableVersion, Hoodie
return toWriteVersion.lesserThan(fromTableVersion);
}
- public static boolean needsUpgrade(HoodieTableMetaClient metaClient,
HoodieWriteConfig config, HoodieTableVersion toWriteVersion) {
+ public static boolean needsUpgrade(HoodieTableMetaClient metaClient,
+ HoodieWriteConfig config,
+ HoodieTableVersion toVersion) {
HoodieTableVersion fromTableVersion =
metaClient.getTableConfig().getTableVersion();
- if (fromTableVersion.greaterThan(toWriteVersion)) {
- LOG.warn("Table version {} is greater than write version {}. No upgrade
needed", fromTableVersion, toWriteVersion);
- return false;
- }
- if (fromTableVersion.equals(toWriteVersion)) {
- // table version is same as write version, no upgrade needed
- return false;
- }
- if (fromTableVersion.lesserThan(HoodieTableVersion.SIX)) {
- throw new HoodieUpgradeDowngradeException(
- String.format("Hudi 1.x release only supports table version greater
than "
- + "version 6 or above. Please upgrade table from version %s
to %s "
- + "using a Hudi release prior to 1.0.0",
- fromTableVersion.versionCode(),
HoodieTableVersion.SIX.versionCode()));
+ boolean shouldUpgrade = true;
+ if (fromTableVersion.greaterThanOrEquals(toVersion)) {
+ LOG.warn("Table version {} is greater than write version {}. No upgrade
needed",
+ fromTableVersion, toVersion);
+ shouldUpgrade = false;
+ } else {
+ if (fromTableVersion.lesserThan(HoodieTableVersion.SIX)) {
+ throw new HoodieUpgradeDowngradeException(
+ String.format("Hudi 1.x release only supports table version
greater than "
+ + "version 6 or above. Please upgrade table from version
%s to %s "
+ + "using a Hudi release prior to 1.0.0",
+ fromTableVersion.versionCode(),
HoodieTableVersion.SIX.versionCode()));
+ }
+ if (!config.autoUpgrade()) {
+ shouldUpgrade = false;
+ }
}
- if (!config.autoUpgrade()) {
- // if autoUpgrade is disabled and table version is greater than or equal
to SIX,
- // then we must ensure the write version is set to the table version.
- // and skip the upgrade
- config.setWriteVersion(fromTableVersion);
- LOG.warn("hoodie.write.auto.upgrade was disabled. Table version {} does
not match write version {}. "
- + "Setting hoodie.write.table.version={} to match
hoodie.table.version, and skipping upgrade",
- fromTableVersion.versionCode(), toWriteVersion.versionCode(),
fromTableVersion.versionCode());
- return false;
+ if (!shouldUpgrade && fromTableVersion != toVersion) {
+ if (!config.autoUpgrade()) {
Review Comment:
@linliu-code Can you help me understand why the logic in this path is
changed, it feels little more complex then the original? Maybe if you can let
me know what was the case/condition missed in the old path would be helpful.
--
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]
