wombatu-kun commented on code in PR #19295:
URL: https://github.com/apache/hudi/pull/19295#discussion_r3679361621
##########
hudi-trino/src/main/java/io/trino/plugin/hudi/reader/HudiTrinoReaderContext.java:
##########
@@ -243,9 +267,8 @@ protected Option<HoodieRecordMerger>
getRecordMerger(RecordMergeMode mergeMode,
// Dispatch on the table's merge mode, mirroring
HoodieAvroReaderContext. The Trino reader
// operates on IndexedRecord, so the Avro mergers apply directly.
Using the read-time merger
// (combineAndGetUpdateValue) rather than a fixed preCombine merger
keeps COMMIT_TIME_ORDERING
- // and custom-payload tables correct on MoR reads.
- // TODO(apache/hudi#18898): add MoR read tests for delete markers and
custom payloads to
- // exercise the EVENT_TIME_ORDERING (combineAndGetUpdateValue) and
CUSTOM branches below.
+ // and custom-payload tables correct on MoR reads;
TestHudiMorMergeModeSemantics and
+ // TestHudiMorPayloadSemantics cover these branches end-to-end
(apache/hudi#18898).
Review Comment:
`BufferedRecordMergerFactory.create` returns
`PartialUpdateBufferedRecordMerger` ahead of the mode switch whenever a log
block carries `IS_PARTIAL`, and that merger calls `partialMerge` on the merger
returned here - neither `HoodieAvroRecordMerger` nor
`OverwriteWithLatestMerger` overrides it, so the ordering arms are reachable
and hit the throwing default. Can the comment stop at "not covered by these
suites" and keep the TODO, rather than stating those arms are not testable?
--
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]
