danny0405 commented on code in PR #13635:
URL: https://github.com/apache/hudi/pull/13635#discussion_r2239149971
##########
hudi-common/src/main/java/org/apache/hudi/common/util/HoodieRecordUtils.java:
##########
@@ -75,12 +77,17 @@ public static HoodieRecordMerger loadRecordMerger(String
mergerClass) {
* Instantiate a given class with a record merge.
*/
public static HoodieRecordMerger createRecordMerger(String basePath,
EngineType engineType,
- List<String>
mergerClassList, String recordMergerStrategy) {
- if (mergerClassList.isEmpty() ||
HoodieTableMetadata.isMetadataTable(basePath)) {
+ List<String>
mergerClassList, String recordMergerStrategy,
+ String payloadClass) {
+ HoodieRecordMerger defaultAvroRecordMerger =
(StringUtils.isNullOrEmpty(payloadClass) ||
HoodieTableConfig.AVRO_BINARY_PAYLOADS.contains(payloadClass))
+ ? HoodieAvroBinaryRecordMerger.INSTANCE :
HoodieAvroRecordMerger.INSTANCE;
Review Comment:
with this path, we should only have 2 avro records impl:
`HoodieAvroIndexedRecord` and `HoodieAvroBinaryRecord`, the `HoodieAvroRecord`
should also be deprecated from the path, we should migrate the logic in
`HoodieAvroRecordMerger.INSTANCE` to adapt to both of these two cases.
And there is no need to pass around the payload class name.
--
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]
