wombatu-kun commented on code in PR #20049:
URL: https://github.com/apache/hudi/pull/20049#discussion_r4102215873
##########
hudi-common/src/main/java/org/apache/hudi/common/util/OrderingValues.java:
##########
@@ -91,6 +92,21 @@ public static Comparable getDefault() {
/**
* Returns whether the given {@code orderingValue} is default.
*/
+ /**
Review Comment:
`isMissing` landed between `isDefault` and its Javadoc, so `isDefault` is
now undocumented and its old comment dangles above this one. Moving `isMissing`
below `isDefault` fixes both.
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/HoodieStreamerUtils.java:
##########
@@ -97,6 +99,9 @@ public static Option<JavaRDD<HoodieRecord>>
createHoodieRecords(HoodieStreamer.C
String payloadClassName = StringUtils.isNullOrEmpty(cfg.payloadClassName)
? HoodieRecordPayload.getAvroPayloadForMergeMode(cfg.recordMergeMode,
cfg.payloadClassName)
: cfg.payloadClassName;
+ boolean requiresOrderingValue = shouldUseOrderingField
+ && cfg.recordMergeMode != RecordMergeMode.COMMIT_TIME_ORDERING
Review Comment:
`requiresOrderingValue` keys off `cfg.recordMergeMode`, which
`HoodieStreamer` infers from the CLI flags alone, so a run that omits
`--source-ordering-fields` and takes the ordering fields from `tableConfig`
resolves to COMMIT_TIME_ORDERING here and skips this check on an
EVENT_TIME_ORDERING table. Could this read `tableConfig.getRecordMergeMode()`
instead, as `HoodieCreateRecordUtils` does?
--
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]
