vinothchandar commented on code in PR #13650:
URL: https://github.com/apache/hudi/pull/13650#discussion_r2292518993
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -258,6 +258,33 @@ public class HoodieWriteConfig extends HoodieConfig {
"**Note** This is being actively worked on. Please use "
+ "`hoodie.datasource.write.keygenerator.class` instead.");
+ public static final ConfigProperty<Boolean>
COMPLEX_KEYGEN_ENCODE_SINGLE_RECORD_KEY_FIELD_NAME = ConfigProperty
+ .key("hoodie.write.complex.keygen.encode.single.record.key.field.name")
Review Comment:
naming.. `hoodie.write.complex.keygen.old.encoding` (or sth simpler).. I
dont think we want to make this sound like a feature
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -1689,4 +1694,22 @@ private InternalSchema
getInternalSchema(TableSchemaResolver schemaUtil) {
}
});
}
+
+ private void validateComplexKeygen(HoodieTableMetaClient metaClient) {
Review Comment:
Can we preserve a single point for all these validation checks..
`CommonClientUtils#validateTableVersion` is already wired into all write
paths? Can we move all these checks into the same file?
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -130,6 +131,7 @@ public abstract class BaseHoodieWriteClient<T, I, K, O>
extends BaseHoodieClient
protected static final String LOOKUP_STR = "lookup";
private static final long serialVersionUID = 1L;
private static final Logger LOG =
LoggerFactory.getLogger(BaseHoodieWriteClient.class);
+ private static final String SPARK_COMPLEX_KEYGEN_CLASS_NAME =
"org.apache.hudi.keygen.ComplexKeyGenerator";
Review Comment:
this looks lil unclean.. Can't we do
`org.apache.hudi.keygen.ComplexKeyGenerator.getClass.getName` or sth?
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -1689,4 +1694,22 @@ private InternalSchema
getInternalSchema(TableSchemaResolver schemaUtil) {
}
});
}
+
+ private void validateComplexKeygen(HoodieTableMetaClient metaClient) {
+ HoodieTableConfig tableConfig = metaClient.getTableConfig();
+ String keyGeneratorClassName = tableConfig.getKeyGeneratorClassName();
+ Option<String[]> recordKeyFields = tableConfig.getRecordKeyFields();
+ if ((SPARK_COMPLEX_KEYGEN_CLASS_NAME.equals(keyGeneratorClassName)
Review Comment:
lets just fix the key generator with the regression.. Does both of them get
affected by this? I don't think it's a good idea to handle it generally like
this based on record key/multiple partition fields. ..
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -258,6 +258,33 @@ public class HoodieWriteConfig extends HoodieConfig {
"**Note** This is being actively worked on. Please use "
+ "`hoodie.datasource.write.keygenerator.class` instead.");
+ public static final ConfigProperty<Boolean>
COMPLEX_KEYGEN_ENCODE_SINGLE_RECORD_KEY_FIELD_NAME = ConfigProperty
+ .key("hoodie.write.complex.keygen.encode.single.record.key.field.name")
+ .defaultValue(true)
+ .markAdvanced()
+ .sinceVersion("0.14.2,0.15.1,1.0.3,1.1.0")
Review Comment:
just a single version? is n't this a new usage pattern.. if you want to do
this.. make a `supportedVersions(<List>)` . `since` implies that there is a
version `>=` that supports everything
--
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]
