fix: support reading tables partitioned on a nested column by vinishjail97 · Pull Request #19123 · apache/hudi

vinishjail97 · 2026-06-30T20:40:29Z

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
Enough context is provided in the sections above
Adequate tests were added if applicable

A partition field whose name is a nested path (e.g. "nested_record.level") was added to the StructType converted into a top-level Avro field in HoodieFileGroupReaderBasedFileFormat#buildReaderWithPartitionValues, so the read failed with "Illegal character in: nested_record.level" before it could start. 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 is read via the data schema. Treat nested partition fields (dotted names) as appended partition fields rather than fields read from the file, so they are not converted into a top-level Avro field. This mirrors the existing root-level handling in HoodieBaseRelation#appendMandatoryColumns. Closes apache#19122 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ble version 9 Re-enables the three conversion cases apache#772 disabled for the Hudi 1.x target: - Un-partitioned Paimon -> Hudi (ITConversionController and ITConversionService). BaseFileUpdatesExtractor now emits Hudi's external file-group-prefix format (Hudi PR #17788) for bucketed files instead of folding the "bucket-N" directory into the partition path: the file is registered under its true partition (empty for un-partitioned) with fileId "bucket-N/<file>" and the 3-arg marker "<file>_<commit>_fg%3Dbucket-N_hudiext". Applied consistently across the snapshot path, the diff path, and file-id derivation; non-bucketed sources are unaffected. - HUDI (partitioned on the nested column "nested_record.level") -> ICEBERG. This depends on the Hudi reader fix in apache/hudi#19123, so it will only pass once that fix is available in the Hudi snapshot the build resolves. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

hudi-agent

⚠️ 🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for working on this! This PR makes tables partitioned on a nested column readable again by treating nested partition fields (dotted names) as appended-from-partition-path fields rather than fields read from the data file, avoiding the Avro name validation failure. The core approach is sound for the common single-nested-partition case, but a couple of edge cases around the snapshot/MOR path and mandatory partition fields are worth double-checking in the inline comments. Please take a look at any inline comments, and this should be ready for a Hudi committer or PMC member to take it from here. A couple of small readability suggestions around a repeated subexpression in an if-condition and a duplicated filter predicate.

hudi-agent · 2026-06-30T20:54:28Z

+    // 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


🤖 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?

_{⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.}

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 30b1874 (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-agent · 2026-06-30T20:54:28Z

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


🤖 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?

_{⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.}

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 30b1874.

@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.

hudi-agent · 2026-06-30T20:54:29Z

      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


🤖 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?

_{⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.}

Done in 30b1874 — pulled the repeated partitionSchema.fields(a._2 - requiredSchema.length).name into a local val fieldName before the predicate.

…tidy vectorTypes - Add two regression tests in TestFileGroupReaderPartitionColumn covering a table partitioned on a nested column that is also a mandatory (precombine) field, and a mixed nested + top-level partition table. Both reproduce the "Illegal character in: nested_record.level" failure without the fix and pass with it (verified on Spark 3.5). The existing TestCOWDataSource#testNestedFieldPartition covers the common non-mandatory path and does not hit this case. - Extract the duplicated partition field-name lookup in vectorTypes into a local val. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…tests) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

vinishjail97 · 2026-07-01T00:01:19Z

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


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.:

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.

vinishjail97 · 2026-07-01T00:01:19Z

+   * common (non-mandatory) path where the field is already appended from the path and does not hit
+   * this case.
+   */
+  @Test


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.

vinishjail97 · 2026-07-01T00:01:19Z

+    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)))


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.

vinishjail97 · 2026-07-01T00:01:19Z

+   * (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(".")


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.

hudi-agent

⚠️ 🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for the follow-ups here. The readability nit at line 213 is addressed (the repeated partitionSchema.fields(...).name is now pulled into a local fieldName), and the two new regression tests are a nice addition for the COW/readBaseFile path. The question about nested mandatory fields being appended from the path (line 277) was answered reasonably — the flat nested column value isn't consumed for query results since Spark resolves it via the struct, so I'll consider that resolved.

The one prior finding I think is still open is the snapshot/MOR appendPartitionAndProject path. The reply stated both getFixedPartitionValues call sites pass the full partitionSchema, but tracing the binding, the snapshot path passes remainingPartitionSchema into that method, so line 429 filters against the wrong schema when there's a mix of read-from-file and appended partition fields. Both new tests are COW-only and don't exercise this branch (which the author also noted). Left an inline comment with a concrete trace. Please take a look at the inline comment, and this should be ready for a Hudi committer or PMC member to take it from here.

hudi-agent · 2026-07-01T01:07:15Z

🤖 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 getFixedPartitionValues here (while keeping remainingPartitionSchema for the join projection)?

_{⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.}

hudi-bot · 2026-07-01T01:16:33Z

CI report:

8e064fa Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

@hudi-bot run azure re-run the last Azure build

hudi-agent reviewed Jun 30, 2026

View reviewed changes

github-actions Bot added the size:S PR with lines of changes in (10, 100] label Jun 30, 2026

github-actions Bot added size:M PR with lines of changes in (100, 300] and removed size:S PR with lines of changes in (10, 100] labels Jun 30, 2026

ci: retrigger build (flaky runner OOM/loss on scala-2.13 scala-other-…

8e064fa

…tests) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

vinishjail97 commented Jul 1, 2026

View reviewed changes

hudi-agent reviewed Jul 1, 2026

View reviewed changes

Uh oh!

Conversation

vinishjail97 commented Jun 30, 2026

Describe the issue this Pull Request addresses

Summary and Changelog

Impact

Risk Level

Documentation Update

Contributor's checklist

Uh oh!

hudi-agent left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hudi-agent left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hudi-bot commented Jul 1, 2026

CI report:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants