ziting-openai commented on code in PR #5262:
URL: https://github.com/apache/datafusion-comet/pull/5262#discussion_r3935733976
##########
spark/src/main/scala/org/apache/comet/serde/operator/CometIcebergNativeScan.scala:
##########
@@ -1082,6 +1082,19 @@ object CometIcebergNativeScan extends
CometOperatorSerde[CometBatchScanExec] wit
}
}
+ // iceberg-rust validates a FileScanTask by resolving its
partition spec against
+ // the task schema, so a task carrying a partition spec needs
that spec's source
+ // columns present even when the query projects them out (e.g.
selecting only
+ // _spec_id / _partition). Union them in. project_field_ids
still drives the read,
+ // so these columns are not materialized into the output.
+ val schema: AnyRef =
+ IcebergReflection
+ .schemaWithRequiredFields(
+ baseSchema,
+ metadata.table,
+ IcebergReflection.partitionSourceFieldIds(task,
fileScanTaskClass))
Review Comment:
[P2] Preserve valid schemas when adding partition source fields
This new call can construct an invalid schema in two supported cases. First,
for `s:struct<region:string>` partitioned by `identity(s.region)`, the full
table schema already contains the nested source ID, but
`schemaWithRequiredFields` checks only top-level IDs through
`buildFieldIdMapping`. It appends the same field at the top level, so Iceberg's
`Schema` constructor rejects the duplicate ID. Even an ordinary `SELECT id`
without deletes reaches this path.
Second, [current Iceberg 1.11
CI](https://github.com/apache/datafusion-comet/actions/runs/33891332915/job/101092878262)
fails `testReaddColumnAfterIdentityPartitionDrop` at this call with `Invalid
schema: multiple fields for name category: 4 and 2`. The v1 test drops the
category partition field and column, re-adds category under a new ID, inserts a
row, then selects it. The original task spec still references the old void
source ID 2; this union restores its historical field alongside current
category ID 4. The partition serializer already omits the UNKNOWN value and
sends an empty spec for this case, so requiring that discarded source
introduces the collision.
Please derive required sources from the partition representation actually
sent to native, and merge fields with recursive ID checks while preserving
valid names and nesting. These failures propagate from execution-time
`serializePartitions`, without fallback. The nested case is source-traced; the
re-added-column case was observed on synthetic merge
`282cc8faa482c4254a3173cfd7087e1130128712`, whose relevant Comet files match
this head. No local tests were run.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
