linliu-code commented on PR #19908: URL: https://github.com/apache/hudi/pull/19908#issuecomment-5718752472
Closing this for now. On current master the condition this PR changes is not reachable with the default configuration: hoodie.write.merge.handle.class defaults to FileGroupReaderBasedMergeHandle, so requiresPayload is false and records already take the payload free path, where the ordering value is derived from the record data. The gap only opens when that config points at a merge handle that is not FileGroupReaderBasedMergeHandle, for example HoodieWriteMergeHandle, HoodieSortedMergeHandle or HoodieMergeHandleWithChangeLog. In that case a prepped Spark SQL write (UPDATE or DELETE, where shouldCombine is false) builds the payload through its single argument constructor and the record serves OrderingValues.getDefault(), an Integer, instead of the ordering field value. Comparing that against a long ordering column throws ClassCastException at merge time. Two notes for anyone picking this up later: 1. Removing the requiresPayload gate entirely is an alternative fix, but it only helps payload classes listed in HoodieRecordUtils.DEPRECATED_PAYLOADS, since isPayloadClassDeprecated is an exact set membership test on the class name and is not subclass aware. Any other payload class, including subclasses of the listed ones, keeps the old behaviour. 2. MERGE INTO is not affected, because MergeIntoHoodieTableCommand sets COMBINE_BEFORE_UPSERT from whether ordering fields are configured, which forces the ordering value to be computed. The HoodieStreamer ingestion path carries the same shouldCombine conflation in HoodieStreamerUtils and is not touched by this PR; op UPSERT is safe there because shouldCombine is forced true, while op DELETE and UPSERT_PREPPED are not. -- 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]
