yihua commented on code in PR #13687:
URL: https://github.com/apache/hudi/pull/13687#discussion_r2265064168
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -90,14 +90,34 @@ public static boolean
needsUpgradeOrDowngrade(HoodieTableMetaClient metaClient,
public static boolean needsUpgrade(HoodieTableMetaClient metaClient,
HoodieWriteConfig config, HoodieTableVersion toWriteVersion) {
HoodieTableVersion fromTableVersion =
metaClient.getTableConfig().getTableVersion();
- // If table version is less than SIX, then we need to upgrade to SIX first
before upgrading to any other version, irrespective of autoUpgrade flag
+ if (fromTableVersion.versionCode() >= toWriteVersion.versionCode()) {
+ // if the table version is greater than or equal to the write version,
then this is not an upgrade
+ LOG.warn("Table version {} is greater than or equal to write version {}.
No upgrade needed", fromTableVersion, toWriteVersion);
Review Comment:
In this case and autoUpgrade is false and `fromTableVersion.versionCode() >
toWriteVersion.versionCode()`, should an error be thrown given the mismatch (or
does the code automatically elevate the write table version to be the same as
the existing table version)? Could a test case like this be added at the write
client or data source write level?
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -90,14 +90,34 @@ public static boolean
needsUpgradeOrDowngrade(HoodieTableMetaClient metaClient,
public static boolean needsUpgrade(HoodieTableMetaClient metaClient,
HoodieWriteConfig config, HoodieTableVersion toWriteVersion) {
HoodieTableVersion fromTableVersion =
metaClient.getTableConfig().getTableVersion();
- // If table version is less than SIX, then we need to upgrade to SIX first
before upgrading to any other version, irrespective of autoUpgrade flag
+ if (fromTableVersion.versionCode() >= toWriteVersion.versionCode()) {
+ // if the table version is greater than or equal to the write version,
then this is not an upgrade
+ LOG.warn("Table version {} is greater than or equal to write version {}.
No upgrade needed", fromTableVersion, toWriteVersion);
+ return false;
+ }
if (fromTableVersion.versionCode() < HoodieTableVersion.SIX.versionCode()
&& toWriteVersion.versionCode() >= HoodieTableVersion.EIGHT.versionCode()) {
+ // if the table's current version is less than SIX and the write version
is greater than or equal to EIGHT,
+ // we require the user to upgrade to SIX before upgrading to versions
greater than or equal to EIGHT
Review Comment:
This comment is redundant.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -90,14 +90,34 @@ public static boolean
needsUpgradeOrDowngrade(HoodieTableMetaClient metaClient,
public static boolean needsUpgrade(HoodieTableMetaClient metaClient,
HoodieWriteConfig config, HoodieTableVersion toWriteVersion) {
HoodieTableVersion fromTableVersion =
metaClient.getTableConfig().getTableVersion();
- // If table version is less than SIX, then we need to upgrade to SIX first
before upgrading to any other version, irrespective of autoUpgrade flag
+ if (fromTableVersion.versionCode() >= toWriteVersion.versionCode()) {
+ // if the table version is greater than or equal to the write version,
then this is not an upgrade
+ LOG.warn("Table version {} is greater than or equal to write version {}.
No upgrade needed", fromTableVersion, toWriteVersion);
+ return false;
+ }
if (fromTableVersion.versionCode() < HoodieTableVersion.SIX.versionCode()
&& toWriteVersion.versionCode() >= HoodieTableVersion.EIGHT.versionCode()) {
Review Comment:
As discussed, let's enforce that the fromVersion should be at least SIX.
Disable any tests (e.g., 4 -> 5, 5 -> 6) that are not applicable any more (we
can have a separate PR to clean up the code).
```suggestion
if (fromTableVersion.versionCode() <
HoodieTableVersion.SIX.versionCode())
```
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -90,14 +90,34 @@ public static boolean
needsUpgradeOrDowngrade(HoodieTableMetaClient metaClient,
public static boolean needsUpgrade(HoodieTableMetaClient metaClient,
HoodieWriteConfig config, HoodieTableVersion toWriteVersion) {
HoodieTableVersion fromTableVersion =
metaClient.getTableConfig().getTableVersion();
- // If table version is less than SIX, then we need to upgrade to SIX first
before upgrading to any other version, irrespective of autoUpgrade flag
+ if (fromTableVersion.versionCode() >= toWriteVersion.versionCode()) {
Review Comment:
nit: use `HoodieTableVersion#greaterThan`
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -90,14 +90,34 @@ public static boolean
needsUpgradeOrDowngrade(HoodieTableMetaClient metaClient,
public static boolean needsUpgrade(HoodieTableMetaClient metaClient,
HoodieWriteConfig config, HoodieTableVersion toWriteVersion) {
HoodieTableVersion fromTableVersion =
metaClient.getTableConfig().getTableVersion();
- // If table version is less than SIX, then we need to upgrade to SIX first
before upgrading to any other version, irrespective of autoUpgrade flag
+ if (fromTableVersion.versionCode() >= toWriteVersion.versionCode()) {
+ // if the table version is greater than or equal to the write version,
then this is not an upgrade
+ LOG.warn("Table version {} is greater than or equal to write version {}.
No upgrade needed", fromTableVersion, toWriteVersion);
+ return false;
+ }
if (fromTableVersion.versionCode() < HoodieTableVersion.SIX.versionCode()
&& toWriteVersion.versionCode() >= HoodieTableVersion.EIGHT.versionCode()) {
+ // if the table's current version is less than SIX and the write version
is greater than or equal to EIGHT,
+ // we require the user to upgrade to SIX before upgrading to versions
greater than or equal to EIGHT
throw new HoodieUpgradeDowngradeException(
- String.format("Please upgrade table from version %s to %s before
upgrading to version %s.", fromTableVersion,
HoodieTableVersion.SIX.versionCode(), toWriteVersion));
+ String.format("Please upgrade table from version %s to %s before
upgrading to version %s.", fromTableVersion,
HoodieTableVersion.SIX.versionCode(), toWriteVersion));
}
-
- // allow upgrades otherwise.
- return toWriteVersion.versionCode() > fromTableVersion.versionCode();
+ if (!config.autoUpgrade()) {
+ if (fromTableVersion.versionCode() <
HoodieTableVersion.SIX.versionCode()) {
+ // throw an exception as autoUpgrade is disabled, and table version is
less than SIX
+ // need to upgrade to SIX and set to autoUpgrade to true, otherwise
the setting the write config value to a value lower than six will fail
+ throw new HoodieUpgradeDowngradeException(
+ String.format("AUTO_UPGRADE_VERSION was disabled, Please
upgrade table to version %s by setting AUTO_UPGRADE_VERSION to true.",
HoodieTableVersion.SIX.versionCode()));
+ }
+ // if autoUpgrade is disabled and table version is greater than SIX,
then we must ensure the write version is set to the table version.
+ // and skip the upgrade
+ config.setWriteVersion(fromTableVersion);
+ LOG.warn("AUTO_UPGRADE_VERSION was disabled. Table version {} does not
match write version {}. "
Review Comment:
nit: replace `AUTO_UPGRADE_VERSION` with actual config key
```suggestion
LOG.warn("<config key> was disabled. Table version {} does not match
write version {}. "
```
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -90,14 +90,34 @@ public static boolean
needsUpgradeOrDowngrade(HoodieTableMetaClient metaClient,
public static boolean needsUpgrade(HoodieTableMetaClient metaClient,
HoodieWriteConfig config, HoodieTableVersion toWriteVersion) {
HoodieTableVersion fromTableVersion =
metaClient.getTableConfig().getTableVersion();
- // If table version is less than SIX, then we need to upgrade to SIX first
before upgrading to any other version, irrespective of autoUpgrade flag
+ if (fromTableVersion.versionCode() >= toWriteVersion.versionCode()) {
+ // if the table version is greater than or equal to the write version,
then this is not an upgrade
Review Comment:
nit: remove
--
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]
