chore(deps): bump DataFusion to 54.1.0 - #114
Merged
Merged
Conversation
Move `datafusion`, `datafusion-proto`, `datafusion-spark` and `datafusion-substrait` from 53.1.0 to 54.1.0, and move the `datafusion.version` Maven property in lock step so the generated Java protobuf classes are built from the matching upstream `.proto` definitions. `arrow` (58) and `object_store` (0.13) are unchanged -- 54.1 resolves to the same majors. Adapt to the upstream API changes the bump requires: - `TableProvider`, `ExecutionPlan` and `ScalarUDFImpl` now take `Any` as a supertrait, so the manual `as_any` overrides are no longer trait members. - `MemoryPool` gained a `name()` method and a `Display` supertrait; `TrackingMemoryPool` implements both, deferring to the inner pool for the usage detail. - `CacheManagerConfig::table_files_statistics_cache` is now `file_statistics_cache`, and the accompanying limit is the on/off switch: `CacheManager::try_new` installs a default statistics cache whenever the limit is non-zero, even with an empty cache slot. An explicit `fileStatisticsCache(false)` from the Java surface therefore has to zero the limit, or upstream would install the cache the caller asked us to skip. - The `cache_unit` module is gone; the default cache impls moved to `cache::file_statistics_cache` and `cache`. - `DataFusionError::AvroError` is gone -- Avro is read through `arrow-avro` rather than `apache-avro` -- so Avro decode failures now arrive as `ArrowError::AvroError`, which the classifier already routed to `ExecutionException` alongside the `CsvError` / `JsonError` decoder variants. Drop the dead arm and the `avro` feature on `datafusion-jni-common` that existed only to gate it. Also correct the Substrait test fixture: it declared its base schema columns `NULLABILITY_REQUIRED` while registering a CSV, whose inferred schema is always nullable. DataFusion 54's Substrait consumer validates that a field a plan declares non-nullable really is non-nullable in the table, so the mismatch is now rejected -- correctly. The fixture is fixed to declare nullable columns; it only passed before because 53 did not check.
Member
Author
|
Merged. Thanks @milenkovicm! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
N/A — routine dependency bump; no tracking issue was filed.
Rationale for this change
Keeps the binding current with upstream DataFusion. 54.1.0 is the latest release
line, and staying close to it keeps the next bump small and makes upstream fixes
available to Java callers.
What changes are included in this PR?
Bumps
datafusion,datafusion-proto,datafusion-sparkanddatafusion-substraitfrom 53.1.0 to 54.1.0. Thedatafusion.versionMavenproperty moves in lock step, since it selects the upstream tag the
datafusion.proto/datafusion_common.protodefinitions are downloaded from —the generated Java protobuf classes must match what
datafusion-proto54.1.0decodes. The pinned sha512 digests for both protos are updated accordingly; each
was verified to match the copy vendored in the published
datafusion-proto/
datafusion-proto-common54.1.0 crates, independently of the GitHub download.arrow(58) andobject_store(0.13) are unchanged — 54.1.0 resolves to thesame majors, so the
object_storepin comment still holds.Adapting to the upstream API changes the bump requires:
TableProvider,ExecutionPlanandScalarUDFImplnow takeAnyas asupertrait, so the manual
as_anyoverrides are no longer trait members andare removed.
MemoryPoolgained aname()method and aDisplaysupertrait.TrackingMemoryPoolimplements both, following upstream's wrapper convention:name the wrapper, add the counters it exists to expose, and defer to the inner
pool for the usage detail.
CacheManagerConfig::table_files_statistics_cacheis renamed tofile_statistics_cache, and the accompanying limit is now the on/off switch —CacheManager::try_newinstalls a default statistics cache wheneverfile_statistics_cache_limit > 0, even when the cache slot isNone, and thedefault limit is non-zero. An explicit
fileStatisticsCache(false)from theJava surface therefore has to zero the limit as well; otherwise upstream would
install the very cache the caller asked us to skip. This is the one place the
bump would have silently changed observable Java behavior.
cache_unitmodule is gone; the default cache impls now live incache::file_statistics_cacheandcache.DataFusionError::AvroErroris gone — DataFusion 54 reads Avro througharrow-avrorather thanapache-avro. Avro decode failures now arrive asArrowError::AvroError, which the exception classifier already routed toExecutionExceptionalongside theCsvError/JsonErrordecoder variants,so the mapping stays coherent. The dead arm is dropped, along with the
avrofeature on
datafusion-jni-commonthat existed only to gate it.One test fixture is also corrected.
SessionContextSubstraitTestbuilt planswhose base schema declared both columns
NULLABILITY_REQUIRED, while the testsregister a CSV — whose inferred schema is always nullable. DataFusion 54's
Substrait consumer now validates that a field a plan declares non-nullable
really is non-nullable in the table, and rejects the mismatch. That check is
correct: a plan built around a "never null" assumption must not run against data
that can contain nulls. The fixture is fixed to declare nullable columns; it
only passed before because 53 did not check.
Are these changes tested?
Covered by the existing suites — this is a dependency bump, so the value is in
the current tests continuing to pass against the new version rather than in new
assertions.
./mvnw test— 349 tests, 0 failures. Run with thesubstraitCargo featureenabled (
cargo build -p datafusion-jni --features substrait) so theSubstrait tests execute rather than skip.
cargo test --workspace— all passing.cargo clippy --workspace --all-targets -- -D warnings— clean.cargo fmt --all -- --checkand./mvnw -q spotless:check— clean.cargo build --workspace --all-featureswithRUSTFLAGS="--cfg tokio_unstable",to cover the optional
substraitandruntime-metricsfeatures that thedefault build does not compile.
Are there any user-facing changes?
No API changes. Two behavioral notes, both inherited from upstream:
ExecutionExceptionrather thanIoException, following the move toarrow-avro. TheIoExceptionjavadocis updated to match.
Callers who never configured
CacheManagerOptionspick up upstream's newdefaults; an explicit
fileStatisticsCache(false)continues to disable thecache, as described above.