You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "wombatu-kun (via GitHub)" <gi...@apache.org> on 2026/10/16 05:48:40 UTC
[PR] fix(spark): keep Java ordering values when log compaction rewrites deletes [hudi]
wombatu-kun opened a new pull request, #20366:
URL: https://github.com/apache/hudi/pull/20366
### Describe the issue this Pull Request addresses
Closes #20365
Spark log compaction failed on table versions 8 and 9 whenever it rewrote a delete with a string or decimal ordering value. The file group reader had already converted those values to Spark engine types, and `HoodieDeleteBlock` rejects them with `UnsupportedOperationException` from `wrapValueIntoAvro`.
### Summary and Changelog
- New `RecordContext.convertOrderingValueToJava` turns an engine-typed ordering value into a Java type that a delete block can store. It is the identity by default.
- `BaseSparkInternalRecordContext` overrides it: `HoodieUTF8String` becomes `String` and Spark `Decimal` becomes `BigDecimal`, element by element when there are several ordering fields.
- `FileGroupReaderBasedInlineLogAppendHandle` converts the ordering value of every delete before it rebuilds the record for the compacted log file.
- `TestMORDataSource` runs log compaction over a delete block with string and decimal ordering, for both record types, at versions 8 and 9. After compaction, an older update must leave the key deleted and a newer one must bring it back.
- `TestBaseSparkInternalRowReaderContext` covers the conversion itself.
### Impact
Log compaction of Spark MOR tables at versions 8 and 9 works again for string and decimal ordering fields, and the compacted deletes keep their ordering values. Version 9 is the default of 1.2.x. The fix applies with the Avro record merger and the Spark one, because log compaction always reads through the Spark context.
Other ordering types already reached the delete block as Java values and are written as before. Version 10 tables use the native log handle, which is unchanged. Flink pipelines only log-compact the metadata table, which reads through the Avro context, so no Flink override is added. #20341 fixes the same failure for deletes that win Spark precombine.
### Risk Level
low
The only change is the type of the ordering value that log compaction writes into inline delete blocks.
Verified:
- On master, every row of the new test fails with `Unsupported type of the value (class org.apache.hudi.Spark3HoodieUTF8String)` or `(class org.apache.spark.sql.types.Decimal)`.
- Removing either the Spark override or the handle change brings that failure back.
- With `hoodie.log.compaction.inline=true`, the same failure reaches a plain datasource write.
### Documentation Update
none
### 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
--
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): keep Java ordering values when log compaction rewrites deletes [hudi]
Posted by "wombatu-kun (via GitHub)" <gi...@apache.org>.
wombatu-kun commented on code in PR #20366:
URL: https://github.com/apache/hudi/pull/20366#discussion_r4303633226
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/FileGroupReaderBasedNativeLogAppendHandle.java:
##########
@@ -97,8 +98,13 @@ public void doAppend() {
.withSortOutput(hoodieTable.requireSortedRecords())
// instead of using config.enableOptimizedLogBlocksScan(), we set to true as log compaction blocks only supported in scanV2
.build()) {
+ // Log compaction reads deletes through the engine reader context. A table that writes them through another
+ // record context, e.g. the Avro one for the Avro record type, can only store Java ordering values.
+ boolean javaOrderingForDeletes = readerContext.getRecordContext().getEngineRecordType()
+ != hoodieTable.getRecordContextForWrite().getEngineRecordType();
recordItr = new CloseableMappingIterator<>(fileGroupReader.getLogRecordsOnly(), record -> {
- HoodieRecord<T> hoodieRecord = readerContext.getRecordContext().constructHoodieRecord(record);
+ HoodieRecord<T> hoodieRecord = readerContext.getRecordContext().constructHoodieRecord(
+ javaOrderingForDeletes ? withJavaOrderingValue(record) : record);
Review Comment:
Done bbc84e2cb9af: the native handle is back to master, and `HoodieNativeLogFormatWriter` now stores the Java copy of an empty record that another record context built, the value the inline `writeDelete()` already stores through `getOrderingValueAsJava()`. Log compaction keeps the Spark reader because the Avro one breaks decimal ordering: with your change, a decimal-ordered key whose update and older delete meet in log compaction, now part of `testLogCompactionRewritesDeletesWithEngineOrdering`, fails at version 9 with `ClassCastException: GenericData$Fixed cannot be cast to BigDecimal` in `shouldKeepNewerRecord`, since delete blocks yield `BigDecimal` and data blocks raw `GenericData$Fixed`.
--
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): keep Java ordering values when log compaction rewrites deletes [hudi]
Posted by "cshuo (via GitHub)" <gi...@apache.org>.
cshuo commented on PR #20366:
URL: https://github.com/apache/hudi/pull/20366#issuecomment-6287305800
After further analysis, I think we can address this with two changes:
- **Use a converter in `HoodieEmptyRecord`.** Keep the engine ordering value for comparisons and convert it to Java only when `getOrderingValueAsJava` is called, instead of eagerly creating and storing a second value. A shared, stateless converter avoids allocating a converter per record or retaining the reader context.
- **Align the reader and writer record contexts for native log compaction.** The native delete writer should use the same record context that produced the ordering values. This removes the need for special-case logic in `HoodieNativeLogFormatWriter` to handle empty records from a different context. The storage-form conversion in [#20373](https://github.com/apache/hudi/pull/20373) is still needed.
I've put together an alternative implementation along these lines in [#20396](https://github.com/apache/hudi/pull/20396). All six log compaction regression cases from this PR pass with that implementation. Feedback and review are welcome!
--
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): keep Java ordering values when log compaction rewrites deletes [hudi]
Posted by "wombatu-kun (via GitHub)" <gi...@apache.org>.
wombatu-kun commented on PR #20366:
URL: https://github.com/apache/hudi/pull/20366#issuecomment-6291952324
Closing in favor of #20396; I left one note there on ORC base files.
--
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): keep Java ordering values when log compaction rewrites deletes [hudi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #20366:
URL: https://github.com/apache/hudi/pull/20366#issuecomment-6209416816
## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/20366?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 `6 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 80.73%. Comparing base ([`c167f7b`](https://app.codecov.io/gh/apache/hudi/commit/c167f7b392441cfb86b1dd64bb6c2708878e80d6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`e46b77d`](https://app.codecov.io/gh/apache/hudi/commit/e46b77db2013778d5da9c4ca08c71249bbcee0b5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/hudi/pull/20366?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../io/FileGroupReaderBasedInlineLogAppendHandle.java](https://app.codecov.io/gh/apache/hudi/pull/20366?src=pr&el=tree&filepath=hudi-client%2Fhudi-client-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fio%2FFileGroupReaderBasedInlineLogAppendHandle.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jbGllbnQvaHVkaS1jbGllbnQtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2lvL0ZpbGVHcm91cFJlYWRlckJhc2VkSW5saW5lTG9nQXBwZW5kSGFuZGxlLmphdmE=) | 0.00% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/hudi/pull/20366?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...a/org/apache/hudi/common/engine/RecordContext.java](https://app.codecov.io/gh/apache/hudi/pull/20366?src=pr&el=tree&filepath=hudi-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fcommon%2Fengine%2FRecordContext.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL2VuZ2luZS9SZWNvcmRDb250ZXh0LmphdmE=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/hudi/pull/20366?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 #20366 +/- ##
============================================
- Coverage 80.73% 80.73% -0.01%
- Complexity 35201 35202 +1
============================================
Files 2553 2553
Lines 143629 143634 +5
Branches 17504 17504
============================================
+ Hits 115964 115966 +2
- Misses 19703 19707 +4
+ Partials 7962 7961 -1
```
| [Components](https://app.codecov.io/gh/apache/hudi/pull/20366/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/20366/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `84.17% <0.00%> (+<0.01%)` | :arrow_up: |
| [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/20366/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.94% <0.00%> (-0.02%)` | :arrow_down: |
| [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/20366/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `86.07% <ø> (ø)` | |
| [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/20366/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `74.19% <ø> (ø)` | |
| [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/20366/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/20366/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.80% <ø> (ø)` | |
| [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/20366/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.18% <ø> (ø)` | |
| [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/20366/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.78% <ø> (ø)` | |
| [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/20366/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.48% <ø> (ø)` | |
| [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/20366/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.41% <ø> (ø)` | |
| [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/20366/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.00% <ø> (ø)` | |
| [hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/20366/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.90% <ø> (ø)` | |
| [Flag](https://app.codecov.io/gh/apache/hudi/pull/20366/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hadoop-mr-java-client](https://app.codecov.io/gh/apache/hudi/pull/20366/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.94% <0.00%> (-0.01%)` | :arrow_down: |
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/20366?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...rg/apache/hudi/BaseSparkInternalRecordContext.java](https://app.codecov.io/gh/apache/hudi/pull/20366?src=pr&el=tree&filepath=hudi-client%2Fhudi-spark-client%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fhudi%2FBaseSparkInternalRecordContext.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jbGllbnQvaHVkaS1zcGFyay1jbGllbnQvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9odWRpL0Jhc2VTcGFya0ludGVybmFsUmVjb3JkQ29udGV4dC5qYXZh) | `89.61% <ø> (ø)` | |
| [...a/org/apache/hudi/common/engine/RecordContext.java](https://app.codecov.io/gh/apache/hudi/pull/20366?src=pr&el=tree&filepath=hudi-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fcommon%2Fengine%2FRecordContext.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL2VuZ2luZS9SZWNvcmRDb250ZXh0LmphdmE=) | `84.81% <0.00%> (-1.09%)` | :arrow_down: |
| [.../io/FileGroupReaderBasedInlineLogAppendHandle.java](https://app.codecov.io/gh/apache/hudi/pull/20366?src=pr&el=tree&filepath=hudi-client%2Fhudi-client-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fio%2FFileGroupReaderBasedInlineLogAppendHandle.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jbGllbnQvaHVkaS1jbGllbnQtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2lvL0ZpbGVHcm91cFJlYWRlckJhc2VkSW5saW5lTG9nQXBwZW5kSGFuZGxlLmphdmE=) | `0.00% <0.00%> (ø)` | |
... and [1 file with indirect coverage changes](https://app.codecov.io/gh/apache/hudi/pull/20366/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): keep Java ordering values when log compaction rewrites deletes [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #20366:
URL: https://github.com/apache/hudi/pull/20366#discussion_r4291408276
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/FileGroupReaderBasedInlineLogAppendHandle.java:
##########
@@ -116,6 +117,18 @@ public void doAppend() {
}
}
+ /**
+ * The file group reader holds ordering values in the engine type. A delete is written back into a delete block,
+ * which only stores Java types, so its ordering value is converted back.
+ */
+ private BufferedRecord<T> withJavaOrderingValue(BufferedRecord<T> record) {
+ if (!record.isDelete()) {
+ return record;
+ }
Review Comment:
🤖 nit: could you declare this as `Comparable<?>`-free but typed consistently, or add a short `@SuppressWarnings("rawtypes")`, since the raw `Comparable` is used here?
<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): keep Java ordering values when log compaction rewrites deletes [hudi]
Posted by "wombatu-kun (via GitHub)" <gi...@apache.org>.
wombatu-kun commented on code in PR #20366:
URL: https://github.com/apache/hudi/pull/20366#discussion_r4293851965
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/FileGroupReaderBasedInlineLogAppendHandle.java:
##########
@@ -116,6 +117,18 @@ public void doAppend() {
}
}
+ /**
+ * The file group reader holds ordering values in the engine type. A delete is written back into a delete block,
+ * which only stores Java types, so its ordering value is converted back.
+ */
+ private BufferedRecord<T> withJavaOrderingValue(BufferedRecord<T> record) {
+ if (!record.isDelete()) {
+ return record;
+ }
Review Comment:
Done b050060b85ae: the inline handle no longer has this helper, and the one left in the native handle holds a `Comparable<?>`.
--
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): keep Java ordering values when log compaction rewrites deletes [hudi]
Posted by "wombatu-kun (via GitHub)" <gi...@apache.org>.
wombatu-kun commented on code in PR #20366:
URL: https://github.com/apache/hudi/pull/20366#discussion_r4293868678
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/FileGroupReaderBasedInlineLogAppendHandle.java:
##########
@@ -103,7 +104,7 @@ public void doAppend() {
// instead of using config.enableOptimizedLogBlocksScan(), we set to true as log compaction blocks only supported in scanV2
.build()) {
recordItr = new CloseableMappingIterator<>(fileGroupReader.getLogRecordsOnly(), record -> {
- HoodieRecord<T> hoodieRecord = readerContext.getRecordContext().constructHoodieRecord(record);
+ HoodieRecord<T> hoodieRecord = readerContext.getRecordContext().constructHoodieRecord(withJavaOrderingValue(record));
Review Comment:
Done b050060b85ae: the inline handle no longer converts. The `HoodieEmptyRecord` branch of the Spark `constructHoodieRecord` now does, and `HoodieEmptyRecord.getOrderingValueAsJava()` returns that Java value while `getOrderingValue()` keeps the engine one, the same split `HoodieSparkRecord` has. Storing only the Java value breaks v10, whose native delete writer puts `getOrderingValue()` into a Spark row: `SPARK;decimal(10,2);10` then fails with `BigDecimal cannot be cast to Decimal` and log compaction drops the delete. This also covers deletes that win precombine: `testPrecombinedDeleteWithEngineOrdering` from #20341 passes on this branch without the `isEmpty()` guard. The native handle keeps a conversion only for the Avro record type, where Spark-read deletes reach the Avro delete writer.
--
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): keep Java ordering values when log compaction rewrites deletes [hudi]
Posted by "cshuo (via GitHub)" <gi...@apache.org>.
cshuo commented on code in PR #20366:
URL: https://github.com/apache/hudi/pull/20366#discussion_r4292580329
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/FileGroupReaderBasedInlineLogAppendHandle.java:
##########
@@ -103,7 +104,7 @@ public void doAppend() {
// instead of using config.enableOptimizedLogBlocksScan(), we set to true as log compaction blocks only supported in scanV2
.build()) {
recordItr = new CloseableMappingIterator<>(fileGroupReader.getLogRecordsOnly(), record -> {
- HoodieRecord<T> hoodieRecord = readerContext.getRecordContext().constructHoodieRecord(record);
+ HoodieRecord<T> hoodieRecord = readerContext.getRecordContext().constructHoodieRecord(withJavaOrderingValue(record));
Review Comment:
**Move the conversion to the `HoodieEmptyRecord` construction boundary**
`HoodieInlineLogAppendHandle.writeDelete()` already calls `HoodieRecord.getOrderingValueAsJava()`, so the append handle should not need to convert ordering values before constructing records.
The missing conversion is in the creation of `HoodieEmptyRecord`: it is engine-agnostic, and its `getOrderingValueAsJava()` returns the stored ordering value unchanged. Please convert the ordering value in the `HoodieEmptyRecord` branch of `RecordContext.constructHoodieRecord()`. This covers all callers constructing empty records and preserves engine-typed ordering values for engine-specific records.
--
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): keep Java ordering values when log compaction rewrites deletes [hudi]
Posted by "cshuo (via GitHub)" <gi...@apache.org>.
cshuo commented on code in PR #20366:
URL: https://github.com/apache/hudi/pull/20366#discussion_r4303123521
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/FileGroupReaderBasedNativeLogAppendHandle.java:
##########
@@ -97,8 +98,13 @@ public void doAppend() {
.withSortOutput(hoodieTable.requireSortedRecords())
// instead of using config.enableOptimizedLogBlocksScan(), we set to true as log compaction blocks only supported in scanV2
.build()) {
+ // Log compaction reads deletes through the engine reader context. A table that writes them through another
+ // record context, e.g. the Avro one for the Avro record type, can only store Java ordering values.
+ boolean javaOrderingForDeletes = readerContext.getRecordContext().getEngineRecordType()
+ != hoodieTable.getRecordContextForWrite().getEngineRecordType();
recordItr = new CloseableMappingIterator<>(fileGroupReader.getLogRecordsOnly(), record -> {
- HoodieRecord<T> hoodieRecord = readerContext.getRecordContext().constructHoodieRecord(record);
+ HoodieRecord<T> hoodieRecord = readerContext.getRecordContext().constructHoodieRecord(
+ javaOrderingForDeletes ? withJavaOrderingValue(record) : record);
Review Comment:
**Align the log-compaction reader with the write record type**
The changes to this handle should be unnecessary if log compaction selects a reader record context that matches its writer record context. In the non-metadata `LOG_COMPACT` branch of `HoodieCompactor`, could we use:
```java
readerContextFactory = context.getReaderContextFactoryForWrite(
metaClient, config.getRecordMerger().getRecordType(), config.getProps());
```
This selects an Avro reader for Avro writes and a Spark reader for Spark writes, eliminating the cross-context ordering conversion here. The existing metadata-table reader setup can remain unchanged.
--
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): keep Java ordering values when log compaction rewrites deletes [hudi]
Posted by "wombatu-kun (via GitHub)" <gi...@apache.org>.
wombatu-kun closed pull request #20366: fix(spark): keep Java ordering values when log compaction rewrites deletes
URL: https://github.com/apache/hudi/pull/20366
--
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