Skip to content

Commit 4296c5e

Browse files
timsaucerclaude
andcommitted
Correct three more claims in the extension bundle docs
`QueryPlannerExportable` said `session` is the `datafusion.context.SessionContext` the planner is being installed on. It is not. The capsule getters are called from Rust and receive the PyO3 context, so `isinstance(session, SessionContext)` is False -- while its repr reads `datafusion.SessionContext`, because the pyclass declares `module = "datafusion"`. It carries every capsule getter and `__datafusion_codec_id__`, which is all the protocol needs, so the fix is to say duck-type it rather than to change what is passed. The two bundle hooks are the exception and do receive the wrapper, since `with_extensions` dispatches them from Python; the ffi.md section on capsule getters now draws the same distinction. The `with_extensions` `Raises:` section listed ValueError for colliding codec ids only. A getter returning a capsule of the wrong kind also raises it -- `Expected name 'datafusion_query_planner' in PyCapsule, instead got 'datafusion_logical_extension_codec'` -- which `test_with_extensions_rejects_bad_codec_capsule` already pins. The `datafusion.extensions` module docstring said phase two runs the planner hook "once per bundle". Once per bundle that implements it; a bundle implements either hook or both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 07792ad commit 4296c5e

3 files changed

Lines changed: 31 additions & 8 deletions

File tree

docs/source/contributor-guide/ffi.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,14 @@ the session, and never touches a provider directly. That also matches what insta
602602
does anyway: `set_query_planner` builds the planner against the codecs of the session
603603
that will run the query.
604604

605+
Duck-type `session`, and do not check its type. It is the PyO3 context the binding
606+
installs through, not the `datafusion.context.SessionContext` wrapper, so it carries
607+
every capsule getter and `__datafusion_codec_id__` — everything the protocol asks of it
608+
— but `isinstance(session, SessionContext)` is `False` in Python even though its `repr`
609+
reads `datafusion.SessionContext`. The two bundle hooks
610+
`__datafusion_session_extension__` and `__datafusion_session_planner__` are the
611+
exception: `with_extensions` dispatches them from Python and hands them the wrapper.
612+
605613
`SessionContext` accepts the argument on all three getters and ignores it, so a session
606614
satisfies the same protocol an extension library implements. When you export the current
607615
planner to wrap it, `ctx.__datafusion_query_planner__()` and

python/datafusion/context.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1915,7 +1915,11 @@ class as :py:meth:`with_logical_extension_codec` describes. Declare
19151915
``__datafusion_codec_id__`` on at least one of them; the
19161916
collision is refused rather than resolved by position, because
19171917
a positional id would break stored plans the first time the
1918-
extension reordered what it returns.
1918+
extension reordered what it returns. Also if a capsule getter
1919+
returns a capsule of the wrong kind — a physical codec handed
1920+
over under ``__datafusion_logical_extension_codec__``, say —
1921+
which is reported against the name the getter should have
1922+
produced.
19191923
19201924
Examples:
19211925
The example is skipped here because it needs a built FFI

python/datafusion/extensions.py

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,11 @@
3737
session's codec chain holds many codecs and dispatches between them by id, so
3838
codecs merely accumulate and their order does not affect decoding. A session
3939
holds exactly *one* query planner, so planners compose by nesting: each wraps
40-
the one before it. Phase one collects every bundle's codecs through
40+
the one before it. Phase one collects the codecs of every bundle implementing
4141
:py:class:`SessionExtensionExportable` and installs them; phase two runs
42-
:py:class:`SessionPlannerExportable` once per bundle, in argument order,
43-
handing each the planner built so far.
42+
:py:class:`SessionPlannerExportable` once for each bundle that implements it,
43+
in argument order, handing each the planner built so far. A bundle implements
44+
either hook or both, and one it does not implement is simply not called.
4445
4546
That split is what lets several libraries that each ship a planner coexist. It
4647
also means bundle order is significant for planners and irrelevant for codecs.
@@ -72,10 +73,20 @@ class QueryPlannerExportable(Protocol):
7273
"""Type hint for object that has a __datafusion_query_planner__ PyCapsule.
7374
7475
The method returns a PyCapsule wrapping an ``FFI_QueryPlanner``, typically
75-
produced by a separate compiled extension. ``session`` is the
76-
:py:class:`~datafusion.context.SessionContext` the planner is being
77-
installed on; take the extension codecs from it rather than building your
78-
own.
76+
produced by a separate compiled extension. ``session`` is a handle on the
77+
session the planner is being installed on; take the extension codecs from
78+
it rather than building your own.
79+
80+
Duck-type that handle rather than checking its type. It is the PyO3
81+
context from ``datafusion._internal``, not the
82+
:py:class:`~datafusion.context.SessionContext` wrapper, so it exposes every
83+
capsule getter and ``__datafusion_codec_id__`` — which is all the protocol
84+
asks of it — but ``isinstance(session, SessionContext)`` is ``False`` even
85+
though its ``repr`` reads ``datafusion.SessionContext``. The same is true
86+
of the codec getters in :py:mod:`datafusion.user_defined`. The two bundle
87+
hooks are the exception: :py:class:`SessionExtensionExportable` and
88+
:py:class:`SessionPlannerExportable` are dispatched from Python and receive
89+
the wrapper.
7990
"""
8091

8192
def __datafusion_query_planner__(self, session: Any) -> object: ... # noqa: D105

0 commit comments

Comments
 (0)