You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "voonhous (via GitHub)" <gi...@apache.org> on 2026/10/14 12:30:44 UTC
[PR] fix(spark): read shredded variant leaves like Spark 4 [hudi]
voonhous opened a new pull request, #20332:
URL: https://github.com/apache/hudi/pull/20332
**TLDR:** brings Hudi's read of shredded variant base files to parity with Spark 4. Each `typed_value` leaf is now read from its Parquet type, as Spark 4's own reader does, so leaves that came back wrong are rebuilt correctly and leaves Spark rejects fail when the file is opened.
Stacked on #20326 (write-side parity); only the last commit is new.
### Describe the issue this Pull Request addresses
Read side of #19442. `HoodieAvroParquetReader` rebuilt shredded variants from the footer's Avro conversion, which drops DECIMAL on INT32 / INT64, the unsigned flag, JSON and NANOS. So for files written by other engines:
- DECIMAL on INT32 / INT64 came back as the unscaled integer (1.50 as 150), with no error.
- Plain FLBA failed with a `ClassCastException`, and TIMESTAMP(MILLIS) with an NPE.
- TIME and TIMESTAMP(NANOS) came back as integers, where Spark 4 refuses the file.
Map of every case: https://github.com/apache/hudi/issues/19442#issuecomment-6165989504
### Summary and Changelog
- `HoodieAvroParquetReader` passes the file's Parquet schema to `HoodieVariantReconstruction`, which prepares one rebuilder per shredded variant column when the file is opened (records, both LIST encodings, map values).
- `VariantShreddingProvider.rebuildVariantRecord` becomes `prepareRebuild(columnPath, parquetGroup, unshreddedSchema)`.
- `Spark4VariantShreddingProvider` maps each leaf from its Parquet type the way Spark's `ParquetToSparkSchemaConverter` and `SparkShreddingUtils.buildVariantSchema` do, and decodes values per leaf. Dead code from the old path is removed.
- Tests in `TestSpark4VariantShreddingProvider`, `TestHoodieVariantReconstructionRoundTrip` (real files through the reader) and `TestHoodieVariantReconstruction`.
### Impact
Shredded files written by Hudi read as before. Files from other engines now read like Spark 4. A file Spark 4 refuses (TIME, TIMESTAMP(NANOS), FLOAT16, UUID, INTERVAL, MAP `typed_value`) now fails at open with an error naming the column, including such files written by Hudi before #20326 from a hand-written shredding schema.
### Risk Level
medium
<details><summary>Per-encoding behaviour (matches Spark 4.1.1)</summary>
| `typed_value` encoding | Read as |
|---|---|
| BOOLEAN, FLOAT, DOUBLE, INT32, INT64, INT(8/16/32/64 signed), DATE, TIMESTAMP(MICROS) | unchanged |
| STRING, ENUM, JSON on BINARY | string (JSON was binary) |
| BINARY, BSON, plain FLBA | binary (plain FLBA threw) |
| DECIMAL on INT32 / INT64 / BINARY / FLBA | decimal (INT32 / INT64 were unscaled integers) |
| TIMESTAMP(MILLIS), UTC / local | timestamp / timestamp_ntz, scaled to micros (threw) |
| INT(32,false) | long, unsigned (was negative above 2^31) |
| INT(64,false) | decimal(20,0) (was a signed long) |
| TIME, TIMESTAMP(NANOS), FLOAT16, UUID on FLBA(16), INTERVAL | rejected at open |
| INT96 | rejected; not supported by Hudi's Avro reader (Spark reads it as a timestamp) |
</details>
<details><summary>Known differences from Spark</summary>
- Spark applies datetime rebase to very old DATE / TIMESTAMP values; Hudi passes raw values through (Spark's CORRECTED mode).
- Spark throws `AnalysisException`; Hudi throws `HoodieException`.
- `spark.sql.legacy.parquet.nanosAsLong` is not honoured: NANOS is always rejected.
</details>
### 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): read shredded variant leaves like Spark 4 [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous commented on code in PR #20332:
URL: https://github.com/apache/hudi/pull/20332#discussion_r4336019514
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/io/storage/hadoop/HoodieVariantReconstruction.java:
##########
@@ -110,90 +124,153 @@ static HoodieVariantReconstruction create(HoodieSchema fileSchema, HoodieSchema
+ HoodieStorageConfig.PARQUET_VARIANT_SHREDDING_PROVIDER_CLASS.key()
+ " or add a provider implementation (e.g. the Spark variant module) to the classpath.");
}
+ for (VariantColumnRebuilder variantColumn : planner.variantColumns) {
+ variantColumn.prepare(provider);
+ }
return new HoodieVariantReconstruction(
- VariantSchemaUtils.toShreddedReadSchema(requestedSchema, fileSchema), provider, rootRebuilder);
+ VariantSchemaUtils.toShreddedReadSchema(requestedSchema, fileSchema), rootRebuilder);
}
/**
* Rebuilds shredded variant columns of {@code in} (read in the intermediate shredded shape) into
* a record conforming to the unshredded output schema.
*/
IndexedRecord reconstruct(IndexedRecord in) {
- return (IndexedRecord) rootRebuilder.rebuild(in, provider);
+ return (IndexedRecord) rootRebuilder.rebuild(in);
}
- /**
- * Plans the rebuild for one schema position, or returns {@code null} when nothing below it is a
- * shredded variant and the value can be passed through untouched. The walk is driven by the
- * output (requested) side because that is the shape the record read from parquet has; the file
- * side is matched into it by field name. Detection is anchored by the requested side because the
- * file schema usually comes from converting the parquet footer MessageType, which loses the
- * variant logical type; see VariantSchemaUtils.isShreddedVariantTarget (#19567). Records, array
- * elements and map values are all descended into, matching what the writers can emit; see
- * {@code VariantSchemaUtils.swapShreddedVariantFields} for what actually produces a nested
- * shredded file today (both the row and the AVRO write path shred variant record members at any
- * depth off the forced-shredding property; a variant that is directly an array element or a map
- * value shreds on neither path unless the write schema itself declares typed_value there).
- */
- private static Rebuilder buildRebuilder(HoodieSchema outputSchema, HoodieSchema fileSchema) {
- if (VariantSchemaUtils.isShreddedVariantTarget(fileSchema, outputSchema)) {
- return new VariantRebuilder(fileSchema.getNonNullType().getAvroSchema(),
- outputSchema.getNonNullType().getAvroSchema());
- }
- HoodieSchema output = outputSchema.getNonNullType();
- HoodieSchema file = fileSchema.getNonNullType();
- if (output.getType() != file.getType()) {
- return null;
+ /** Plans the rebuild of a file, collecting its shredded variant columns for preparation. */
+ private static final class Planner {
+ // Mirrors the file schema conversion's ADD_LIST_ELEMENT_RECORDS, which decides the list element.
Review Comment:
The two rules run on different groups. The planner's listElementType only walks the lists around a variant column (e.g. array<variant>) and mirrors parquet-avro (AvroSchemaConverter.isElementType plus ADD_LIST_ELEMENT_RECORDS), because it has to match the Avro shape the reader hands back. The provider's only applies to the typed_value list inside a variant and mirrors ParquetToSparkSchemaConverter.isElementType, since it validates the layout as Spark would read it. Sharing one rule would make one of them wrong. The only overlap is parquet-avro shaping typed_value before the provider decodes it, and they diverge there only when the element is itself a repeated group. Added a javadoc cross-reference on both in 6aa750d so a change to one points at the other.
--
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): read shredded variant leaves like Spark 4 [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous merged PR #20332:
URL: https://github.com/apache/hudi/pull/20332
--
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): read shredded variant leaves like Spark 4 [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous commented on PR #20332:
URL: https://github.com/apache/hudi/pull/20332#issuecomment-6168482694
@wombatu-kun This needs a review too, thank you.
--
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): read shredded variant leaves like Spark 4 [hudi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #20332:
URL: https://github.com/apache/hudi/pull/20332#issuecomment-6171396538
## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/20332?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 `40.44944%` with `53 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 78.01%. Comparing base ([`61dde86`](https://app.codecov.io/gh/apache/hudi/commit/61dde86311368a7d373e582f40b941db55aa60a4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`248a62a`](https://app.codecov.io/gh/apache/hudi/commit/248a62a16f7645c54ffd8adb9415dc4eb28b6e44?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/20332?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/storage/hadoop/HoodieVariantReconstruction.java](https://app.codecov.io/gh/apache/hudi/pull/20332?src=pr&el=tree&filepath=hudi-hadoop-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fio%2Fstorage%2Fhadoop%2FHoodieVariantReconstruction.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1oYWRvb3AtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2lvL3N0b3JhZ2UvaGFkb29wL0hvb2RpZVZhcmlhbnRSZWNvbnN0cnVjdGlvbi5qYXZh) | 41.53% | [23 Missing and 15 partials :warning: ](https://app.codecov.io/gh/apache/hudi/pull/20332?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...e/hudi/variant/Spark4VariantShreddingProvider.java](https://app.codecov.io/gh/apache/hudi/pull/20332?src=pr&el=tree&filepath=hudi-spark-datasource%2Fhudi-spark4-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fvariant%2FSpark4VariantShreddingProvider.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3Bhcms0LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS92YXJpYW50L1NwYXJrNFZhcmlhbnRTaHJlZGRpbmdQcm92aWRlci5qYXZh) | 6.25% | [15 Missing :warning: ](https://app.codecov.io/gh/apache/hudi/pull/20332?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 #20332 +/- ##
============================================
- Coverage 80.71% 78.01% -2.70%
+ Complexity 35144 33971 -1173
============================================
Files 2553 2553
Lines 143547 143679 +132
Branches 17478 17540 +62
============================================
- Hits 115860 112093 -3767
- Misses 19741 23424 +3683
- Partials 7946 8162 +216
```
| [Components](https://app.codecov.io/gh/apache/hudi/pull/20332/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/20332/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `82.54% <ø> (-1.64%)` | :arrow_down: |
| [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/20332/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.59% <ø> (-4.36%)` | :arrow_down: |
| [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/20332/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `86.07% <ø> (-0.01%)` | :arrow_down: |
| [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/20332/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `66.86% <6.25%> (-7.33%)` | :arrow_down: |
| [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/20332/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.19% <ø> (+0.03%)` | :arrow_up: |
| [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/20332/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/20332/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.74% <47.94%> (-2.43%)` | :arrow_down: |
| [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/20332/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.62% <ø> (ø)` | |
| [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/20332/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.38% <ø> (-0.15%)` | :arrow_down: |
| [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/20332/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.53% <ø> (-4.88%)` | :arrow_down: |
| [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/20332/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/20332/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `54.48% <ø> (ø)` | |
| [Flag](https://app.codecov.io/gh/apache/hudi/pull/20332/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/20332/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.94% <47.94%> (-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/20332?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...udi/io/storage/hadoop/HoodieAvroParquetReader.java](https://app.codecov.io/gh/apache/hudi/pull/20332?src=pr&el=tree&filepath=hudi-hadoop-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fio%2Fstorage%2Fhadoop%2FHoodieAvroParquetReader.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1oYWRvb3AtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2lvL3N0b3JhZ2UvaGFkb29wL0hvb2RpZUF2cm9QYXJxdWV0UmVhZGVyLmphdmE=) | `92.64% <100.00%> (-0.91%)` | :arrow_down: |
| [...e/hudi/variant/Spark4VariantShreddingProvider.java](https://app.codecov.io/gh/apache/hudi/pull/20332?src=pr&el=tree&filepath=hudi-spark-datasource%2Fhudi-spark4-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fvariant%2FSpark4VariantShreddingProvider.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3Bhcms0LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS92YXJpYW50L1NwYXJrNFZhcmlhbnRTaHJlZGRpbmdQcm92aWRlci5qYXZh) | `60.82% <6.25%> (-18.75%)` | :arrow_down: |
| [...io/storage/hadoop/HoodieVariantReconstruction.java](https://app.codecov.io/gh/apache/hudi/pull/20332?src=pr&el=tree&filepath=hudi-hadoop-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fio%2Fstorage%2Fhadoop%2FHoodieVariantReconstruction.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1oYWRvb3AtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2lvL3N0b3JhZ2UvaGFkb29wL0hvb2RpZVZhcmlhbnRSZWNvbnN0cnVjdGlvbi5qYXZh) | `63.70% <41.53%> (-28.85%)` | :arrow_down: |
... and [374 files with indirect coverage changes](https://app.codecov.io/gh/apache/hudi/pull/20332/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): read shredded variant leaves like Spark 4 [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous commented on code in PR #20332:
URL: https://github.com/apache/hudi/pull/20332#discussion_r4336019514
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/io/storage/hadoop/HoodieVariantReconstruction.java:
##########
@@ -110,90 +124,153 @@ static HoodieVariantReconstruction create(HoodieSchema fileSchema, HoodieSchema
+ HoodieStorageConfig.PARQUET_VARIANT_SHREDDING_PROVIDER_CLASS.key()
+ " or add a provider implementation (e.g. the Spark variant module) to the classpath.");
}
+ for (VariantColumnRebuilder variantColumn : planner.variantColumns) {
+ variantColumn.prepare(provider);
+ }
return new HoodieVariantReconstruction(
- VariantSchemaUtils.toShreddedReadSchema(requestedSchema, fileSchema), provider, rootRebuilder);
+ VariantSchemaUtils.toShreddedReadSchema(requestedSchema, fileSchema), rootRebuilder);
}
/**
* Rebuilds shredded variant columns of {@code in} (read in the intermediate shredded shape) into
* a record conforming to the unshredded output schema.
*/
IndexedRecord reconstruct(IndexedRecord in) {
- return (IndexedRecord) rootRebuilder.rebuild(in, provider);
+ return (IndexedRecord) rootRebuilder.rebuild(in);
}
- /**
- * Plans the rebuild for one schema position, or returns {@code null} when nothing below it is a
- * shredded variant and the value can be passed through untouched. The walk is driven by the
- * output (requested) side because that is the shape the record read from parquet has; the file
- * side is matched into it by field name. Detection is anchored by the requested side because the
- * file schema usually comes from converting the parquet footer MessageType, which loses the
- * variant logical type; see VariantSchemaUtils.isShreddedVariantTarget (#19567). Records, array
- * elements and map values are all descended into, matching what the writers can emit; see
- * {@code VariantSchemaUtils.swapShreddedVariantFields} for what actually produces a nested
- * shredded file today (both the row and the AVRO write path shred variant record members at any
- * depth off the forced-shredding property; a variant that is directly an array element or a map
- * value shreds on neither path unless the write schema itself declares typed_value there).
- */
- private static Rebuilder buildRebuilder(HoodieSchema outputSchema, HoodieSchema fileSchema) {
- if (VariantSchemaUtils.isShreddedVariantTarget(fileSchema, outputSchema)) {
- return new VariantRebuilder(fileSchema.getNonNullType().getAvroSchema(),
- outputSchema.getNonNullType().getAvroSchema());
- }
- HoodieSchema output = outputSchema.getNonNullType();
- HoodieSchema file = fileSchema.getNonNullType();
- if (output.getType() != file.getType()) {
- return null;
+ /** Plans the rebuild of a file, collecting its shredded variant columns for preparation. */
+ private static final class Planner {
+ // Mirrors the file schema conversion's ADD_LIST_ELEMENT_RECORDS, which decides the list element.
Review Comment:
The two rules run on different groups. The planner's `listElementType` only walks the lists around a variant column (e.g. `array<variant>`) and mirrors parquet-avro (`AvroSchemaConverter.isElementType` plus **ADD_LIST_ELEMENT_RECORDS**), because it has to match the Avro shape the reader hands back. The provider's only applies to the typed_value list inside a variant and mirrors `ParquetToSparkSchemaConverter.isElementType`, since it validates the layout as Spark would read it.
Sharing one rule would make one of them wrong. The only overlap is parquet-avro shaping typed_value before the provider decodes it, and they diverge there only when the element is itself a repeated group.
Added a javadoc cross-reference on both in 6aa750d so a change to one points at the other.
--
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): read shredded variant leaves like Spark 4 [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous commented on PR #20332:
URL: https://github.com/apache/hudi/pull/20332#issuecomment-6445383376
Rebased
--
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): read shredded variant leaves like Spark 4 [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous commented on PR #20332:
URL: https://github.com/apache/hudi/pull/20332#issuecomment-6191846650
@wombatu-kun Can you please help to review this too? Thank you.
--
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): read shredded variant leaves like Spark 4 [hudi]
Posted by "shangxinli (via GitHub)" <gi...@apache.org>.
shangxinli commented on code in PR #20332:
URL: https://github.com/apache/hudi/pull/20332#discussion_r4304761388
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/io/storage/hadoop/HoodieVariantReconstruction.java:
##########
@@ -110,90 +124,153 @@ static HoodieVariantReconstruction create(HoodieSchema fileSchema, HoodieSchema
+ HoodieStorageConfig.PARQUET_VARIANT_SHREDDING_PROVIDER_CLASS.key()
+ " or add a provider implementation (e.g. the Spark variant module) to the classpath.");
}
+ for (VariantColumnRebuilder variantColumn : planner.variantColumns) {
+ variantColumn.prepare(provider);
+ }
return new HoodieVariantReconstruction(
- VariantSchemaUtils.toShreddedReadSchema(requestedSchema, fileSchema), provider, rootRebuilder);
+ VariantSchemaUtils.toShreddedReadSchema(requestedSchema, fileSchema), rootRebuilder);
}
/**
* Rebuilds shredded variant columns of {@code in} (read in the intermediate shredded shape) into
* a record conforming to the unshredded output schema.
*/
IndexedRecord reconstruct(IndexedRecord in) {
- return (IndexedRecord) rootRebuilder.rebuild(in, provider);
+ return (IndexedRecord) rootRebuilder.rebuild(in);
}
- /**
- * Plans the rebuild for one schema position, or returns {@code null} when nothing below it is a
- * shredded variant and the value can be passed through untouched. The walk is driven by the
- * output (requested) side because that is the shape the record read from parquet has; the file
- * side is matched into it by field name. Detection is anchored by the requested side because the
- * file schema usually comes from converting the parquet footer MessageType, which loses the
- * variant logical type; see VariantSchemaUtils.isShreddedVariantTarget (#19567). Records, array
- * elements and map values are all descended into, matching what the writers can emit; see
- * {@code VariantSchemaUtils.swapShreddedVariantFields} for what actually produces a nested
- * shredded file today (both the row and the AVRO write path shred variant record members at any
- * depth off the forced-shredding property; a variant that is directly an array element or a map
- * value shreds on neither path unless the write schema itself declares typed_value there).
- */
- private static Rebuilder buildRebuilder(HoodieSchema outputSchema, HoodieSchema fileSchema) {
- if (VariantSchemaUtils.isShreddedVariantTarget(fileSchema, outputSchema)) {
- return new VariantRebuilder(fileSchema.getNonNullType().getAvroSchema(),
- outputSchema.getNonNullType().getAvroSchema());
- }
- HoodieSchema output = outputSchema.getNonNullType();
- HoodieSchema file = fileSchema.getNonNullType();
- if (output.getType() != file.getType()) {
- return null;
+ /** Plans the rebuild of a file, collecting its shredded variant columns for preparation. */
+ private static final class Planner {
+ // Mirrors the file schema conversion's ADD_LIST_ELEMENT_RECORDS, which decides the list element.
Review Comment:
nit: this and Spark4VariantShreddingProvider now carry separate list-element rules for the same parquet group; they only disagree when the element is itself a repeated group, and then the provider throws invalidShreddingLayout at open, but it would be easy to change one and not the other.
--
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