yihua commented on code in PR #11943:
URL: https://github.com/apache/hudi/pull/11943#discussion_r1805679985
##########
hudi-common/src/main/java/org/apache/hudi/common/util/HoodieRecordUtils.java:
##########
@@ -71,6 +71,11 @@ public static HoodieRecordMerger loadRecordMerger(String
mergerClass) {
}
}
+ public static HoodieRecordMerger createRecordMerger(String basePath,
EngineType engineType,
+ String mergerImpls,
String recordMergerStrategy) {
+ return createRecordMerger(basePath, engineType,
ConfigUtils.split2List(mergerImpls), recordMergerStrategy);
Review Comment:
Should `HoodieTableConfig` parse the custom merge implementation class list,
i.e., `HoodieTableConfig#getRecordMergerStrategy` returns the `List<String>
mergerClassList`, instead of passing the config value all the way down? and
get rid of this new API.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodiePositionBasedFileGroupRecordBuffer.java:
##########
@@ -68,11 +68,11 @@ public class HoodiePositionBasedFileGroupRecordBuffer<T>
extends HoodieKeyBasedF
public HoodiePositionBasedFileGroupRecordBuffer(HoodieReaderContext<T>
readerContext,
HoodieTableMetaClient
hoodieTableMetaClient,
+ RecordMergeMode
recordMergeMode,
Review Comment:
Makes sense. I misread the code. No need to revert the change.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodiePositionBasedFileGroupRecordBuffer.java:
##########
@@ -199,7 +199,7 @@ public void processNextDeletedRecord(DeleteRecord
deleteRecord, Serializable rec
if (recordOpt.isPresent()) {
String recordKey = recordOpt.get().getRecordKey();
records.put(recordPosition, Pair.of(Option.empty(),
readerContext.generateMetadataForRecord(
- recordKey, recordOpt.get().getPartitionPath(),
recordOpt.get().getOrderingValue(), orderingFieldType)));
+ recordKey, recordOpt.get().getPartitionPath(),
recordOpt.get().getOrderingValue() == null ? orderingFieldDefault :
recordOpt.get().getOrderingValue(), orderingFieldTypeOpt)));
Review Comment:
We should not set `orderingFieldDefault` which can cause confusion and
written to the storage eventually.
--
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]
