yihua commented on code in PR #11943:
URL: https://github.com/apache/hudi/pull/11943#discussion_r1798508733
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -1239,16 +1234,69 @@ public String getBasePath() {
}
public HoodieFileFormat getBaseFileFormat() {
- return HoodieFileFormat.valueOf(getStringOrDefault(BASE_FILE_FORMAT));
+ return
HoodieFileFormat.valueOf(getStringOrDefault(BASE_FILE_FORMAT).toUpperCase());
+ }
+
+ public String getRecordMergerStrategy() {
+ return getString(RECORD_MERGER_STRATEGY);
+ }
+
+ public RecordMergeMode getRecordMergeMode() {
+ return RecordMergeMode.getValue(getString(RECORD_MERGE_MODE));
}
public HoodieRecordMerger getRecordMerger() {
- List<String> mergers =
StringUtils.split(getStringOrDefault(RECORD_MERGER_IMPLS), ",").stream()
+ List<String> mergers = StringUtils.split(getString(RECORD_MERGER_IMPLS),
",").stream()
.map(String::trim)
.distinct()
.collect(Collectors.toList());
- String recordMergerStrategy = getString(RECORD_MERGER_STRATEGY);
- return HoodieRecordUtils.createRecordMerger(getString(BASE_PATH),
engineType, mergers, recordMergerStrategy);
+ return getRecordMerger(getString(BASE_PATH), getRecordMergeMode(),
+ engineType, getLogDataBlockFormat(), mergers,
getStringOpt(RECORD_MERGER_STRATEGY));
+ }
+
+ public static HoodieRecordMerger getRecordMerger(String basePath,
Review Comment:
We should get rid of the complexity of determining merger based on the log
block type.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -168,21 +168,21 @@ public class HoodieWriteConfig extends HoodieConfig {
public static final ConfigProperty<String> WRITE_PAYLOAD_CLASS_NAME =
ConfigProperty
.key("hoodie.datasource.write.payload.class")
- .defaultValue(DefaultHoodieRecordPayload.class.getName())
+ .noDefaultValue()
.markAdvanced()
+ .deprecatedAfter("1.0.0")
.withDocumentation("Payload class used. Override this, if you like to
roll your own merge logic, when upserting/inserting. "
+ "This will render any value set for PRECOMBINE_FIELD_OPT_VAL
in-effective");
- public static final ConfigProperty<String> WRITE_PAYLOAD_TYPE =
ConfigProperty
- .key("hoodie.datasource.write.payload.type")
- .defaultValue(RecordPayloadType.HOODIE_AVRO_DEFAULT.name())
- .markAdvanced()
+ public static final ConfigProperty<String> RECORD_MERGE_MODE = ConfigProperty
+ .key("hoodie.record.merge.mode")
+ .defaultValue(RecordMergeMode.EVENT_TIME_ORDERING.name())
Review Comment:
One benefit is to check conflicts. My original point is we should not
redefine the same config in both `HoodieTableConfig` and `HoodieWriteConfig`
classes which leaves room for discrepancy.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -168,21 +168,21 @@ public class HoodieWriteConfig extends HoodieConfig {
public static final ConfigProperty<String> WRITE_PAYLOAD_CLASS_NAME =
ConfigProperty
.key("hoodie.datasource.write.payload.class")
- .defaultValue(DefaultHoodieRecordPayload.class.getName())
+ .noDefaultValue()
Review Comment:
The point is that, for the backwards compatible writer writing table version
6 (0.14 release and above) in Hudi 1.0 release, it should not use the merge
mode and only leverage payload class to create the table.
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieRecordMerger.java:
##########
@@ -48,6 +48,8 @@ public interface HoodieRecordMerger extends Serializable {
String OVERWRITE_MERGER_STRATEGY_UUID =
"ce9acb64-bde0-424c-9b91-f6ebba25356d";
+ String PAYLOAD_BASED_MERGER_STRATEGY_UUDID =
"00000000-0000-0000-0000-000000000000";
Review Comment:
```suggestion
String PAYLOAD_BASED_MERGER_STRATEGY_UUID =
"00000000-0000-0000-0000-000000000000";
```
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -1746,6 +1785,10 @@ public int getAsyncClusterMaxCommits() {
return getInt(HoodieClusteringConfig.ASYNC_CLUSTERING_MAX_COMMITS);
}
+ public String getAvroPayloadClass() {
+ return getStringOpt(HoodiePayloadConfig.PAYLOAD_CLASS_NAME).orElseGet(()
-> HoodieRecordPayload.getAvroPayloadForMergeMode(getRecordMergeMode()));
Review Comment:
We still need to main the Avro merger path for custom payload for some time.
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieRecordMerger.java:
##########
@@ -48,6 +48,8 @@ public interface HoodieRecordMerger extends Serializable {
String OVERWRITE_MERGER_STRATEGY_UUID =
"ce9acb64-bde0-424c-9b91-f6ebba25356d";
+ String PAYLOAD_BASED_MERGER_STRATEGY_UUDID =
"00000000-0000-0000-0000-000000000000";
Review Comment:
Let's add comprehensive docs on each UUID to explain what they are and how
they are used in code.
--
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]
