chore: Update to latest main on upstream DataFusion - #1662
Conversation
AI Disclosure: This code was written in part by an AI agent.:
AI Disclosure: This code was written in part by an AI agent.:
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Updates this repo to track the latest upstream DataFusion main branch to reduce lead time for the upcoming DF55 release, with accompanying API/test adjustments for new enum variants and PyO3 upgrades.
Changes:
- Bump Arrow/DataFusion/PyO3 dependencies and align FFI/Capsule API usage with newer PyO3.
- Update Python/Rust bindings and tests for DataFusion API changes (e.g.,
CreateExternalTablelocations, newWriteOpvariants). - Adjust and extend Python test suite to reflect changed upstream behavior (CSV list paths, Spark slice behavior, sqrt negative-input handling).
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| python/tests/test_sql.py | Uses distinct CSV paths when registering multiple files. |
| python/tests/test_spark_functions.py | Adds slice coverage and marks Spark-array behavior as xfail. |
| python/tests/test_lambda.py | Updates lambda expr serialization expectations via pickle round-trip. |
| python/tests/test_io.py | Adjusts multi-path CSV read test to use distinct file paths. |
| python/tests/test_functions.py | Avoids sqrt on negatives in general math test; adds negative-input rejection test. |
| python/tests/test_expr.py | Adds deprecation-warning test for CreateExternalTable.location(). |
| python/tests/test_context.py | Mirrors CSV multi-path adjustments in SessionContext tests. |
| python/datafusion/functions/spark.py | Updates slice doc example to operate on an array column. |
| python/datafusion/expr.py | Adds deprecated CreateExternalTable.location() shim delegating to locations(). |
| examples/datafusion-ffi-example/src/window_udf.rs | Updates PyCapsule creation API for PyO3 0.29. |
| examples/datafusion-ffi-example/src/table_provider_factory.rs | Updates PyCapsule creation API for PyO3 0.29. |
| examples/datafusion-ffi-example/src/table_provider.rs | Updates PyCapsule creation API for PyO3 0.29. |
| examples/datafusion-ffi-example/src/table_function.rs | Updates PyCapsule creation API for PyO3 0.29. |
| examples/datafusion-ffi-example/src/scalar_udf.rs | Updates PyCapsule creation API for PyO3 0.29. |
| examples/datafusion-ffi-example/src/physical_optimizer.rs | Updates PyCapsule creation API for PyO3 0.29. |
| examples/datafusion-ffi-example/src/physical_extension_codec.rs | Adapts PhysicalExtensionCodec trait signature changes; updates PyCapsule creation. |
| examples/datafusion-ffi-example/src/logical_extension_codec.rs | Updates PyCapsule creation API for PyO3 0.29. |
| examples/datafusion-ffi-example/src/config.rs | Updates PyCapsule creation API for PyO3 0.29. |
| examples/datafusion-ffi-example/src/catalog_provider.rs | Fixes record_batch import source; updates PyCapsule creation API. |
| examples/datafusion-ffi-example/src/aggregate_udf.rs | Updates PyCapsule creation API for PyO3 0.29. |
| examples/datafusion-ffi-example/python/tests/_test_table_provider_factory.py | Updates SQL to provide non-empty LOCATION literal for DF changes. |
| crates/util/src/lib.rs | Improves PyCapsule name UTF-8 error mapping; updates PyCapsule creation API. |
| crates/core/src/substrait.rs | Updates Substrait deserialization call site to match upstream API. |
| crates/core/src/sql/logical.rs | Adapts to upstream DDL plan container type changes. |
| crates/core/src/expr/recursive_query.rs | Uses fallible RecursiveQuery constructor and returns PyDataFusionResult. |
| crates/core/src/expr/drop_catalog_schema.rs | Aligns TableReference import location. |
| crates/core/src/expr/dml.rs | Handles new/unknown WriteOp variants with PyNotImplementedError. |
| crates/core/src/expr/create_external_table.rs | Migrates CreateExternalTable from location to locations. |
| crates/core/src/dataframe.rs | Updates Arrow stream capsule creation; adapts fill_null signature change. |
| crates/core/src/context.rs | Updates PyCapsule creation API for PyO3 0.29. |
| crates/core/src/codec.rs | Adapts PhysicalExtensionCodec physical-expr encode/decode context changes. |
| crates/core/src/array.rs | Updates Arrow schema/array capsule creation API for PyO3 0.29. |
| crates/core/Cargo.toml | Enables datafusion-spark core feature. |
| Cargo.toml | Bumps workspace deps; adds git patches pinning DF crates to upstream rev. |
Suppressed comments (2)
python/tests/test_spark_functions.py:1
- The xfail reason references Spark array field names, but the test only asserts values. With
strict=True, this becomes brittle (an upstream fix that preserves names but keeps values correct will turn into an XPASS failure without actually validating the reason). Consider asserting the schema/field-name behavior that is currently broken, or adjust the xfail reason to match the actual failing condition.
python/tests/test_functions.py:1 - Matching the full error string makes this test fragile to upstream wording changes. Consider matching a more stable substring/regex (e.g., just
square root/negative) or asserting on an error code/type if one is available in this project’s Python exception hierarchy.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
AI Disclosure: This code was written in part by an AI agent.:
kosiew
left a comment
There was a problem hiding this comment.
Thanks for working on this! The changes look good overall and I did not find any blocking issues. I have one small suggestion that could strengthen the test coverage around the new upstream API behavior.
| return ctx | ||
|
|
||
|
|
||
| def test_create_external_table_location_is_deprecated(): |
There was a problem hiding this comment.
Nice to see the deprecated shim covered. One thought: since this test calls the shim with a MagicMock, it does not verify that CreateExternalTable.locations() is actually bound to the upstream locations field.
Would it make sense to build a real CREATE EXTERNAL TABLE logical plan instead and assert both locations() and the deprecated location() on the resulting variant? That would exercise the Rust/Python boundary introduced by this upgrade and give a bit more confidence that the binding is wired up correctly.
Which issue does this PR close?
No particular issue. This PR is to update our datafusion dependency so that when DF55 is released we can release our project more quickly.
Rationale for this change
Prepare for DF55 release.
What changes are included in this PR?
Are there any user-facing changes?
None.