danny0405 commented on code in PR #13334:
URL: https://github.com/apache/hudi/pull/13334#discussion_r2122403427
##########
hudi-common/src/main/java/org/apache/hudi/common/util/HoodieRecordUtils.java:
##########
@@ -76,20 +81,43 @@ public static HoodieRecordMerger loadRecordMerger(String
mergerClass) {
*/
public static HoodieRecordMerger createRecordMerger(String basePath,
EngineType engineType,
List<String>
mergerClassList, String recordMergerStrategy) {
- if (mergerClassList.isEmpty() ||
HoodieTableMetadata.isMetadataTable(basePath)) {
+ if (HoodieTableMetadata.isMetadataTable(basePath)) {
return HoodieAvroRecordMerger.INSTANCE;
+ } else if (mergerClassList.isEmpty()) {
+ // No merger class is given, we fall back to use Avro based mergers.
+ return createValidRecordMerger(engineType, "",
recordMergerStrategy).get();
} else {
return createValidRecordMerger(engineType, mergerClassList,
recordMergerStrategy)
.orElse(HoodieAvroRecordMerger.INSTANCE);
}
}
public static Option<HoodieRecordMerger> createValidRecordMerger(EngineType
engineType,
- String
mergerImpls, String recordMergerStrategy) {
- if
(recordMergerStrategy.equals(HoodieRecordMerger.PAYLOAD_BASED_MERGE_STRATEGY_UUID))
{
- return Option.of(HoodieAvroRecordMerger.INSTANCE);
+ String
mergerImpls,
+ String
recordMergerStrategy) {
+ switch (recordMergerStrategy) {
+ case HoodieRecordMerger.FIRST_VALUE_MERGE_STRATEGY_UUID:
+ return Option.of(FirstValueAvroRecordMerger.INSTANCE);
+ case HoodieRecordMerger.PARTIAL_UPDATE_MERGE_STRATEGY_UUID:
+ return Option.of(PartialUpdateAvroMerger.INSTANCE);
+ case
HoodieRecordMerger.PARTIAL_UPDATE_WITH_NON_DEFAULT_VALUE_MERGE_STRATEGY_UUID:
+ return Option.of(PartialUpdateWithNonDefaultValueAvroMerger.INSTANCE);
+ case HoodieRecordMerger.PAYLOAD_BASED_MERGE_STRATEGY_UUID:
+ return Option.of(HoodieAvroRecordMerger.INSTANCE);
+ default:
+ // Without merger classes provided; We use default Avro merger.
+ if (mergerImpls.isEmpty()) {
Review Comment:
This is so confusing, some questions here:
1. what is the priority between strategy id and merger class;
2. can one strategy id been shared with multiple merger classes.
if the strategy id has higher priority than merger classes, then we should
always match with the strategy id first no matter whether the merger class is
there or not.
--
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]
