yihua commented on code in PR #11943:
URL: https://github.com/apache/hudi/pull/11943#discussion_r1798529192
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieRecordMerger.java:
##########
@@ -167,17 +169,16 @@ default String[]
getMandatoryFieldsForMerging(HoodieTableConfig cfg) {
*/
String getMergingStrategy();
- /**
- * The record merge mode that corresponds to this record merger
- */
- default RecordMergeMode getRecordMergeMode() {
- switch (getMergingStrategy()) {
- case DEFAULT_MERGER_STRATEGY_UUID:
- return RecordMergeMode.EVENT_TIME_ORDERING;
- case OVERWRITE_MERGER_STRATEGY_UUID:
- return RecordMergeMode.OVERWRITE_WITH_LATEST;
+ static String getAvroMergerStrategyFromMergeMode(RecordMergeMode mergeMode) {
+ switch (mergeMode) {
+ case OVERWRITE_WITH_LATEST:
+ return OVERWRITE_MERGER_STRATEGY_UUID;
+ case EVENT_TIME_ORDERING:
+ return DEFAULT_MERGER_STRATEGY_UUID;
+ case CUSTOM:
+ return PAYLOAD_BASED_MERGER_STRATEGY_UUDID;
default:
- return RecordMergeMode.CUSTOM;
+ throw new IllegalStateException("afdsafdsafds");
Review Comment:
Fix the error message?
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieAvroRecordMerger.java:
##########
@@ -39,7 +39,7 @@ public class HoodieAvroRecordMerger implements
HoodieRecordMerger, OperationMode
@Override
public String getMergingStrategy() {
- return HoodieRecordMerger.DEFAULT_MERGER_STRATEGY_UUID;
+ return HoodieRecordMerger.PAYLOAD_BASED_MERGER_STRATEGY_UUDID;
Review Comment:
We should add a validation that `HoodieAvroRecordMerger` should be used with
this payload merging strategy only, if there is a change of decrepancy in the
code, to be defensive.
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieAvroIndexedRecord.java:
##########
@@ -162,7 +162,7 @@ public HoodieRecord wrapIntoHoodieRecordPayloadWithParams(
Option<String> partitionNameOp,
Boolean populateMetaFields,
Option<Schema> schemaWithoutMetaFields) {
- String payloadClass = ConfigUtils.getPayloadClass(props);
+ String payloadClass = ConfigUtils.getAvroPayloadClass(props);
Review Comment:
I think we can keep the `getPayloadClass` naming. The payload class
implicitly indicates it's using the Avro record.
--
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]
