You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "vinishjail97 (via GitHub)" <gi...@apache.org> on 2026/06/30 20:40:31 UTC
[PR] fix: support reading tables partitioned on a nested column [hudi]
vinishjail97 opened a new pull request, #19123:
URL: https://github.com/apache/hudi/pull/19123
### Describe the issue this Pull Request addresses
Closes #19122
Reading a Hudi table partitioned on a nested column (a partition field whose name is a dotted path, e.g. `nested_record.level`) fails on the only batch read path with `HoodieSchemaException: Illegal character in: nested_record.level`, so the table cannot be read at all.
### Summary and Changelog
When a partition field is mandatory, `HoodieFileGroupReaderBasedFileFormat#buildReaderWithPartitionValues` adds it to the `StructType` it converts into a `HoodieSchema` (a top-level Avro field) so it can be read from the file. For a nested partition column the field name is a dotted path, which is not a valid Avro name, so `convertStructTypeToHoodieSchema` throws before the read starts.
A nested partition column is never a flat top-level column in the data file: its value is materialized from the partition path, and its root field (`nested_record`) is already read via the data schema. This PR treats nested partition fields (names containing `.`) as appended partition fields rather than fields read from the file, so they are not converted into a top-level Avro field:
- Add an `isNestedPartitionField(name)` helper (`name.contains(".")`).
- Keep nested partition fields in the appended ("remaining") partition fields when splitting `remainingPartitionSchema` / `fixedPartitionIndexes`, so their values are appended from the partition path.
- Exclude nested partition fields from the two `StructType -> HoodieSchema` conversions (`requestedStructType`, `dataStructTypeWithMandatoryPartitionFields`) so the dotted name is never converted to an Avro field.
- Apply the same exclusion in `vectorTypes`.
This mirrors the existing root-level handling for nested mandatory columns in `HoodieBaseRelation#appendMandatoryColumns` (`HoodieAvroUtils.getRootLevelFieldName`).
### Impact
Tables partitioned on a nested column become readable. No change for tables partitioned on top-level columns (the new predicate only matches dotted names).
### Risk Level
low. The behavioral change is scoped to partition fields whose name contains `.`, which previously could not be read at all. Validated end-to-end via Apache XTable's `ITConversionController#testPartitionedData` (HUDI partitioned on `nested_record.level` -> ICEBERG) against a locally built snapshot: the conversion now succeeds and the `nested_record.level = 'INFO'` filter round-trips, confirming the partition value is materialized correctly (not just that the crash is avoided).
### Documentation Update
none
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [ ] 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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19123:
URL: https://github.com/apache/hudi/pull/19123#issuecomment-4848389563
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052",
"triggerID" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"triggerType" : "PUSH"
}, {
"hash" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15054",
"triggerID" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 49ab365258fe84a3fba5847b3133fe7989baa08c Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052)
* 30b187407d0ce5b62c77ad2b6d274ec17a374eae Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15054)
<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: support reading tables partitioned on a nested column [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous merged PR #19123:
URL: https://github.com/apache/hudi/pull/19123
--
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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19123:
URL: https://github.com/apache/hudi/pull/19123#discussion_r3501783792
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -253,7 +255,11 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath: String,
val augmentedStorageConf = new HadoopStorageConfiguration(hadoopConf).getInline
setSchemaEvolutionConfigs(augmentedStorageConf)
augmentedStorageConf.set(ENABLE_LOGICAL_TIMESTAMP_REPAIR, hasTimestampMillisFieldInTableSchema.toString)
- val (remainingPartitionSchemaArr, fixedPartitionIndexesArr) = partitionSchema.fields.toSeq.zipWithIndex.filter(p => !mandatoryFields.contains(p._1.name)).unzip
+ // Nested partition columns (e.g. "nested_record.level") are never read from the data file: the
+ // flattened dotted name is not a valid top-level field and the value is materialized from the
+ // partition path. Always keep them in the appended ("remaining") partition fields so they are
+ // not converted into a top-level Avro field below, which would fail Avro name validation.
+ val (remainingPartitionSchemaArr, fixedPartitionIndexesArr) = partitionSchema.fields.toSeq.zipWithIndex.filter(p => !mandatoryFields.contains(p._1.name) || isNestedPartitionField(p._1.name)).unzip
Review Comment:
🤖 Now that a nested partition field can be appended (remaining) while a top-level partition field is still read from the file, the snapshot/MOR path hits `appendPartitionAndProject`, which calls `getFixedPartitionValues(partitionValues, remainingPartitionSchema, fixedPartitionIndexes)` (line 426). But `fixedPartitionIndexes` are positions in the *full* `partitionSchema`, and `getFixedPartitionValues` does `allPartitionValues.toSeq(partitionSchema)` — the base-file path correctly passes the full schema (line 529), whereas here it gets `remainingPartitionSchema`. Could a nested partition field mixed with a top-level field that's read from the file (e.g. timestamp keygen over multiple partition cols) drop/misread the nested value here? @nsivabalan could you verify this path?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -265,9 +271,9 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath: String,
val exclusionFields = new java.util.HashSet[String]()
exclusionFields.add("op")
partitionSchema.fields.foreach(f => exclusionFields.add(f.name))
- val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name)))
+ val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name) && !isNestedPartitionField(f.name)))
Review Comment:
🤖 A partition field lands in `mandatoryFields` precisely because its partition-path encoding isn't the true column value (precombine, or variable timestamp/custom keygen), so it's read from the file. By excluding nested mandatory fields here and appending them from the partition path instead, do we risk the output value being the path-encoded form (or null on a type mismatch) for those keygen cases? The merge precombine value can still come from the nested struct read via the data schema, but the appended flat partition column value would now be path-derived — is that intended?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -210,7 +210,9 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath: String,
}
originalVectorTypes.map {
o: Seq[String] => o.zipWithIndex.map(a => {
- if (a._2 >= requiredSchema.length && mandatoryFields.contains(partitionSchema.fields(a._2 - requiredSchema.length).name)) {
+ if (a._2 >= requiredSchema.length
Review Comment:
🤖 nit: `partitionSchema.fields(a._2 - requiredSchema.length).name` is evaluated twice in this condition — could you pull it into a `val fieldName = ...` before the `if` to make the predicate easier to scan?
<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: support reading tables partitioned on a nested column [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous commented on PR #19123:
URL: https://github.com/apache/hudi/pull/19123#issuecomment-5376949513
Azure failure is already fixed on master by #19681/#19682. This branch has not been rebased to pickup the changes that's why it is failing. Will squash and merge.
--
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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19123:
URL: https://github.com/apache/hudi/pull/19123#issuecomment-4849359101
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052",
"triggerID" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"triggerType" : "PUSH"
}, {
"hash" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15054",
"triggerID" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"triggerType" : "PUSH"
}, {
"hash" : "8e064fa892d832c41cdc1389f7a8e587d92ad13b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15057",
"triggerID" : "8e064fa892d832c41cdc1389f7a8e587d92ad13b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8e064fa892d832c41cdc1389f7a8e587d92ad13b Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15057)
<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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19123:
URL: https://github.com/apache/hudi/pull/19123#issuecomment-4848415121
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052",
"triggerID" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"triggerType" : "PUSH"
}, {
"hash" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15054",
"triggerID" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 49ab365258fe84a3fba5847b3133fe7989baa08c Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052)
* 30b187407d0ce5b62c77ad2b6d274ec17a374eae Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15054)
<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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19123:
URL: https://github.com/apache/hudi/pull/19123#discussion_r3502669372
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
Review Comment:
🤖 **Line 429:** Following up on the earlier thread — I don't think this call site passes the full `partitionSchema`. At line 341 `appendPartitionAndProject` is invoked with `remainingPartitionSchema` as its `partitionSchema` arg, so here `partitionSchema` is the *remaining* schema, whereas `readBaseFile` (line 532) passes the full one. With a mix of one top-level mandatory field (read from file) + one nested field (appended), e.g. partition `country,nested_record.level` both ordering fields: remaining=[nested_record.level] (len 1), indices={1}, partitionValues.numFields=2 → line 424 is false → `getFixedPartitionValues(partitionValues, remaining, {1})` does `toSeq(remaining)` which materializes only ordinal 0 (country's value, as StringType) then filters to {1} → empty, so the nested value is dropped. The new tests are COW-only so they hit `readBaseFile` (correct) and never this MOR branch. Could you confirm whether this should pass the full `partitionSchema` to `getFixe
dPartitionValues` here (while keeping `remainingPartitionSchema` for the join projection)?
<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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19123:
URL: https://github.com/apache/hudi/pull/19123#issuecomment-4848883578
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052",
"triggerID" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"triggerType" : "PUSH"
}, {
"hash" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15054",
"triggerID" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"triggerType" : "PUSH"
}, {
"hash" : "8e064fa892d832c41cdc1389f7a8e587d92ad13b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15057",
"triggerID" : "8e064fa892d832c41cdc1389f7a8e587d92ad13b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 30b187407d0ce5b62c77ad2b6d274ec17a374eae Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15054)
* 8e064fa892d832c41cdc1389f7a8e587d92ad13b Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15057)
<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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19123:
URL: https://github.com/apache/hudi/pull/19123#issuecomment-4847781437
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 49ab365258fe84a3fba5847b3133fe7989baa08c 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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19123:
URL: https://github.com/apache/hudi/pull/19123#issuecomment-4848354143
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052",
"triggerID" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"triggerType" : "PUSH"
}, {
"hash" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 49ab365258fe84a3fba5847b3133fe7989baa08c Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052)
* 30b187407d0ce5b62c77ad2b6d274ec17a374eae 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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19123:
URL: https://github.com/apache/hudi/pull/19123#issuecomment-4848853791
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052",
"triggerID" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"triggerType" : "PUSH"
}, {
"hash" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15054",
"triggerID" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"triggerType" : "PUSH"
}, {
"hash" : "8e064fa892d832c41cdc1389f7a8e587d92ad13b",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "8e064fa892d832c41cdc1389f7a8e587d92ad13b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 49ab365258fe84a3fba5847b3133fe7989baa08c Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052)
* 30b187407d0ce5b62c77ad2b6d274ec17a374eae Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15054)
* 8e064fa892d832c41cdc1389f7a8e587d92ad13b 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: support reading tables partitioned on a nested column [hudi]
Posted by "vinishjail97 (via GitHub)" <gi...@apache.org>.
vinishjail97 commented on code in PR #19123:
URL: https://github.com/apache/hudi/pull/19123#discussion_r3502118771
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -253,7 +255,11 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath: String,
val augmentedStorageConf = new HadoopStorageConfiguration(hadoopConf).getInline
setSchemaEvolutionConfigs(augmentedStorageConf)
augmentedStorageConf.set(ENABLE_LOGICAL_TIMESTAMP_REPAIR, hasTimestampMillisFieldInTableSchema.toString)
- val (remainingPartitionSchemaArr, fixedPartitionIndexesArr) = partitionSchema.fields.toSeq.zipWithIndex.filter(p => !mandatoryFields.contains(p._1.name)).unzip
+ // Nested partition columns (e.g. "nested_record.level") are never read from the data file: the
+ // flattened dotted name is not a valid top-level field and the value is materialized from the
+ // partition path. Always keep them in the appended ("remaining") partition fields so they are
+ // not converted into a top-level Avro field below, which would fail Avro name validation.
+ val (remainingPartitionSchemaArr, fixedPartitionIndexesArr) = partitionSchema.fields.toSeq.zipWithIndex.filter(p => !mandatoryFields.contains(p._1.name) || isNestedPartitionField(p._1.name)).unzip
Review Comment:
Both `getFixedPartitionValues` call sites pass the **full** `partitionSchema`, not `remainingPartitionSchema`: `appendPartitionAndProject` (line ~426) and `readBaseFile` (line ~529) both call `getFixedPartitionValues(<values>, partitionSchema, fixedPartitionIndexes)`. `fixedPartitionIndexes` are positions in that same full `partitionSchema`, so `allPartitionValues.toSeq(partitionSchema)` and the index set line up — the nested value is selected by its true position.
To cover exactly the mixed scenario you describe (a top-level partition column read from the file + a nested partition column appended from the path), I added `testReadTablePartitionedOnNestedAndTopLevelColumns` in 30b187407d (partition `country,nested_record.level`, both ordering fields). It hits the `readBaseFile` branch where `remainingPartitionSchema` (the nested field) differs from the full `partitionSchema`, and asserts `country` (from file) and `nested_record.level` (from path) are both correct. Verified on Spark 3.5: fails with `Illegal character in: nested_record.level` without the fix, passes with it. @nsivabalan PTAL.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -265,9 +271,9 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath: String,
val exclusionFields = new java.util.HashSet[String]()
exclusionFields.add("op")
partitionSchema.fields.foreach(f => exclusionFields.add(f.name))
- val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name)))
+ val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name) && !isNestedPartitionField(f.name)))
Review Comment:
Good question, but there is no regression here for a nested partition field specifically. A nested partition column (`nested_record.level`) is never a flat physical column in the data file — the file persists the `nested_record` struct, not a top-level `nested_record.level` column — so its value can only ever be materialized from the partition path. Before this change the read did not produce a path-derived-vs-true-value discrepancy; it failed outright with `Illegal character in: nested_record.level` in `convertStructTypeToHoodieSchema`, i.e. the table was unreadable. Appending from the path is therefore the only available (and correct) source, and it is consistent with how `_partitionSchemaFromProperties` already treats `TimestampBasedKeyGenerator` partitions (StringType materialized from the path).
The precombine/merge value is unaffected: it is read from the `nested_record` struct via the data schema, not from the synthetic flat partition column. The fix only changes the appended *output* partition column (which equals the path value), not what merging sees.
Note the guard is scoped to nested (dotted) names — top-level mandatory partition columns (incl. variable timestamp/custom-keygen columns) are still read from the file exactly as before. Covered by the two regression tests added in 30b187407d.
--
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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19123:
URL: https://github.com/apache/hudi/pull/19123#issuecomment-5320261435
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052",
"triggerID" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"triggerType" : "PUSH"
}, {
"hash" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15054",
"triggerID" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"triggerType" : "PUSH"
}, {
"hash" : "8e064fa892d832c41cdc1389f7a8e587d92ad13b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15057",
"triggerID" : "8e064fa892d832c41cdc1389f7a8e587d92ad13b",
"triggerType" : "PUSH"
}, {
"hash" : "03342c63e28faa4adcdf91d179b3a3d056a491a3",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16348",
"triggerID" : "03342c63e28faa4adcdf91d179b3a3d056a491a3",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8e064fa892d832c41cdc1389f7a8e587d92ad13b Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15057)
* 03342c63e28faa4adcdf91d179b3a3d056a491a3 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16348)
<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: support reading tables partitioned on a nested column [hudi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19123:
URL: https://github.com/apache/hudi/pull/19123#issuecomment-5320314895
## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/19123?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 73.60%. Comparing base ([`989badc`](https://app.codecov.io/gh/apache/hudi/commit/989badc3952e05052754dc1f028800c660646aee?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`03342c6`](https://app.codecov.io/gh/apache/hudi/commit/03342c63e28faa4adcdf91d179b3a3d056a491a3?dropdown=coverage&el=desc&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 #19123 +/- ##
============================================
- Coverage 73.60% 73.60% -0.01%
+ Complexity 31322 31315 -7
============================================
Files 2525 2525
Lines 139644 139640 -4
Branches 16809 16809
============================================
- Hits 102791 102784 -7
- Misses 29701 29703 +2
- Partials 7152 7153 +1
```
| [Components](https://app.codecov.io/gh/apache/hudi/pull/19123/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/19123/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `82.65% <ø> (-0.01%)` | :arrow_down: |
| [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/19123/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.73% <ø> (-0.01%)` | :arrow_down: |
| [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/19123/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.75% <ø> (ø)` | |
| [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/19123/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `47.33% <ø> (-0.01%)` | :arrow_down: |
| [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/19123/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.80% <ø> (ø)` | |
| [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/19123/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `15.26% <ø> (ø)` | |
| [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/19123/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.85% <ø> (ø)` | |
| [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/19123/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `75.53% <ø> (ø)` | |
| [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/19123/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.32% <ø> (ø)` | |
| [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/19123/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.44% <ø> (ø)` | |
| [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/19123/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `64.33% <ø> (ø)` | |
| [hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/19123/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/19123/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/19123/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.90% <ø> (+0.04%)` | :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/19123?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...parquet/HoodieFileGroupReaderBasedFileFormat.scala](https://app.codecov.io/gh/apache/hudi/pull/19123?src=pr&el=tree&filepath=hudi-spark-datasource%2Fhudi-spark-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fspark%2Fsql%2Fexecution%2Fdatasources%2Fparquet%2FHoodieFileGroupReaderBasedFileFormat.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3BhcmstY29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9kYXRhc291cmNlcy9wYXJxdWV0L0hvb2RpZUZpbGVHcm91cFJlYWRlckJhc2VkRmlsZUZvcm1hdC5zY2FsYQ==) | `81.78% <ø> (+0.10%)` | :arrow_up: |
... and [2 files with indirect coverage changes](https://app.codecov.io/gh/apache/hudi/pull/19123/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: support reading tables partitioned on a nested column [hudi]
Posted by "vinishjail97 (via GitHub)" <gi...@apache.org>.
vinishjail97 commented on code in PR #19123:
URL: https://github.com/apache/hudi/pull/19123#discussion_r3502118135
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -210,7 +210,9 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath: String,
}
originalVectorTypes.map {
o: Seq[String] => o.zipWithIndex.map(a => {
- if (a._2 >= requiredSchema.length && mandatoryFields.contains(partitionSchema.fields(a._2 - requiredSchema.length).name)) {
+ if (a._2 >= requiredSchema.length
Review Comment:
Done in 30b187407d — pulled the repeated `partitionSchema.fields(a._2 - requiredSchema.length).name` into a local `val fieldName` before the predicate.
--
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: support reading tables partitioned on a nested column [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #19123:
URL: https://github.com/apache/hudi/pull/19123#discussion_r3825868512
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestFileGroupReaderPartitionColumn.scala:
##########
@@ -155,4 +155,132 @@ class TestFileGroupReaderPartitionColumn extends SparkClientFunctionalTestHarnes
assertEquals("IN", rows(12L), "id=12 partition column must be IN")
assertEquals("IN", rows(14L), "id=14 partition column must be IN")
}
+
+ /**
+ * Regression test for reading a table partitioned on a nested column when that nested column is
+ * also a mandatory field (here, the precombine/ordering field). Being mandatory, the file group
+ * reader requests the partition column as a top-level field; for a nested path
+ * ("nested_record.level") this previously failed in `buildReaderWithPartitionValues` with
+ * `HoodieSchemaException: Illegal character in: nested_record.level` when converting the
+ * StructType into an Avro-backed HoodieSchema.
+ *
+ * The nested partition value is never a flat top-level column in the data file — it must be
+ * materialized from the partition path — so the fix keeps it out of the file-read schema and
+ * appends it from the path. The existing `TestCOWDataSource#testNestedFieldPartition` covers the
+ * common (non-mandatory) path where the field is already appended from the path and does not hit
+ * this case.
+ */
+ @Test
+ def testReadTablePartitionedOnNestedColumnThatIsAlsoPrecombine(): Unit = {
+ val nestedSchema = StructType(Array(
+ StructField("nested_int", IntegerType, nullable = false),
+ StructField("level", StringType, nullable = false)
+ ))
+ val schema = StructType(Array(
+ StructField("id", LongType, nullable = false),
+ StructField("name", StringType, nullable = true),
+ StructField("nested_record", nestedSchema, nullable = true)
+ ))
+
+ val opts = Map(
+ "hoodie.table.name" -> "test_nested_partition_precombine",
+ "hoodie.datasource.write.table.type" -> "COPY_ON_WRITE",
+ "hoodie.datasource.write.recordkey.field" -> "id",
+ // partition column is ALSO the precombine field -> it becomes mandatory (read from file),
+ // which is what drives the nested-name conversion the fix guards against.
+ "hoodie.datasource.write.partitionpath.field" -> "nested_record.level",
+ "hoodie.datasource.write.precombine.field" -> "nested_record.level",
Review Comment:
nit: could `hoodie.datasource.write.precombine.field` use a different nested field instead of the same partition path field?
--
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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19123:
URL: https://github.com/apache/hudi/pull/19123#issuecomment-5320781746
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052",
"triggerID" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"triggerType" : "PUSH"
}, {
"hash" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15054",
"triggerID" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"triggerType" : "PUSH"
}, {
"hash" : "8e064fa892d832c41cdc1389f7a8e587d92ad13b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15057",
"triggerID" : "8e064fa892d832c41cdc1389f7a8e587d92ad13b",
"triggerType" : "PUSH"
}, {
"hash" : "03342c63e28faa4adcdf91d179b3a3d056a491a3",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16348",
"triggerID" : "03342c63e28faa4adcdf91d179b3a3d056a491a3",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 03342c63e28faa4adcdf91d179b3a3d056a491a3 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16348)
<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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19123:
URL: https://github.com/apache/hudi/pull/19123#issuecomment-4847815712
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052",
"triggerID" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 49ab365258fe84a3fba5847b3133fe7989baa08c Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052)
<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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19123:
URL: https://github.com/apache/hudi/pull/19123#discussion_r3798820868
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -265,9 +274,9 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath: String,
val exclusionFields = new java.util.HashSet[String]()
exclusionFields.add("op")
partitionSchema.fields.foreach(f => exclusionFields.add(f.name))
- val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name)))
+ val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name) && !isNestedPartitionField(f.name)))
val requestedSchema = HoodieSchemaUtils.pruneDataSchema(schema, HoodieSchemaConversionUtils.convertStructTypeToHoodieSchema(requestedStructType, sanitizedTableName), exclusionFields)
- val dataStructTypeWithMandatoryPartitionFields = StructType(dataStructType.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name)))
+ val dataStructTypeWithMandatoryPartitionFields = StructType(dataStructType.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name) && !isNestedPartitionField(f.name)))
Review Comment:
🤖 Agree this is very likely safe — a partition column is constant across every record in a file group (they all share the same partition path), so it can never change the outcome of an ordering-field tiebreak, and excluding it from the reader's schema is fine. Worth noting the same reasoning holds on the log-merge path: log records in that file group carry the identical partition value, so the precombine comparison degenerates to the fallback (newer wins) exactly as it would with the field present. The MOR test suggested above would be good confirmation to lock this down.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -267,9 +276,9 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath: String,
val exclusionFields = new java.util.HashSet[String]()
exclusionFields.add("op")
partitionSchema.fields.foreach(f => exclusionFields.add(f.name))
- val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name)))
+ val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name) && !isNestedPartitionField(f.name)))
Review Comment:
🤖 nit: the compound predicate `mandatoryFields.contains(f.name) && !isNestedPartitionField(f.name)` (and its negated form) appears four times across this change — could you extract it into a small helper like `isMandatoryFlatPartitionField(name: String)` to make each call-site read more naturally and keep the definition in one place?
<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: support reading tables partitioned on a nested column [hudi]
Posted by "vinishjail97 (via GitHub)" <gi...@apache.org>.
vinishjail97 commented on code in PR #19123:
URL: https://github.com/apache/hudi/pull/19123#discussion_r3502481199
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -265,9 +274,9 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath: String,
val exclusionFields = new java.util.HashSet[String]()
exclusionFields.add("op")
partitionSchema.fields.foreach(f => exclusionFields.add(f.name))
- val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name)))
+ val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name) && !isNestedPartitionField(f.name)))
val requestedSchema = HoodieSchemaUtils.pruneDataSchema(schema, HoodieSchemaConversionUtils.convertStructTypeToHoodieSchema(requestedStructType, sanitizedTableName), exclusionFields)
- val dataStructTypeWithMandatoryPartitionFields = StructType(dataStructType.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name)))
+ val dataStructTypeWithMandatoryPartitionFields = StructType(dataStructType.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name) && !isNestedPartitionField(f.name)))
Review Comment:
When the nested partition column is also the precombine/ordering field, it's now excluded from the `dataSchema`/`requestedSchema` passed to `HoodieFileGroupReader`. For MOR merges the ordering field is what resolves which record wins — is it safe for the reader to never receive it in this case? Since a partition column is constant across all records in a file group the tiebreak is effectively a no-op, so I suspect this is fine, but it'd be good to confirm and cover it with the MOR test suggested above.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestFileGroupReaderPartitionColumn.scala:
##########
@@ -155,4 +155,132 @@ class TestFileGroupReaderPartitionColumn extends SparkClientFunctionalTestHarnes
assertEquals("IN", rows(12L), "id=12 partition column must be IN")
assertEquals("IN", rows(14L), "id=14 partition column must be IN")
}
+
+ /**
+ * Regression test for reading a table partitioned on a nested column when that nested column is
+ * also a mandatory field (here, the precombine/ordering field). Being mandatory, the file group
+ * reader requests the partition column as a top-level field; for a nested path
+ * ("nested_record.level") this previously failed in `buildReaderWithPartitionValues` with
+ * `HoodieSchemaException: Illegal character in: nested_record.level` when converting the
+ * StructType into an Avro-backed HoodieSchema.
+ *
+ * The nested partition value is never a flat top-level column in the data file — it must be
+ * materialized from the partition path — so the fix keeps it out of the file-read schema and
+ * appends it from the path. The existing `TestCOWDataSource#testNestedFieldPartition` covers the
+ * common (non-mandatory) path where the field is already appended from the path and does not hit
+ * this case.
+ */
+ @Test
Review Comment:
Both new tests are COPY_ON_WRITE and only exercise the `readBaseFile` code path. The snapshot-with-log-files branch (`appendPartitionAndProject`, ~L341 in the file format) also consumes `remainingPartitionSchema`/`fixedPartitionIndexes` for a nested mandatory field and isn't covered here. Could you add a MERGE_ON_READ variant that writes an initial batch then an update batch (so log files exist) on a table partitioned on a nested mandatory column, and read it back? That would lock down the log-merge path this fix also touches. Non-blocking but valuable.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -265,9 +274,9 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath: String,
val exclusionFields = new java.util.HashSet[String]()
exclusionFields.add("op")
partitionSchema.fields.foreach(f => exclusionFields.add(f.name))
- val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name)))
+ val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name) && !isNestedPartitionField(f.name)))
Review Comment:
The "mandatory AND not-nested" (i.e. read-from-file) predicate is now duplicated here, at L279, and in `vectorTypes` (L217), and appears De Morgan-negated in the `remainingPartitionSchema` filter (L265). Consider extracting a single helper, e.g.:
```scala
private def partitionFieldReadFromFile(field: StructField): Boolean =
mandatoryFields.contains(field.name) && !isNestedPartitionField(field.name)
```
and reusing it in all four spots. That keeps the negated form at L265 from silently drifting out of sync with the positive forms if the condition ever changes. Non-blocking.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -548,6 +557,14 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath: String,
}.asInstanceOf[Iterator[InternalRow]]
}
+ /**
+ * A partition column whose name is a nested field path (e.g. "nested_record.level") cannot be
+ * read from the data file as a flat top-level column, nor converted into a top-level Avro field
+ * (Avro rejects '.' in names). Its value is always materialized from the partition path, so such
+ * fields are treated as appended partition fields rather than read from the file.
+ */
+ private def isNestedPartitionField(name: String): Boolean = name.contains(".")
Review Comment:
nit: `name.contains(".")` is a sound heuristic here — top-level Avro/Spark field names can't contain `.`, so there are no false positives, and the javadoc explains it well. Minor: the key generators already split `partitionpath.field` on `.` to express nesting; if there's a shared constant/util for that separator, reusing it would be preferable to a bare literal.
--
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: support reading tables partitioned on a nested column [hudi]
Posted by "vinishjail97 (via GitHub)" <gi...@apache.org>.
vinishjail97 commented on code in PR #19123:
URL: https://github.com/apache/hudi/pull/19123#discussion_r3502704769
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -265,9 +271,9 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath: String,
val exclusionFields = new java.util.HashSet[String]()
exclusionFields.add("op")
partitionSchema.fields.foreach(f => exclusionFields.add(f.name))
- val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name)))
+ val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name) && !isNestedPartitionField(f.name)))
Review Comment:
@yihua Have a question to clarify here - required nested partitioning fields will be read via the data file or will be parsed from the partition path? That's the bug this PR is trying to fix.
--
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: support reading tables partitioned on a nested column [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19123:
URL: https://github.com/apache/hudi/pull/19123#issuecomment-5320205807
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15052",
"triggerID" : "49ab365258fe84a3fba5847b3133fe7989baa08c",
"triggerType" : "PUSH"
}, {
"hash" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15054",
"triggerID" : "30b187407d0ce5b62c77ad2b6d274ec17a374eae",
"triggerType" : "PUSH"
}, {
"hash" : "8e064fa892d832c41cdc1389f7a8e587d92ad13b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15057",
"triggerID" : "8e064fa892d832c41cdc1389f7a8e587d92ad13b",
"triggerType" : "PUSH"
}, {
"hash" : "03342c63e28faa4adcdf91d179b3a3d056a491a3",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "03342c63e28faa4adcdf91d179b3a3d056a491a3",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8e064fa892d832c41cdc1389f7a8e587d92ad13b Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15057)
* 03342c63e28faa4adcdf91d179b3a3d056a491a3 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: support reading tables partitioned on a nested column [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #19123:
URL: https://github.com/apache/hudi/pull/19123#discussion_r3825893134
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestFileGroupReaderPartitionColumn.scala:
##########
@@ -155,4 +155,132 @@ class TestFileGroupReaderPartitionColumn extends SparkClientFunctionalTestHarnes
assertEquals("IN", rows(12L), "id=12 partition column must be IN")
assertEquals("IN", rows(14L), "id=14 partition column must be IN")
}
+
+ /**
+ * Regression test for reading a table partitioned on a nested column when that nested column is
+ * also a mandatory field (here, the precombine/ordering field). Being mandatory, the file group
+ * reader requests the partition column as a top-level field; for a nested path
+ * ("nested_record.level") this previously failed in `buildReaderWithPartitionValues` with
+ * `HoodieSchemaException: Illegal character in: nested_record.level` when converting the
+ * StructType into an Avro-backed HoodieSchema.
+ *
+ * The nested partition value is never a flat top-level column in the data file — it must be
+ * materialized from the partition path — so the fix keeps it out of the file-read schema and
+ * appends it from the path. The existing `TestCOWDataSource#testNestedFieldPartition` covers the
+ * common (non-mandatory) path where the field is already appended from the path and does not hit
+ * this case.
+ */
+ @Test
+ def testReadTablePartitionedOnNestedColumnThatIsAlsoPrecombine(): Unit = {
+ val nestedSchema = StructType(Array(
+ StructField("nested_int", IntegerType, nullable = false),
+ StructField("level", StringType, nullable = false)
+ ))
+ val schema = StructType(Array(
+ StructField("id", LongType, nullable = false),
+ StructField("name", StringType, nullable = true),
+ StructField("nested_record", nestedSchema, nullable = true)
+ ))
+
+ val opts = Map(
+ "hoodie.table.name" -> "test_nested_partition_precombine",
+ "hoodie.datasource.write.table.type" -> "COPY_ON_WRITE",
+ "hoodie.datasource.write.recordkey.field" -> "id",
+ // partition column is ALSO the precombine field -> it becomes mandatory (read from file),
+ // which is what drives the nested-name conversion the fix guards against.
+ "hoodie.datasource.write.partitionpath.field" -> "nested_record.level",
+ "hoodie.datasource.write.precombine.field" -> "nested_record.level",
Review Comment:
Does this work as a regression test so that `nested_record.level` becomes a mandatory field that triggers the issue?
--
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: support reading tables partitioned on a nested column [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #19123:
URL: https://github.com/apache/hudi/pull/19123#discussion_r3825830025
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala:
##########
@@ -265,9 +271,9 @@ class HoodieFileGroupReaderBasedFileFormat(tablePath: String,
val exclusionFields = new java.util.HashSet[String]()
exclusionFields.add("op")
partitionSchema.fields.foreach(f => exclusionFields.add(f.name))
- val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name)))
+ val requestedStructType = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name) && !isNestedPartitionField(f.name)))
Review Comment:
It depends on how the partition paths are generated. For simple partition fields, i.e., the partition path is the same as the partition column value, parsing from the path is OK. For a timestamp-based partition path that requires a transformation of the partition column, the partition path and value differ. For partition pruning or attaching partition path meta column, it's better to get them from the path, and we can handle the timestamp key generator separately.
--
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