This is an automated email from the ASF dual-hosted git repository.

xushiyan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hudi.git


The following commit(s) were added to refs/heads/master by this push:
     new 5078d29  [HUDI-2818] Fix 2to3 upgrade when set 
`hoodie.table.keygenerator.class` (#4077)
5078d29 is described below

commit 5078d29eb429d7eca46c3d5c3aa72d94e088d43e
Author: Raymond Xu <[email protected]>
AuthorDate: Tue Nov 23 19:43:34 2021 -0800

    [HUDI-2818] Fix 2to3 upgrade when set `hoodie.table.keygenerator.class` 
(#4077)
---
 .../hudi/table/upgrade/OneToTwoUpgradeHandler.java |  4 +-
 .../table/upgrade/TwoToThreeUpgradeHandler.java    | 12 +++-
 .../hudi/table/upgrade/UpgradeDowngrade.java       |  4 +-
 .../upgrade/TestTwoToThreeUpgradeHandler.java      | 68 ++++++++++++++++++++++
 4 files changed, 81 insertions(+), 7 deletions(-)

diff --git 
a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/OneToTwoUpgradeHandler.java
 
b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/OneToTwoUpgradeHandler.java
index dddd5f4..efa0fe4 100644
--- 
a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/OneToTwoUpgradeHandler.java
+++ 
b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/OneToTwoUpgradeHandler.java
@@ -24,7 +24,7 @@ import org.apache.hudi.common.table.HoodieTableConfig;
 import org.apache.hudi.config.HoodieWriteConfig;
 import org.apache.hudi.keygen.constant.KeyGeneratorOptions;
 
-import java.util.HashMap;
+import java.util.Hashtable;
 import java.util.Map;
 
 /**
@@ -36,7 +36,7 @@ public class OneToTwoUpgradeHandler implements UpgradeHandler 
{
   public Map<ConfigProperty, String> upgrade(
       HoodieWriteConfig config, HoodieEngineContext context, String 
instantTime,
       BaseUpgradeDowngradeHelper upgradeDowngradeHelper) {
-    Map<ConfigProperty, String> tablePropsToAdd = new HashMap<>();
+    Map<ConfigProperty, String> tablePropsToAdd = new Hashtable<>();
     tablePropsToAdd.put(HoodieTableConfig.PARTITION_FIELDS, 
upgradeDowngradeHelper.getPartitionColumns(config));
     tablePropsToAdd.put(HoodieTableConfig.RECORDKEY_FIELDS, 
config.getString(KeyGeneratorOptions.RECORDKEY_FIELD_NAME.key()));
     tablePropsToAdd.put(HoodieTableConfig.BASE_FILE_FORMAT, 
config.getString(HoodieTableConfig.BASE_FILE_FORMAT));
diff --git 
a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/TwoToThreeUpgradeHandler.java
 
b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/TwoToThreeUpgradeHandler.java
index e1dbfbb..bff3788 100644
--- 
a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/TwoToThreeUpgradeHandler.java
+++ 
b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/TwoToThreeUpgradeHandler.java
@@ -22,10 +22,12 @@ package org.apache.hudi.table.upgrade;
 import org.apache.hudi.common.config.ConfigProperty;
 import org.apache.hudi.common.engine.HoodieEngineContext;
 import org.apache.hudi.common.table.HoodieTableConfig;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.common.util.ValidationUtils;
 import org.apache.hudi.config.HoodieWriteConfig;
 import org.apache.hudi.metadata.HoodieTableMetadataUtil;
 
-import java.util.HashMap;
+import java.util.Hashtable;
 import java.util.Map;
 
 /**
@@ -40,10 +42,14 @@ public class TwoToThreeUpgradeHandler implements 
UpgradeHandler {
       // table has been updated and is not backward compatible.
       HoodieTableMetadataUtil.deleteMetadataTable(config.getBasePath(), 
context);
     }
-    Map<ConfigProperty, String> tablePropsToAdd = new HashMap<>();
+    Map<ConfigProperty, String> tablePropsToAdd = new Hashtable<>();
     tablePropsToAdd.put(HoodieTableConfig.URL_ENCODE_PARTITIONING, 
config.getStringOrDefault(HoodieTableConfig.URL_ENCODE_PARTITIONING));
     tablePropsToAdd.put(HoodieTableConfig.HIVE_STYLE_PARTITIONING_ENABLE, 
config.getStringOrDefault(HoodieTableConfig.HIVE_STYLE_PARTITIONING_ENABLE));
-    tablePropsToAdd.put(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME, 
config.getString(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME));
+    String keyGenClassName = 
Option.ofNullable(config.getString(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME))
+        .orElse(config.getString(HoodieWriteConfig.KEYGENERATOR_CLASS_NAME));
+    ValidationUtils.checkState(keyGenClassName != null, String.format("Missing 
config: %s or %s",
+            HoodieTableConfig.KEY_GENERATOR_CLASS_NAME, 
HoodieWriteConfig.KEYGENERATOR_CLASS_NAME));
+    tablePropsToAdd.put(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME, 
keyGenClassName);
     return tablePropsToAdd;
   }
 }
diff --git 
a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java
 
b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java
index c1b8910..0e8f752 100644
--- 
a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java
+++ 
b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java
@@ -31,7 +31,7 @@ import org.apache.hadoop.fs.Path;
 import org.apache.log4j.LogManager;
 import org.apache.log4j.Logger;
 
-import java.util.HashMap;
+import java.util.Hashtable;
 import java.util.Map;
 
 /**
@@ -110,7 +110,7 @@ public class UpgradeDowngrade {
 
     // Perform the actual upgrade/downgrade; this has to be idempotent, for 
now.
     LOG.info("Attempting to move table from version " + fromVersion + " to " + 
toVersion);
-    Map<ConfigProperty, String> tableProps = new HashMap<>();
+    Map<ConfigProperty, String> tableProps = new Hashtable<>();
     if (fromVersion.versionCode() < toVersion.versionCode()) {
       // upgrade
       while (fromVersion.versionCode() < toVersion.versionCode()) {
diff --git 
a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/upgrade/TestTwoToThreeUpgradeHandler.java
 
b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/upgrade/TestTwoToThreeUpgradeHandler.java
new file mode 100644
index 0000000..35928dc
--- /dev/null
+++ 
b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/upgrade/TestTwoToThreeUpgradeHandler.java
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.table.upgrade;
+
+import org.apache.hudi.common.config.ConfigProperty;
+import org.apache.hudi.common.config.HoodieMetadataConfig;
+import org.apache.hudi.common.table.HoodieTableConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.keygen.KeyGenerator;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class TestTwoToThreeUpgradeHandler {
+
+  HoodieWriteConfig config;
+
+  @BeforeEach
+  void setUp() {
+    config = HoodieWriteConfig.newBuilder()
+        .forTable("foo")
+        .withPath("/foo")
+        
.withMetadataConfig(HoodieMetadataConfig.newBuilder().enable(false).build())
+        .build();
+  }
+
+  @ParameterizedTest
+  @ValueSource(strings = {"hoodie.table.keygenerator.class", 
"hoodie.datasource.write.keygenerator.class"})
+  void upgradeHandlerShouldRetrieveKeyGeneratorConfig(String keyGenConfigKey) {
+    config.setValue(keyGenConfigKey, KeyGenerator.class.getName());
+    TwoToThreeUpgradeHandler handler = new TwoToThreeUpgradeHandler();
+    Map<ConfigProperty, String> kv = handler.upgrade(config, null, null, null);
+    assertEquals(KeyGenerator.class.getName(), 
kv.get(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME));
+  }
+
+  @Test
+  void upgradeHandlerShouldThrowWhenKeyGeneratorNotSet() {
+    TwoToThreeUpgradeHandler handler = new TwoToThreeUpgradeHandler();
+    Throwable t = assertThrows(IllegalStateException.class, () -> handler
+        .upgrade(config, null, null, null));
+    assertTrue(t.getMessage().startsWith("Missing config:"));
+  }
+}

Reply via email to