Repository navigation
deps: bump to datafusion 55.2.0 - #6766
Conversation
sunchao
left a comment
There was a problem hiding this comment.
Reviewed 0571a6344323d258dfa72432551dce3fcb6c9051. No introduced P1/P2 issues found within this review.
Scope covered all six files in the PR diff against supplied base e76c35f1e782b78525d447ba09d6f580f9bf6dec, using merge base a32529c4d84f5157be7a23aa0742ecdc15090eb7. There are no stacked prerequisites. The PR is not a draft. Snapshot and live discussion checks contained no reviews, issue comments, inline comments, or review threads.
Routed skills: review-comet-pr and review-comet-expression-pr, with the contributor and timezone guides. Compared the source of all 32 upgraded DataFusion crates and verified downloaded 55.2.0 artifacts against lockfile checksums.
Summary
- Prior state and problem: Comet used DataFusion 55.1.0. Its INT96 schema coercion discarded metadata on rebuilt struct, list, and map fields. This update incorporates the upstream metadata fix and other maintenance changes.
- Design approach: Upgrade the DataFusion dependency family together, synchronize the standalone Delta dependency, refresh both lockfiles, and update the scan comment describing metadata preservation.
- Correctness: The metadata fix preserves field IDs without changing timestamp coercion units or timezone labels. Compared field-ID resolution and INT96 handling against Spark sources. Expression and execution kernels are unchanged between the upgraded crate versions. No correctness issue met the P1/P2 reporting bar.
- Compatibility analysis: Checked relevant Spark sources for 3.4.3, 3.5.9, 4.0.4, 4.1.3, and 4.2.0. Existing version-specific INT96-to-NTZ behavior remains unchanged. Locked Arrow/Parquet 59.3.0 and object_store versions remain unchanged, as does DataFusion's Rust minimum.
- Key design decisions: Comet retains explicit
crypto_expressionsandserde_json/preserve_orderfeatures. Removing their implicit activation upstream therefore does not remove Comet's crypto functions or change JSON key ordering. - Implementation sketch: Three manifests change version requirements, two lockfiles resolve the new crate family, and one documentation comment changes. The lockfile also switches
tempfileto the already-presentgetrandom0.4.3 dependency. - Performance: No expression or execution hot-loop changes were found. Preserving metadata adds cloning during schema coercion. The upstream dependency reduction does not establish a Comet runtime speedup because Comet still enables the relevant features. No evidence-backed performance regression was identified.
- Design: Taking the upstream fixes keeps this change small and avoids introducing a Comet-specific workaround. Comet constructs and executes physical plans directly, so the upstream distribution-reoptimization fix does not alter its normal planning path.
- Abstraction & complexity: No new Comet abstraction, configuration, or shim is introduced. The upstream JSON serialization changes are confined to plan display and preserve ordering explicitly.
- Behavioral changes worth calling out: Compared with
branch-1.1, which also uses DataFusion 55.1.0, preserving container field IDs through INT96 coercion is an intended improvement. The raw-Parquet missing-field-ID check remains necessary and unchanged. No unintended behavior change was identified. - Suggested improvements: No code improvement supported by a reproducible P1/P2 issue was identified.
Exact-head CI: only the successful label check was present. There was no build or test verdict, including for the requested macOS and Delta coverage.
Validation: four upstream schema-coercion tests passed in a disposable harness using the checksum-verified 55.2.0 source and the PR's locked Arrow/Parquet 59.3.0 versions. These cover nested metadata, mixed timestamps, nested types, and timezone labels. git diff --check passed. Full workspace validation was blocked before compilation because the configured registry mirror lacks DataFusion 55.2.0. Comet JVM, Spark SQL, and platform integration suites were not run. The focused tests do not establish those integration results.
Which issue does this PR close?
N/A. Routine dependency bump, no issue filed.
Rationale for this change
Pick up the fixes in DataFusion 55.2.0. It is a patch release over 55.1.0 with seven
backported changes. The changes most relevant to Comet are:
datafusion-sparkdependency footprintserde_json/preserve_orderfeature from DataFusion library cratesFull changelog: https://github.com/apache/datafusion/blob/55.2.0/dev/changelog/55.2.0.md
What changes are included in this PR?
datafusion-functions-nestedand the Delta contrib crate to 55.2.0.cargo update.Arrow, Parquet, and object_store remain unchanged.
How are these changes tested?
Locally verified on macOS/aarch64:
cargo check --workspace --all-targets --lockedcargo clippy --workspace --all-targets --locked -- -D warningscargo fmt --all -- --checkcargo check -p datafusion-comet --features contrib-delta --lockedcargo check --locked --all-targetsincontrib/delta/nativeThe
run-macos-testsandrun-delta-build-gatelabels request the corresponding CI coverage.