andygrove opened a new pull request, #6404:
URL: https://github.com/apache/datafusion-comet/pull/6404

   ## Which issue does this PR close?
   
   N/A. No issue was filed for this. It is a draft that tracks DataFusion main 
ahead of the next DataFusion release. Related: #5477, whose Arrow 60 items it 
unblocks.
   
   ## Rationale for this change
   
   DataFusion main is at `4a5d580` (2026-09-29). Since 55.1.0 it has moved to 
arrow/parquet 60.0.0 and object_store 0.14.2 (apache/datafusion#25335), and it 
has a number of API changes (see its [56.0.0 upgrade 
guide](https://github.com/apache/datafusion/blob/4a5d58006e620bf6a175bb578be6652c3b330da9/docs/source/library-user-guide/upgrading/56.0.0.md)).
 Building Comet against main now surfaces the breaks early and lets CI run the 
Comet suites against it, rather than finding everything at release time.
   
   ## What changes are included in this PR?
   
   **Dependencies**
   
   - `datafusion`, `datafusion-datasource`, `datafusion-physical-expr-adapter` 
and `datafusion-spark` in the workspace now use `git = 
"https://github.com/apache/datafusion";, rev = "4a5d580..."`. So do the 
`datafusion-functions-nested` dev-dependency in core and `datafusion` in both 
contrib crates.
   - `arrow`, `arrow-data`, `arrow-select` and `parquet` go from 59.2.0 to 
60.0.0.
   - `object_store` goes from 0.13.2 to 0.14.2. `object_store_opendal` goes 
from 0.58.0 to 0.59.0, the first release on object_store 0.14. That release 
keeps opendal on 0.58, which is what iceberg-rust uses.
   - `iceberg` and `iceberg-storage-opendal` go from `bb1e4a4` to `0d5eb12`, 
the head of apache/iceberg-rust#3257 ("Bump to Arrow 60").
     - iceberg-rust main is still on arrow 59. Comet hands arrow types to 
iceberg-rust directly, so the two must agree.
     - apache/iceberg-rust#3257 is open with green CI and awaiting review. The 
Cargo.toml comment says to move back to a main revision once it merges.
     - Its base is three commits behind the old pin, so this temporarily drops 
apache/iceberg-rust#3235 (a manifest-list cache key fix), 
apache/iceberg-rust#2765 (transform-based sort orders in transactions) and 
apache/iceberg-rust#3254 (a row-group stats speedup). None of them changes an 
API Comet uses, and the build and the Iceberg suite pass without them. The last 
one is a speedup to row-group pruning that the native Iceberg scan picks up 
again once the pin returns to main.
   - Lockfiles: `native/Cargo.lock` changes only for the DataFusion git source 
and for arrow/parquet/object_store and what they pull in (sqlparser 0.63, 
brotli 9, comfy-table 8 and so on). `contrib/delta/native/Cargo.lock` is 
refreshed to match.
   
   **Source changes (all forced by the upgrade)**
   
   - `AlignedArrowStreamReader` is replaced by arrow's stock 
`ArrowArrayStreamReader`.
     - Arrow 60 made the `FFI_ArrowArrayStream` callbacks private, so the 
custom reader no longer compiles.
     - The reader existed only to realign under-aligned `Decimal128` buffers 
from the JVM. Its own doc said to drop it once Comet was on arrow >= 59, where 
`from_ffi_and_data_type` realigns them itself (apache/arrow-rs#10030).
     - Its regression test moves to `scan.rs` and exercises that import.
     - One visible difference: a failed `get_next` now reports `Cannot get next 
batch from input stream. Error code: N. Producer error: ...` instead of only 
the producer's message.
   - Parquet 60 moved the page index behind `ParquetMetaData::page_index()`. 
`with_spark_arrow_schema` carries it over with `set_page_index`, and the tests 
check `page_index()` instead of `column_index()`/`offset_index()`.
   - object_store 0.14 added `GetResult::extensions`, which 
`ScanIoObjectStore::get_opts` now forwards.
   - `OffsetBuffer::first()`/`last()` now return values instead of `Option`s. 
This affects `list_positions.rs` and a `map_sort` test.
   - Field and schema metadata are now arrow's `Metadata` type, which has no 
`capacity()`. The shuffle schema cache's size estimate uses `len()` instead.
   - `fb_to_schema` is deprecated. The shuffle decoder uses `try_fb_to_schema`, 
so an invalid schema message now returns a decode error where it used to panic.
   - `FFI_ArrowSchema::with_metadata` is now `unsafe`. The call in 
`move_to_spark` is sound because the schema was just built by `TryFrom`.
   - `HashJoinExec::with_dynamic_filter_expr` is deprecated in DataFusion 56 
with no replacement.
     - Upstream installs a join's dynamic filter only from 
`handle_child_pushdown_result`. `DynamicFilterJoinExec` wires it at runtime 
instead, so the call stays, under `#[allow(deprecated)]`.
     - The deprecation note asks for an issue if you have a use case. Comet has 
one, so it is probably worth filing one before the setter is removed.
   - `normalize_spark_empty_key_metadata_rejects_other_malformed_encodings`: 
Arrow 60 accepts empty entries in unsorted Variant dictionaries 
(apache/arrow-rs#10352).
     - Two of the five "malformed" dictionaries in the test were rejected only 
because of their empty entry: the one with a repeated entry and the one with 
trailing bytes.
     - Both now pass through unchanged. That matches Spark, which checks only 
the offsets of the keys it looks up, so the test now asserts it.
   
   **DataFusion behavior changes checked against Comet**
   
   - The `floor`/`ceil` return types and the result of `map_extract` for absent 
keys changed. Comet registers its own `floor`, `ceil` and `map_extract`, so 
neither change reaches Comet.
   - Missing Parquet null counts are now treated as unknown instead of zero. 
This is a correctness fix. It can disable some pruning on files written by 
parquet-rs before 53.1.
   
   **Not included (possible follow-ups)**
   
   - #5477: Arrow 60 contains apache/arrow-rs#10810 and apache/arrow-rs#10352, 
so the encoded-metadata decoding and the empty-key retry 
(`canonicalize_spark_empty_key_metadata`) can now be removed. Spark's encoding 
no longer reaches the retry, because Arrow accepts it on the first attempt.
   - Move the iceberg pin back to a main revision once apache/iceberg-rust#3257 
merges.
   
   ## How are these changes tested?
   
   These are existing tests, run locally on macOS aarch64:
   
   - `cargo clippy --all-targets --workspace -- -D warnings`, the jemalloc 
clippy run, `cargo fmt --check`, `cargo machete` and `cargo check -p 
datafusion-comet --all-targets --features contrib-delta,contrib-lance` are all 
clean.
   - `cargo nextest run` in `native/`: 1835 passed.
   - JVM suites on the default Spark 4.1 profile, 1388 tests with no failures:
     - `CometExecSuite`, `CometAggregateSuite`, `CometJoinSuite`, 
`CometWindowExecSuite` and `CometTopKSuite`
     - `CometNativeReaderSuite`, `ParquetReadV1Suite`, 
`CometNativeShuffleSuite` and `CometShuffleSuite`
     - `CometSqlFileTestSuite`
     - `CometIcebergNativeSuite`
     - `CometVariantProjectionSuite`, `CometVariantTypeSuite` and 
`CometVariantShreddingSuite`
   
   Not run yet: the other Spark profiles, Spark's own SQL tests and the Iceberg 
Spark tests. Those need the `run-all-spark-profiles`, `run-spark-4.1-tests` and 
`run-iceberg-tests` labels.
   


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