andygrove opened a new issue, #5257: URL: https://github.com/apache/datafusion-comet/issues/5257
## Describe the bug Three helpers reached from `CometIcebergNativeScan.serializePartitions` answer a reflection failure with an empty collection or a skipped field and a log line. Serde runs after `CometScanRule` has committed the plan to the native scan, so the empty answer is not a fallback: it is fed straight into the plan native executes. ### 1. `buildFieldIdMapping` returns an empty map `spark/src/main/scala/org/apache/comet/iceberg/IcebergReflection.scala:808-834` logs a warning and returns `Map.empty` if the schema walk fails, and drops individual columns from the map if a per-column lookup fails. At serde time that result feeds three places: - `CometIcebergNativeScan.scala:1091-1104`, resolving output columns to field ids. A name missing from the mapping falls through to `metadata.globalFieldIdMapping` and then `MetadataFieldIds` before throwing, so a partial mapping can resolve a column to a different field id rather than failing. - `:920-924` (`hasHistoricalColumns`), where an empty table-schema mapping makes the "does the scan reference dropped columns" test come out false and silently changes which schema the task is serialized with. - `IcebergReflection.scala:543` (`schemaWithRequiredFields`), where an empty `existingIds` makes every equality-delete field id look absent from the task schema, sending the code down the schema-history resolution path unnecessarily and throwing there if history cannot supply them. ### 2. `pageIndexUnsupportedColumns` returns an empty set `IcebergReflection.scala:847-870` logs a warning and returns `Set.empty`. Its own doc comment (`:838-846`) explains that residual predicates on decimal/uuid/fixed/binary columns either fail the native scan outright as an unsupported index type or panic it on a `String::from_utf8(..).unwrap()` over non-UTF-8 column-index bounds. An empty set means exactly those predicates get pushed. The failure is loud, but it surfaces as a native panic with no connection to the reflection failure that caused it. ### 3. `PartitionSpecParser.toJson` resolves to `None` `CometIcebergNativeScan.scala:965-977` resolves the accessor into an `Option` and `:426-453` logs a per-task warning when it is absent, leaving `partition_spec_idx` unset while the partition *data* is still written unconditionally at `:529-544`. The comment at `:491-501` states native requires a task to carry both a spec and data or neither. `partition_spec_idx` is `optional` in `operator.proto:350` and `planner.rs:4031-4034` maps an unset index to `None`, so this is not silently aliased to pool entry 0 — it becomes an iceberg-rust error. As with (2) the outcome is a native failure whose message says nothing about the reflection problem, plus one warning per file scan task. ## Expected behavior At serde time these should fail the query with a message naming the reflection failure, the way `extractDeleteFilesList:337-343` and `serializePartitionData:553-559` already do. The planning-path callers of the same helpers should keep the current quiet behavior, since `CometScanRule` still has the option to fall back. That split is what makes this non-trivial: `buildFieldIdMapping` and `pageIndexUnsupportedColumns` are called from both paths, so the fix needs either separate entry points or an error-returning variant rather than a blanket change. ## Additional context Found while auditing the serde path after review feedback on #5222. Companion issues cover the delete-file fields that fall back to wrong defaults, and the `IcebergReflection` API change that lets callers tell "this Iceberg version lacks the accessor" from "the invoke threw". -- 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]
