hudi-agent commented on code in PR #19908:
URL: https://github.com/apache/hudi/pull/19908#discussion_r4010299729
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieCreateRecordUtils.scala:
##########
@@ -155,9 +155,20 @@ object HoodieCreateRecordUtils {
} else {
avroRecWithoutMeta
}
- val hoodieRecord = if (shouldCombine && !orderingFields.isEmpty) {
+ // `shouldCombine` says whether the incoming batch needs
de-duplicating, which is not the
+ // same question as whether this record needs an ordering value.
Prepped Spark SQL writes
+ // emit one row per key and so set it false, which used to leave
the record with no
+ // ordering value on tables that order by event time.
+ // Deletes take the ordering value too. A delete left on the
default is treated as commit
+ // time ordered by
BufferedRecordMergerFactory#shouldKeepNewerRecord, so excluding them
+ // here would let a stale delete remove a record with a higher
ordering value, while the
+ // same delete written through a path where `shouldCombine` is
true would correctly lose.
+ // A null ordering field stays tolerated for a delete, which may
carry only its key: the
+ // value falls back to the default rather than failing the write.
+ val shouldComputeOrderingValue = shouldCombine ||
requiresOrderingValue
+ val hoodieRecord = if (shouldComputeOrderingValue &&
!orderingFields.isEmpty) {
val orderingVal = getOrderingValue(orderingFields, avroRec,
hoodieKey.getRecordKey,
- consistentLogicalTimestampEnabled, requiresOrderingValue)
+ consistentLogicalTimestampEnabled, requiresOrderingValue &&
!isDelete)
Review Comment:
🤖 nit: `requiresOrderingValue && !isDelete` as a bare positional boolean is
hard to read at the call site — could you bind it to a named val (e.g.
`failOnNullOrderingValue`) or use a named argument so the "deletes may have a
null ordering field" intent is visible without reading the comment block above?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
