hudi-agent commented on code in PR #20049:
URL: https://github.com/apache/hudi/pull/20049#discussion_r4127401718
##########
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:
🤖 nit: the pre-existing "Returns whether the given orderingValue is default"
javadoc now sits above the new isMissing javadoc/method instead of directly
above isDefault, since the new block was inserted between them. Could you
reorder so each comment stays attached to the method it describes?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
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:
🤖 I traced this and I think the concern holds. With no
`--source-ordering-fields`, `--record-merge-mode`, or payload set,
`HoodieStreamer` calls `inferMergingConfigsForWrites(null, null, null, null,
...)` and gets `COMMIT_TIME_ORDERING`. Nothing reconciles `cfg.recordMergeMode`
with the table config afterwards. Meanwhile `orderingFieldsStr` here comes from
`tableConfig.getOrderingFieldsStr()`, so the ordering value is still built but
never validated. One more thing: the `payloadClassName` fallback a few lines up
is derived from the same stale `cfg.recordMergeMode`, so it resolves to
`OverwriteWithLatestAvroPayload`, and the second clause would skip the check
even if only the merge-mode clause were fixed. It might be cleanest to derive
both from `tableConfig`, 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]
