the-other-tim-brown commented on code in PR #13526:
URL: https://github.com/apache/hudi/pull/13526#discussion_r2223825596
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -926,8 +926,12 @@ static RecordMergeMode
inferRecordMergeModeFromMergeStrategyId(String recordMerg
}
}
- public String getPreCombineField() {
- return getString(PRECOMBINE_FIELD);
+ public Option<List<String>> getPreCombineFieldList() {
Review Comment:
I think we should swap the naming here. We want developers to be aware that
there are potentially multiple strings as ordering fields so we should make
this `getPrecombineFields` and the other method `getPrecombineFieldsAsString`
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -814,7 +814,7 @@ public String getRecordMergeStrategyId() {
public static Triple<RecordMergeMode, String, String>
inferCorrectMergingBehavior(RecordMergeMode recordMergeMode,
String payloadClassName,
String recordMergeStrategyId,
-
String orderingFieldName,
+
String orderingFieldNames,
Review Comment:
Should be a list?
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java:
##########
@@ -1153,8 +1153,8 @@ public TableBuilder setBaseFileFormat(String
baseFileFormat) {
return this;
}
- public TableBuilder setPreCombineField(String preCombineField) {
- this.preCombineField = preCombineField;
+ public TableBuilder setPreCombineFields(String preCombineField) {
Review Comment:
Can this be a varargs input so the user can specify more than 1 field?
--
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]
