yihua commented on code in PR #13656:
URL: https://github.com/apache/hudi/pull/13656#discussion_r2268556743


##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -1069,6 +1069,10 @@ public String getKeyGeneratorClassName() {
     return KeyGeneratorType.getKeyGeneratorClassName(this);
   }
 
+  public String getKeyGeneratorType() {
+    return getStringOrDefault(KEY_GENERATOR_TYPE, null);
+  }
+

Review Comment:
   Remove this since it is only used by the test?



##########
hudi-common/src/main/java/org/apache/hudi/keygen/constant/KeyGeneratorType.java:
##########
@@ -146,7 +161,11 @@ public static String getKeyGeneratorClassName(Map<String, 
String> config) {
     if (config.containsKey(KEY_GENERATOR_CLASS_NAME.key())) {
       return config.get(KEY_GENERATOR_CLASS_NAME.key());
     } else if (config.containsKey(KEY_GENERATOR_TYPE.key())) {
-      return 
KeyGeneratorType.valueOf(config.get(KEY_GENERATOR_TYPE.key())).getClassName();
+      KeyGeneratorType type = 
KeyGeneratorType.valueOf(config.get(KEY_GENERATOR_TYPE.key()));
+      if (type == USER_PROVIDED) {
+        return null;

Review Comment:
   Same on throwing the exception here



##########
hudi-common/src/main/java/org/apache/hudi/keygen/constant/KeyGeneratorType.java:
##########
@@ -136,7 +147,11 @@ public static String 
getKeyGeneratorClassName(TypedProperties props) {
       return ConfigUtils.getStringWithAltKeys(props, KEY_GENERATOR_CLASS_NAME);
     }
     if (ConfigUtils.containsConfigProperty(props, KEY_GENERATOR_TYPE)) {
-      return KeyGeneratorType.valueOf(ConfigUtils.getStringWithAltKeys(props, 
KEY_GENERATOR_TYPE)).getClassName();
+      KeyGeneratorType type = 
KeyGeneratorType.valueOf(ConfigUtils.getStringWithAltKeys(props, 
KEY_GENERATOR_TYPE));
+      if (USER_PROVIDED == type) {
+        return null;

Review Comment:
   Throw the exception here since this indicates that the table config is 
invalid?



##########
hudi-common/src/main/java/org/apache/hudi/keygen/constant/KeyGeneratorType.java:
##########
@@ -97,9 +99,12 @@ public enum KeyGeneratorType {
   
SPARK_SQL_MERGE_INTO("org.apache.spark.sql.hudi.command.MergeIntoKeyGenerator"),
 
   @EnumFieldDescription("A test KeyGenerator for deltastreamer tests.")
-  
STREAMER_TEST("org.apache.hudi.utilities.deltastreamer.TestHoodieDeltaStreamer$TestGenerator");
+  
STREAMER_TEST("org.apache.hudi.utilities.deltastreamer.TestHoodieDeltaStreamer$TestGenerator"),

Review Comment:
   Could `STREAMER_TEST` be removed, since this is intended as `USER_PROVIDED` 
key generator type? Adjust the corresponding test to use `USER_PROVIDED` key 
generator type.



-- 
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]

Reply via email to