linliu-code opened a new pull request, #19934:
URL: https://github.com/apache/hudi/pull/19934

   ### Describe the issue this Pull Request addresses
   
   closes #17372
   
   ### Summary and Changelog
   
   `ExpressionPayload.processMatchedRecord` compares ordering values in its 
update branch, through
   `doRecordMerge` and 
`DefaultHoodieRecordPayload.needUpdatingPersistedRecord`, but its delete branch
   returned `HOption.empty()` without ever reading the `targetRecord` it was 
handed. On a copy on write
   table that orders by event time, a `MERGE INTO ... WHEN MATCHED THEN DELETE` 
whose source ordering
   value was older than the stored record deleted it anyway. Merge on read was 
unaffected: it supplies
   no target record and writes the delete into a delete block with its ordering 
value, so the reader
   weighs it there. That is why the two table types disagreed.
   
   The delete branch now weighs the delete against the stored record. It is 
gated the same way the
   update branch is: merge on read is untouched because it passes no target, 
and commit time ordering
   (which resolves to `OverwriteWithLatestAvroPayload`) and custom payloads 
keep their existing
   semantics. `needUpdatingPersistedRecord` is given no incoming record, so it 
falls back to this
   payload's own ordering value, read from the source row at record creation. 
Passing the joined record
   instead would be wrong: `mergeSchema` renames every field to 
`source_*`/`target_*`, so a lookup by
   ordering field name always misses there.
   
   `TestMergeModeEventTimeOrdering` already contained the assertions for this, 
fenced behind
   `if ("mor".equals(tableType))` with a `HUDI-8915` comment. The fence is 
removed, so copy on write now
   runs them too. Most of that file's diff is the resulting dedent; `git diff 
-w` shows the real change.
   
   ### Impact
   
   User visible behavior change on a stable SQL path: on a copy on write table 
with
   `EVENT_TIME_ORDERING`, a `MERGE INTO ... WHEN MATCHED THEN DELETE` whose 
source ordering value is
   lower than the stored record's now leaves the row in place, where it 
previously removed it. This
   matches what merge on read has always done and what the same table does for 
updates. Commit time
   ordered tables, custom payloads, merge on read, `DELETE FROM` and the 
datasource paths are all
   unchanged. No public API change.
   
   ### Risk Level
   
   low
   
   `TestMergeModeEventTimeOrdering` goes from 18 to 24 tests: six copy on write 
merge cases that could
   not run before, across table versions 6, 8, 9 and 10. Reverting only the new 
call fails exactly those
   six, each missing the stale row (`Expected Array([2,B,20.0,100], ...), but 
got Array(...)` with
   `[2,B,20.0,100]` absent), while the six merge on read cases and the twelve 
non-merge cases stay
   green. Full suite passes with checkstyle and scalastyle enabled.
   
   One part is not covered by a test: passing no incoming record to 
`needUpdatingPersistedRecord`
   rather than the joined record. With a single ordering field the two are 
indistinguishable, because
   the failed lookup collapses to null and `.orElse(orderingVal)` rescues it. 
They differ only with two
   or more ordering fields, where `OrderingValues.create` returns an 
`ArrayComparable` of nulls that
   `ArrayComparable.compareTo` would dereference. That case cannot be reached 
from Spark SQL today:
   `HoodieOptionConfig.validateTable` treats the ordering fields value as a 
single column name and
   `HoodieCatalogTable` runs it on every path, so a multi ordering field table 
cannot enter the catalog
   at all, whether created by SQL or registered from an existing location. 
Filed separately.
   
   ### Documentation Update
   
   None. No new config, no default value change, no user facing feature change.
   
   ### Contributor's checklist
   
   - [x] Read through [contributor's 
guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [x] Enough context is provided in the sections above
   - [x] Adequate tests were added if applicable
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to