andygrove opened a new issue, #5758:
URL: https://github.com/apache/datafusion-comet/issues/5758

   ### Describe the bug
   
   Iceberg CI is red on `main`. 
`TestForwardCompatibility.testSparkCanReadUnknownTransform` fails on the 
Iceberg 1.8.1 legs (Spark 3.4 and 3.5):
   
   ```
   TestForwardCompatibility > testSparkCanReadUnknownTransform FAILED
       org.apache.spark.SparkException: Job aborted due to stage failure: Task 
0 in stage 0.0 failed 1 times,
       most recent failure: Lost task 0.0 in stage 0.0 (TID 0) (localhost 
executor driver):
       org.apache.comet.CometNativeException: General execution error with 
reason:
       Invalid Iceberg scan task: DataInvalid => Non-empty FileScanTask 
partition requires a partition spec.
   ```
   
   The table under test is partitioned by a `zero` transform that no Iceberg 
release defines, standing in for a table written by a newer Iceberg. Reading it 
is supposed to work — that is Iceberg's forward-compatibility contract, and the 
data columns are all present in the file.
   
   Root cause: Iceberg Java parses an unrecognized transform into an 
`UnknownTransform` whose `toString()` is the original name (`zero`), and whose 
`getResultType()` is **`StringType`, not `UnknownType`**. So the existing "drop 
partition fields with an unknown type" filter in 
`CometIcebergNativeScan.serializePartitionData` does not drop the field, and 
the spec JSON reaches native with `"transform":"zero"` alongside a real 
partition value for it.
   
   Native-side, `serde_json::from_str::<PartitionSpec>` rejects `zero`, so the 
scan task ends up with `partition = Some(..)` and `partition_spec = None`. 
iceberg-rust's `FileScanTask::validate()` rejects exactly that pair.
   
   This is a regression from #5262, which bumped iceberg-rust to a revision 
where `FileScanTask`'s fields are private and construction goes through a 
validating builder. Before that the task was built by struct literal with no 
validation, and nothing in the read path consumed the missing spec for this 
query, so it happened to work.
   
   The native unit test that looks like it covers this case, 
`test_unified_partition_type_tolerates_unparseable_spec`, passes only because 
it omits `partition_data_idx`. Its comment claims the Scala side filters the 
unknown-transform field out as an unknown type, which was never true.
   
   ### Steps to reproduce
   
   Run the Iceberg Java suites against Iceberg 1.8.1 / Spark 3.4 (the 
`iceberg-spark/iceberg-1.8.1/spark-3.4.3` CI job), or just 
`TestForwardCompatibility`.
   
   ### Expected behavior
   
   The scan reads the table and returns its rows, as it does without Comet.
   
   An unknown transform is exactly what iceberg-rust's `Transform::Unknown` 
models, and it is conservative in the right direction: it contributes no 
partition constants and no pruning. Its result type is `string`, matching what 
`UnknownTransform.getResultType` reports, so serializing the transform name as 
`unknown` keeps the partition type Comet already sends alongside the spec in 
agreement with it.
   
   ### Additional context
   
   Not reachable on Spark 4.1+: Spark validates a V2 relation's metadata 
columns on every read, which forces `SparkTable.metadataColumns()` -> 
`Partitioning.partitionType()` and rejects an unknown transform in the 
analyzer, with or without Comet. Iceberg disabled its own copy of this test on 
4.1 for that reason (SPARK-55626), which is why only the 1.8.1 legs are failing.
   
   The write side needs no equivalent change: Iceberg Java refuses to write 
through an unknown transform (`Cannot write using unsupported transforms: 
zero`) before Comet is involved, which `testSparkWriteFailsUnknownTransform` 
asserts.
   


-- 
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]

Reply via email to