comphead commented on PR #5262: URL: https://github.com/apache/datafusion-comet/pull/5262#issuecomment-5552825912
I tried to address Chao original comments ``` [P2] Nested Parquet pruning bypasses Spark’s ambiguity check. [schema_adapter.rs:132](https://github.com/apache/datafusion-comet/blob/f536ca7f9bcd49b1012f2017cdcf67555b38410d/native/core/src/parquet/schema_adapter.rs#L132) accepts an exact id match within struct<id:bigint,ID:bigint> when case sensitivity is disabled. The retained DataFusion cast returns id; Spark and Comet’s existing converter reject the ambiguity. Reproduced through the actual adapter for both structs and lists of structs. Require uniqueness under Spark’s configured name resolver before retaining the cast. [P2] Iceberg partition-source augmentation creates invalid schemas. [CometIcebergNativeScan.scala:1092](https://github.com/apache/datafusion-comet/blob/f536ca7f9bcd49b1012f2017cdcf67555b38410d/spark/src/main/scala/org/apache/comet/serde/operator/CometIcebergNativeScan.scala#L1092) calls a helper that checks only top-level IDs. A partition source already present as s.region gets appended again, causing a duplicate-ID exception. Dropping and re-adding category can instead produce conflicting same-name fields with different IDs. Both failures reproduced using the exact Scala helper and Iceberg 1.11. Resolve IDs recursively and preserve nesting; exclude partition sources that are omitted from the serialized representation. [P2] Mixed live/dropped v1 partitions fail the new task validation. [planner.rs:4540](https://github.com/apache/datafusion-comet/blob/f536ca7f9bcd49b1012f2017cdcf67555b38410d/native/core/src/execution/planner.rs#L4540) rejects a serialized [identity(region), void(category)] spec containing only the surviving region value. I passed the exact Scala-emitted protobufs into the native parser and reproduced “partition has 1 fields but partition spec has 2 fields.” Preserving a null placeholder makes the control pass. Keep partition specs and values consistent through evolution. ``` now addressing conflicts, I'm not sure btw how those reported issues matches to GH issues created by Andy -- 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]
