Skip to content

Commit 9935bc6

Browse files
timsaucerclaude
timsaucer
and
claude
committed
docs: correct the pin rationale and derived-context caveats
The Cargo.toml comment justified the pre-release pin by "the FFI codec rebinding in `PySessionContext::derived_parts`", a symbol that no longer exists. That rebinding was not removed, it moved into `set_session_query_planner`, which every `with_*` method calls. It depends on `FFI_QueryPlanner::new_with_ffi_codecs` unwrapping a `ForeignQueryPlanner` and replacing its codecs, a swap that is a silent no-op before 55.1.0 (apache/datafusion#24722). So the pin is still required, not droppable. The `with_*` methods rebuild the installed planner on the *shared* session, so the rebind takes effect even when the returned context is discarded. `with_python_udf_inlining` additionally claimed "the original session is unchanged", which the rebuild contradicts; it is the context's own codec settings that are unchanged. Also: - Note that the arity-vs-body TypeError split in `call_capsule_getter` holds only because the call originates in Rust. A Python-level shim between the host and the getter would supply a traceback and silently disable it. - Document the new FFI major-version gate in the 55.0.0 upgrade guide. Table providers previously performed no such check, so a mismatched extension library that used to load now raises ImportError. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
1 parent 78cc12b commit 9935bc6

5 files changed

Lines changed: 49 additions & 12 deletions

File tree

‎Cargo.toml‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,14 @@ codegen-units = 2
7878
# We cannot publish to crates.io with any patches in the below section. Developers
7979
# must remove any entries in this section before creating a release candidate.
8080
#
81-
# When these are removed, raise the DataFusion requirement above to 55.1.0. The
82-
# FFI codec rebinding in `PySessionContext::derived_parts` is a silent no-op
83-
# before that release (apache/datafusion#24722), so a 55.0.0 build would compile
84-
# and then resolve decode callbacks against the pre-fork session.
81+
# When these are removed, raise the DataFusion requirement above to 55.1.0.
82+
# `PySessionContext::set_session_query_planner` rebuilds an installed foreign
83+
# query planner against the session's current codecs, and relies on
84+
# `FFI_QueryPlanner::new_with_ffi_codecs` unwrapping a `ForeignQueryPlanner` and
85+
# replacing its codecs. That replacement is a silent no-op before 55.1.0
86+
# (apache/datafusion#24722), so a 55.0.0 build would compile and then keep
87+
# decoding with whichever codecs the planner was installed with -- installing a
88+
# codec after `set_query_planner` would not take effect.
8589
[patch.crates-io]
8690
datafusion = { git = "https://github.com/apache/datafusion", rev = "1a944f43983188712aa1934d034f3087d82172e1" }
8791
datafusion-substrait = { git = "https://github.com/apache/datafusion", rev = "1a944f43983188712aa1934d034f3087d82172e1" }

‎crates/util/src/lib.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,13 @@ pub fn call_capsule_getter<'a, 'py: 'a>(
343343
// machinery before the getter's frame exists, so nothing unwinds and no
344344
// traceback is attached. An error from the body unwinds that frame and
345345
// carries one.
346+
//
347+
// This holds because the call originates in Rust. `call1` pushes no
348+
// Python caller frame, so an arity error has nothing to unwind and
349+
// arrives bare. If a Python-level shim is ever interposed between the
350+
// host and the getter, its frame would supply a traceback and this
351+
// check would quietly stop firing -- the diagnostic would degrade to
352+
// the bare `TypeError` it exists to replace, with no test failing.
346353
if err.traceback(py).is_some() {
347354
return err;
348355
}

‎docs/source/contributor-guide/ffi.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,10 @@ so it is a property of the session rather than of a handle on it, and installing
348348
visible to every context sharing that session — including ones a `with_*` call returned
349349
earlier. Installing a codec on a session that already has a foreign planner rebuilds
350350
that planner against the new codec for the same reason: there is one planner, and it has
351-
to carry the codecs currently in force.
351+
to carry the codecs currently in force. This happens on the shared session, so it takes
352+
effect even if the returned context is discarded — `ctx.with_python_udf_inlining(...)`
353+
whose result is thrown away still leaves the session's planner carrying the codecs of
354+
that discarded handle.
352355

353356
```python
354357
ctx = SessionContext(config)

‎docs/source/user-guide/upgrade-guides.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,23 @@ way `add_physical_optimizer_rule` does and returns nothing — the query planner
8383
lives in `SessionState`, so it belongs to the session rather than to a
8484
particular handle on it. See the {ref}`ffi` guide for the full protocol.
8585

86+
### Mismatched extension libraries now fail loudly
87+
88+
Objects imported through the capsule protocol are checked against the major
89+
version of `datafusion-ffi` this package was built with. A table provider,
90+
extension codec, or query planner produced by a library built against a
91+
different DataFusion major version now raises an `ImportError` naming the
92+
version found and the one expected, instead of being used as-is. Table
93+
providers previously performed no such check.
94+
95+
This is a diagnostic rather than a soundness guarantee — reading the version
96+
out of the struct already assumes the local field layout — but it turns the
97+
common "extension library built against the wrong DataFusion" mistake into a
98+
clear message rather than undefined behaviour on first use.
99+
100+
`FFI_TaskContextProvider`, `FFI_TableProviderFactory`, and `FFI_ExtensionOptions`
101+
carry no version field, so objects of those types cannot be checked.
102+
86103
### Changes to the `datafusion-python-util` crate
87104

88105
Extension libraries written in Rust usually depend on the

‎python/datafusion/context.py‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2257,7 +2257,9 @@ def with_logical_extension_codec(
22572257
The returned context shares its session state with the original, so a
22582258
later registration on either is visible to both. If a custom query
22592259
planner is installed, it is rebuilt against the new codec on the shared
2260-
session, so the original context plans with the new codec too.
2260+
session, so the original context plans with the new codec too. This
2261+
happens on the shared session, so it takes effect even if the returned
2262+
context is discarded.
22612263
"""
22622264
new_internal = self.ctx.with_logical_extension_codec(codec)
22632265
new = SessionContext.__new__(SessionContext)
@@ -2283,7 +2285,9 @@ def with_physical_extension_codec(
22832285
The returned context shares its session state with the original, so a
22842286
later registration on either is visible to both. If a custom query
22852287
planner is installed, it is rebuilt against the new codec on the shared
2286-
session, so the original context plans with the new codec too.
2288+
session, so the original context plans with the new codec too. This
2289+
happens on the shared session, so it takes effect even if the returned
2290+
context is discarded.
22872291
"""
22882292
new_internal = self.ctx.with_physical_extension_codec(codec)
22892293
new = SessionContext.__new__(SessionContext)
@@ -2326,11 +2330,13 @@ def with_python_udf_inlining(self, *, enabled: bool) -> SessionContext:
23262330
regardless of the toggle.
23272331
23282332
Returns a new :class:`SessionContext` with the toggle applied;
2329-
the original session is unchanged. The returned context shares
2330-
its session state with the original, so a later registration on
2331-
either is visible to both. If a custom query planner is
2332-
installed, it is rebuilt against the new codecs on the shared
2333-
session, so the original context plans with them too.
2333+
the original context's own codec settings are unchanged. The
2334+
returned context shares its session state with the original, so
2335+
a later registration on either is visible to both. If a custom
2336+
query planner is installed, it is rebuilt against the new codecs
2337+
on the shared session, so the original context plans with them
2338+
too. This happens on the shared session, so it takes effect even
2339+
if the returned context is discarded.
23342340
23352341
Examples:
23362342
>>> import pyarrow as pa

0 commit comments

Comments
 (0)