linliu-code commented on code in PR #19908:
URL: https://github.com/apache/hudi/pull/19908#discussion_r4007881456
##########
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
Review Comment:
Thanks — I checked this, and I don't think the MDT framing holds for this
particular flag.
`_hoodie.spark.sql.writes.prepped` (`SPARK_SQL_WRITES_PREPPED_KEY`) is set
in exactly four places on master, all of them user-facing SQL DML on ordinary
tables:
```
hudi-spark3-common/.../UpdateHoodieTableCommand.scala:60
hudi-spark3-common/.../DeleteHoodieTableCommand.scala:76
hudi-spark4-common/.../UpdateHoodieTableCommand.scala:61
hudi-spark4-common/.../DeleteHoodieTableCommand.scala:81
```
Nothing under `metadata/` sets or reads it, and the only other files that
mention it are the plumbing (`DataSourceOptions`, `HoodieSparkSqlWriter`,
`HoodieWriterUtils`). So `shouldCombine` being false here is not an MDT-only
situation: it is every `UPDATE` and every `DELETE` issued through Spark SQL
against a normal Hudi table.
That matches how this was found. The failure was a plain `UPDATE` on a
copy-on-write user table with a `bigint` precombine field and event-time
ordering, and it threw on every merged row:
```
java.lang.ClassCastException: class java.lang.Long cannot be cast to class
java.lang.Integer
at java.lang.Integer.compareTo(Integer.java:71)
at
BufferedRecordMergerFactory.shouldKeepNewerRecord(BufferedRecordMergerFactory.java:515)
at BufferedRecordMergerFactory$EventTimeRecordMerger.finalMerge(...)
at FileGroupRecordBuffer.hasNextBaseRecord(...)
at FileGroupReaderBasedMergeHandle.doMerge(...)
```
You are right that a separate prepped mechanism exists for the metadata
table, and that MDT does not do event-time merging — but that is
`UPSERT_PREPPED` / `preppedWriteOperation`, which is a different path from this
config key, and this change does not affect it.
One caveat on reachability worth stating plainly: the record only takes the
`HoodieAvroRecord` branch (and so serves the payload's `Integer` default) when
`requiresPayload` is true, which needs a merge handle that is not a
`FileGroupReaderBasedMergeHandle` subclass. On stock master with the default
handle, the same branch yields a `HoodieAvroIndexedRecord`, which derives the
ordering value from the record and is unaffected. So the loud failure needs a
non-default merge handle, while the underlying gate — a de-duplication flag
deciding whether a record gets its ordering value — is wrong independently of
that.
--
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]
