You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "linliu-code (via GitHub)" <gi...@apache.org> on 2026/09/13 07:11:37 UTC
[PR] fix(spark): honor event time ordering for COW MERGE INTO deletes [hudi]
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(spark): honor event time ordering for COW MERGE INTO deletes [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19934:
URL: https://github.com/apache/hudi/pull/19934#issuecomment-5651916878
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b76ee734e33b89977f636c162ed95b3dfcec752 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(spark): honor event time ordering for COW MERGE INTO deletes [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19934:
URL: https://github.com/apache/hudi/pull/19934#issuecomment-5652180802
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17071",
"triggerID" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b76ee734e33b89977f636c162ed95b3dfcec752 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17071)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(spark): honor event time ordering for COW MERGE INTO deletes [hudi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19934:
URL: https://github.com/apache/hudi/pull/19934#issuecomment-5652071447
## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/19934?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `7 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 80.22%. Comparing base ([`1f5a4fe`](https://app.codecov.io/gh/apache/hudi/commit/1f5a4fe53aaadd69d91a208a833ace990ae06122?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`8b76ee7`](https://app.codecov.io/gh/apache/hudi/commit/8b76ee734e33b89977f636c162ed95b3dfcec752?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/hudi/pull/19934?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...k/sql/hudi/command/payload/ExpressionPayload.scala](https://app.codecov.io/gh/apache/hudi/pull/19934?src=pr&el=tree&filepath=hudi-spark-datasource%2Fhudi-spark-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fspark%2Fsql%2Fhudi%2Fcommand%2Fpayload%2FExpressionPayload.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3BhcmstY29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2h1ZGkvY29tbWFuZC9wYXlsb2FkL0V4cHJlc3Npb25QYXlsb2FkLnNjYWxh) | 0.00% | [7 Missing :warning: ](https://app.codecov.io/gh/apache/hudi/pull/19934?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19934 +/- ##
============================================
- Coverage 80.23% 80.22% -0.01%
- Complexity 34747 34748 +1
============================================
Files 2546 2546
Lines 142608 142614 +6
Branches 17362 17364 +2
============================================
- Hits 114419 114417 -2
- Misses 20277 20283 +6
- Partials 7912 7914 +2
```
| [Components](https://app.codecov.io/gh/apache/hudi/pull/19934/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [hudi-common](https://app.codecov.io/gh/apache/hudi/pull/19934/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.85% <ΓΈ> (ΓΈ)` | |
| [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/19934/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.33% <ΓΈ> (ΓΈ)` | |
| [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/19934/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.56% <ΓΈ> (-0.01%)` | :arrow_down: |
| [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/19934/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.23% <0.00%> (-0.03%)` | :arrow_down: |
| [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/19934/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.19% <ΓΈ> (ΓΈ)` | |
| [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/19934/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `69.99% <ΓΈ> (ΓΈ)` | |
| [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/19934/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.89% <ΓΈ> (ΓΈ)` | |
| [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/19934/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.02% <ΓΈ> (ΓΈ)` | |
| [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/19934/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.61% <ΓΈ> (ΓΈ)` | |
| [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/19934/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.34% <ΓΈ> (ΓΈ)` | |
| [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/19934/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `80.99% <ΓΈ> (ΓΈ)` | |
| [hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/19934/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.96% <ΓΈ> (ΓΈ)` | |
| [Flag](https://app.codecov.io/gh/apache/hudi/pull/19934/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [flink-integration-tests](https://app.codecov.io/gh/apache/hudi/pull/19934/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `49.05% <ΓΈ> (+<0.01%)` | :arrow_up: |
| [hadoop-mr-java-client](https://app.codecov.io/gh/apache/hudi/pull/19934/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.88% <ΓΈ> (-0.02%)` | :arrow_down: |
| [integration-tests](https://app.codecov.io/gh/apache/hudi/pull/19934/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `13.45% <0.00%> (-0.01%)` | :arrow_down: |
| [spark-client-hadoop-common](https://app.codecov.io/gh/apache/hudi/pull/19934/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.53% <0.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files with missing lines](https://app.codecov.io/gh/apache/hudi/pull/19934?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [...k/sql/hudi/command/payload/ExpressionPayload.scala](https://app.codecov.io/gh/apache/hudi/pull/19934?src=pr&el=tree&filepath=hudi-spark-datasource%2Fhudi-spark-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fspark%2Fsql%2Fhudi%2Fcommand%2Fpayload%2FExpressionPayload.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3BhcmstY29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2h1ZGkvY29tbWFuZC9wYXlsb2FkL0V4cHJlc3Npb25QYXlsb2FkLnNjYWxh) | `69.29% <0.00%> (-2.33%)` | :arrow_down: |
... and [5 files with indirect coverage changes](https://app.codecov.io/gh/apache/hudi/pull/19934/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(spark): honor event time ordering for COW MERGE INTO deletes [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19934:
URL: https://github.com/apache/hudi/pull/19934#issuecomment-5652320815
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17071",
"triggerID" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"triggerType" : "PUSH"
}, {
"hash" : "0d0cc9d1fe575f9208e7e4c0591a2e4e7c43d445",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17074",
"triggerID" : "0d0cc9d1fe575f9208e7e4c0591a2e4e7c43d445",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b76ee734e33b89977f636c162ed95b3dfcec752 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17071)
* 0d0cc9d1fe575f9208e7e4c0591a2e4e7c43d445 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17074)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(spark): honor event time ordering for COW MERGE INTO deletes [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19934:
URL: https://github.com/apache/hudi/pull/19934#issuecomment-5652741997
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17071",
"triggerID" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"triggerType" : "PUSH"
}, {
"hash" : "0d0cc9d1fe575f9208e7e4c0591a2e4e7c43d445",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17074",
"triggerID" : "0d0cc9d1fe575f9208e7e4c0591a2e4e7c43d445",
"triggerType" : "PUSH"
}, {
"hash" : "9e99bb4fb1cb0f03754538962728214722675000",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "9e99bb4fb1cb0f03754538962728214722675000",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0d0cc9d1fe575f9208e7e4c0591a2e4e7c43d445 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17074)
* 9e99bb4fb1cb0f03754538962728214722675000 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(spark): honor event time ordering for COW MERGE INTO deletes [hudi]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code commented on PR #19934:
URL: https://github.com/apache/hudi/pull/19934#issuecomment-5704671209
Closing unmerged β we are reversing the direction of this fix.
Instead of making COW `MERGE INTO ... WHEN MATCHED THEN DELETE` honor event time ordering, the plan is now to make **MOR ignore** the ordering value for SQL merge-into deletes, so both table types apply a SQL-issued delete unconditionally. Rationale: `DELETE FROM` already ignores ordering values on both table types and both merge modes (see `TestDeleteFromTable`), so aligning MIT with it is more consistent with the intent of a SQL delete than making MIT the one SQL surface where a delete can be silently dropped. Ingestion paths (Deltastreamer / CDC) keep respecting ordering values and are unaffected.
Leaving #17372 open: the inconsistency it reports is real, only the intended resolution direction has changed. A replacement PR will follow.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(spark): honor event time ordering for COW MERGE INTO deletes [hudi]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code closed pull request #19934: fix(spark): honor event time ordering for COW MERGE INTO deletes
URL: https://github.com/apache/hudi/pull/19934
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(spark): honor event time ordering for COW MERGE INTO deletes [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19934:
URL: https://github.com/apache/hudi/pull/19934#issuecomment-5652298069
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17071",
"triggerID" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"triggerType" : "PUSH"
}, {
"hash" : "0d0cc9d1fe575f9208e7e4c0591a2e4e7c43d445",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "0d0cc9d1fe575f9208e7e4c0591a2e4e7c43d445",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b76ee734e33b89977f636c162ed95b3dfcec752 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17071)
* 0d0cc9d1fe575f9208e7e4c0591a2e4e7c43d445 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(spark): honor event time ordering for COW MERGE INTO deletes [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19934:
URL: https://github.com/apache/hudi/pull/19934#discussion_r4000399387
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/payload/ExpressionPayload.scala:
##########
@@ -163,6 +163,37 @@ class ExpressionPayload(@transient record: GenericRecord,
}
}
+ /**
+ * Weighs a matched delete against the record already in storage.
+ *
+ * A matched delete carries the source row's ordering value, so on a table that orders by event
+ * time it has to lose to a stored record with a higher ordering value, exactly as the update
+ * branch does through [[doRecordMerge]]. Returning the stored record leaves it untouched.
+ *
+ * The comparison needs the stored record, which only `combineAndGetUpdateValue` supplies. The
+ * `getInsertValue` entry point passes none and the delete is written into a delete block with
+ * its ordering value, so the reader weighs it there instead.
+ *
+ * No incoming record is handed to `needUpdatingPersistedRecord`: it would look the ordering
+ * fields up by name on the joined record, whose fields are all renamed by [[mergeSchema]], so
+ * every lookup misses. Passing none makes it fall back to this payload's own ordering value,
+ * which was read from the source row when the record was created.
+ */
+ private def processDelete(targetRecord: Option[IndexedRecord],
+ properties: Properties): HOption[IndexedRecord] = {
+ val originalPayload = properties.getProperty(PAYLOAD_ORIGINAL_AVRO_PAYLOAD)
+ // Commit time ordering resolves to OverwriteWithLatestAvroPayload, where the delete always
+ // wins. A custom payload keeps its existing semantics rather than having one guessed for it.
+ // TODO(HUDI-8915): EventTimeAvroPayload also infers EVENT_TIME_ORDERING and is not covered.
+ if (targetRecord.isEmpty || !classOf[DefaultHoodieRecordPayload].getName.equals(originalPayload)) {
+ HOption.empty()
+ } else if (needUpdatingPersistedRecord(targetRecord.get, HOption.empty(), properties)) {
+ HOption.empty()
+ } else {
+ HOption.of(targetRecord.get)
Review Comment:
π€ When the delete loses, this returns the stored record, which the merge handles treat as an update and rewrite with the new `_hoodie_commit_time` (vs. `SENTINEL`, which copies the old row untouched and is what `DefaultHoodieRecordPayload` does for the same case). I see the update branch already behaves this way so this is consistent, but is the commit-time bump for a no-op delete intended? It would surface the row in incremental queries even though nothing changed.
<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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(spark): honor event time ordering for COW MERGE INTO deletes [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19934:
URL: https://github.com/apache/hudi/pull/19934#issuecomment-5651941279
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17071",
"triggerID" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b76ee734e33b89977f636c162ed95b3dfcec752 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17071)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(spark): honor event time ordering for COW MERGE INTO deletes [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19934:
URL: https://github.com/apache/hudi/pull/19934#issuecomment-5652412611
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17071",
"triggerID" : "8b76ee734e33b89977f636c162ed95b3dfcec752",
"triggerType" : "PUSH"
}, {
"hash" : "0d0cc9d1fe575f9208e7e4c0591a2e4e7c43d445",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17074",
"triggerID" : "0d0cc9d1fe575f9208e7e4c0591a2e4e7c43d445",
"triggerType" : "PUSH"
}, {
"hash" : "9e99bb4fb1cb0f03754538962728214722675000",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "9e99bb4fb1cb0f03754538962728214722675000",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b76ee734e33b89977f636c162ed95b3dfcec752 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17071)
* 0d0cc9d1fe575f9208e7e4c0591a2e4e7c43d445 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17074)
* 9e99bb4fb1cb0f03754538962728214722675000 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(spark): honor event time ordering for COW MERGE INTO deletes [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #19934:
URL: https://github.com/apache/hudi/pull/19934#discussion_r4002003212
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/payload/ExpressionPayload.scala:
##########
@@ -163,6 +163,37 @@ class ExpressionPayload(@transient record: GenericRecord,
}
}
+ /**
+ * Weighs a matched delete against the record already in storage.
+ *
+ * A matched delete carries the source row's ordering value, so on a table that orders by event
+ * time it has to lose to a stored record with a higher ordering value, exactly as the update
+ * branch does through [[doRecordMerge]]. Returning the stored record leaves it untouched.
+ *
+ * The comparison needs the stored record, which only `combineAndGetUpdateValue` supplies. The
+ * `getInsertValue` entry point passes none and the delete is written into a delete block with
+ * its ordering value, so the reader weighs it there instead.
+ *
+ * No incoming record is handed to `needUpdatingPersistedRecord`: it would look the ordering
+ * fields up by name on the joined record, whose fields are all renamed by [[mergeSchema]], so
+ * every lookup misses. Passing none makes it fall back to this payload's own ordering value,
+ * which was read from the source row when the record was created.
+ */
+ private def processDelete(targetRecord: Option[IndexedRecord],
+ properties: Properties): HOption[IndexedRecord] = {
+ val originalPayload = properties.getProperty(PAYLOAD_ORIGINAL_AVRO_PAYLOAD)
+ // Commit time ordering resolves to OverwriteWithLatestAvroPayload, where the delete always
+ // wins. A custom payload keeps its existing semantics rather than having one guessed for it.
+ // TODO(HUDI-8915): EventTimeAvroPayload also infers EVENT_TIME_ORDERING and is not covered.
+ if (targetRecord.isEmpty || !classOf[DefaultHoodieRecordPayload].getName.equals(originalPayload)) {
Review Comment:
`!classOf[DefaultHoodieRecordPayload].getName.equals(originalPayload)` this compute can be moved outside of the per-record process.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org