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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
