lokeshj1703 commented on code in PR #20049:
URL: https://github.com/apache/hudi/pull/20049#discussion_r4182897184
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/HoodieStreamerUtils.java:
##########
@@ -130,6 +135,13 @@ public static Option<JavaRDD<HoodieRecord>>
createHoodieRecords(HoodieStreamer.C
? OrderingValues.create(orderingFieldsStr.split(","),
field -> (Comparable)
HoodieAvroUtils.getNestedFieldVal(gr, field, false,
useConsistentLogicalTimestamp))
: null;
+ if (requiresOrderingValue &&
OrderingValues.isMissing(orderingValue)) {
+ throw new IllegalArgumentException(
+ "Ordering fields '" + orderingFieldsStr + "' resolved
to a null value for record key '"
+ + hoodieKey.getRecordKey() + "'. Please ensure all
records carry non-null values for "
+ + "the ordering fields, or use a merge mode or
payload class that does not order "
+ + "(e.g., COMMIT_TIME_ORDERING or
OverwriteWithLatestAvroPayload).");
Review Comment:
Good question, and it deserved measurement rather than an argument, so I ran
it on each line.
| | 0.x | 1.x before | 1.x after |
| --- | --- | --- | --- |
| record creation, no error table | throws `HoodieException` | accepted |
throws `IllegalArgumentException` |
| record creation, with error table | quarantined `RECORD_CREATION` |
accepted | quarantined `RECORD_CREATION` |
| outcome at merge | never reached | live row silently deleted | never
reached |
| delete exempt? | no | n/a | no |
On 1.x today a MOR delete carrying a null ordering value removes a live row
with a higher ordering value, unconditionally, with nothing logged. I checked
both the payload path and the file-group-reader path and they agree, and
`master` (1.3.0-SNAPSHOT) and the 1.1 line agree too. I had expected the
file-group-reader path to NPE in `shouldKeepNewerRecord` and it does not — the
probe corrected me.
So "deletes skip the check and carry `OrderingValues.getDefault()`" is what
1.x effectively does today, and the result is the silent deletion above rather
than a benign default. `isCommitTimeOrderingDelete()` is `isDelete() &&
OrderingValues.isDefault(orderingValue)`, so substituting the default is
precisely what turns a stale delete into an unconditional one.
0.x rejects it instead, deletes included: `HoodieRecordUtils.loadPayload`
there has no null short-circuit, so `BaseAvroPayload` throws, with
`isDeletedRecord` already set before the check. I ran all four combinations on
0.14.1 to confirm. 1.x later added that short-circuit, which made the guard
unreachable; this PR restores the 0.x behaviour.
On your second point you are right and I have taken it: the body now carries
this table and states the MOR impact explicitly.
--
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]
