yihua commented on code in PR #13656:
URL: https://github.com/apache/hudi/pull/13656#discussion_r2268572390
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java:
##########
@@ -1551,9 +1551,20 @@ public Properties build() {
tableConfig.setValue(HoodieTableConfig.POPULATE_META_FIELDS,
Boolean.toString(populateMetaFields));
}
if (null != keyGeneratorClassProp) {
- tableConfig.setValue(HoodieTableConfig.KEY_GENERATOR_TYPE,
KeyGeneratorType.fromClassName(keyGeneratorClassProp).name());
+ KeyGeneratorType type =
KeyGeneratorType.fromClassName(keyGeneratorClassProp);
+ tableConfig.setValue(HoodieTableConfig.KEY_GENERATOR_TYPE,
type.name());
+ // An invalid class name may be provided when type is `USER_PROVIDED`.
+ // The runtime exception should be thrown when this class value is
used, e.g., in class loading.
+ tableConfig.setValue(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME,
keyGeneratorClassProp);
} else if (null != keyGeneratorType) {
- tableConfig.setValue(HoodieTableConfig.KEY_GENERATOR_TYPE,
keyGeneratorType);
+
checkArgument(!keyGeneratorType.equals(KeyGeneratorType.USER_PROVIDED.name()),
+ String.format("When key generator type is %s, the key generator
class must be set properly",
+ KeyGeneratorType.USER_PROVIDED.name()));
+ // When `keyGeneratorType` does not match any types, it throws; no
extra check
+ // is needed for confirming the validity of `keyGeneratorType`.
+ KeyGeneratorType type = KeyGeneratorType.valueOf(keyGeneratorType);
+ tableConfig.setValue(HoodieTableConfig.KEY_GENERATOR_TYPE,
type.name());
+ tableConfig.setValue(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME,
type.getClassName());
Review Comment:
There is no need to set the `KEY_GENERATOR_CLASS_NAME` if only
`keyGeneratorType` is set, i.e., the built-in type is used here.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java:
##########
@@ -1551,9 +1551,20 @@ public Properties build() {
tableConfig.setValue(HoodieTableConfig.POPULATE_META_FIELDS,
Boolean.toString(populateMetaFields));
}
if (null != keyGeneratorClassProp) {
- tableConfig.setValue(HoodieTableConfig.KEY_GENERATOR_TYPE,
KeyGeneratorType.fromClassName(keyGeneratorClassProp).name());
+ KeyGeneratorType type =
KeyGeneratorType.fromClassName(keyGeneratorClassProp);
+ tableConfig.setValue(HoodieTableConfig.KEY_GENERATOR_TYPE,
type.name());
+ // An invalid class name may be provided when type is `USER_PROVIDED`.
+ // The runtime exception should be thrown when this class value is
used, e.g., in class loading.
+ tableConfig.setValue(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME,
keyGeneratorClassProp);
Review Comment:
The `KEY_GENERATOR_CLASS_NAME` should only be set for `USER_PROVIDED` key
generator type.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java:
##########
@@ -1551,9 +1551,20 @@ public Properties build() {
tableConfig.setValue(HoodieTableConfig.POPULATE_META_FIELDS,
Boolean.toString(populateMetaFields));
}
if (null != keyGeneratorClassProp) {
- tableConfig.setValue(HoodieTableConfig.KEY_GENERATOR_TYPE,
KeyGeneratorType.fromClassName(keyGeneratorClassProp).name());
+ KeyGeneratorType type =
KeyGeneratorType.fromClassName(keyGeneratorClassProp);
+ tableConfig.setValue(HoodieTableConfig.KEY_GENERATOR_TYPE,
type.name());
+ // An invalid class name may be provided when type is `USER_PROVIDED`.
+ // The runtime exception should be thrown when this class value is
used, e.g., in class loading.
+ tableConfig.setValue(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME,
keyGeneratorClassProp);
Review Comment:
This is for the case that the built-in key generate class is set.
--
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]
