andygrove opened a new issue, #5709: URL: https://github.com/apache/datafusion-comet/issues/5709
### What is the problem the feature request solves? #5262 bumps DataFusion to 55.0 and Arrow/Parquet to 59.2. It is a large branch that has accumulated review findings which are not blockers for the bump itself, but which should not be lost when it merges. Several had no issue at all, and the ones that did were not linked to each other. This epic collects everything raised in review of #5262 that is deliberately **not** being fixed on the branch, so the branch can merge on the strength of the bump alone. **What is already done on the branch**, and so is not listed below: - Compiler migrations for the new DataFusion/arrow APIs (`GroupsAccumulator`, `IpcWriteContext`, `FixedSizeBinaryArray::try_from`, `ExecutionPlan::apply_expressions`, the iceberg-rust builder/getter APIs). - `spark_sqrt`, needed because DataFusion 55 added `validate_sqrt_input`, so `sqrt(-1)` now errors where DataFusion 54 returned NaN. - #4859 nested Parquet leaf pruning, via `is_pure_structural_narrowing`. - The Celeborn map-key assertion for Arrow 59.2 (#5697). - Iceberg partition spec/value/source-column consistency, which was failing 10 tests across the two Iceberg CI jobs. ### Describe the potential solution Each item is a linked issue; check it here when it closes. **Correctness** - [ ] #5701 `array_distinct` and `array_union` diverge from Spark on `-0.0` for Spark versions without SPARK-54918. Needs a runtime, patch-level version check: the fix versions are 4.2.0, 4.1.4 and 4.0.5, so a compile-time shim keyed on the minor version is not enough, and the existing string comparison of `SPARK_VERSION` breaks at 4.0.10. - [ ] #5707 Nested Parquet cast retention accepts an ambiguous case-insensitive field match, so a query Spark rejects can silently return one of two candidate fields. **Metrics and memory accounting** - [ ] #5703 Re-enable the two ignored `CometAggregateSuite` metric tests after the DataFusion 55 `peak_mem_used` change (apache/datafusion#22710). - [ ] #5704 Establish whether the TPC-H off-heap bump from 2g to 4g is a real requirement, and changelog it if so. **Error handling** - [ ] #5705 `SpillWriter::path()` conflates "nothing spilled" with "backend has no local path", so a pathless spill backend would silently drop spilled data. - [ ] #5706 `copy_array` panics on offset overflow instead of returning an error, aborting the JVM rather than failing the task. **Performance** - [ ] #5708 Slice arrays before normalizing, to avoid allocating over the whole backing buffer. **Test hygiene** - [ ] #5702 Signed-zero array fixtures claim the literal case agrees with Spark when it does not. ### Additional context Review threads on #5262 from @sunchao, @andygrove, @comphead and @ajsquared. The threads opened by `ziting-openai` were withdrawn as the output of a misconfigured agent; where a finding in them was independently confirmed it is captured above, and the rest are not carried forward. -- 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]
