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