Skip to content

Commit 94cb55f

Browse files
timsaucerclaude
andcommitted
chore: harden the extension API before it ships
`SessionContext.with_extensions` and `SessionExtensionComponents` landed in #1679 and have not shipped in a release yet. The bundle stack (#1738-#1741) reshapes them substantially, and a release would freeze three surfaces in their current form. `PhysicalOptimizerRuleExportable` was defined in `datafusion.context` and not exported from the package root, so `datafusion.context` would become its canonical import path. Move it to `datafusion.extensions` beside the rest of the `*Exportable` family, re-export it from `datafusion.context` so the old path keeps working, and export it from the package root. The move brings it under `test_extension_api_has_a_doctest`, which drives off `extensions.__all__`, so it gains the example it was missing. `SessionExtensionComponents` was positionally constructible with two fields. The stack takes it to nine, three of them pair-shaped. Make construction keyword-only so every later field addition is additive; no call site in the repository constructed it positionally. This is a new convention rather than a backport, so it has to be applied forward to the stack as well. The ordering that makes `with_extensions` transactional was stated in three docstrings with no canonical home to point at. Record it under `ffi_internals_commit_order` in the contributor guide, and label the existing "Failure and rollback" section `extension_bundles_transaction`, matching the names the stack links to. No released behaviour changes, so no `api change` label and no upgrade-guide entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 516d20d commit 94cb55f

5 files changed

Lines changed: 105 additions & 17 deletions

File tree

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,46 @@ library would serialize, and would do it with the codecs it was imported with.
111111
The extension-facing consequence — install codecs before a layered planner, and
112112
prefer `with_extensions` — is documented at {ref}`planner_codec_rebinding`.
113113

114+
(ffi_internals_commit_order)=
115+
116+
## Why `with_extensions` commits last
117+
118+
`with_extensions` promises that a bundle which raises leaves the session as it
119+
was. Keeping that promise is an ordering constraint on the implementation, not
120+
a property of any one step, because the planner is bound on the shared
121+
`SessionState` rather than on the returned handle.
122+
123+
A call therefore splits into a part that may fail and a part that may not:
124+
125+
1. **Collect.** Every `__datafusion_session_components__` runs and its codecs
126+
are gathered. Nothing is installed yet, so a hook that raises here has
127+
touched nothing.
128+
2. **Chains.** The codecs are assembled into the returned handle. Codec chains
129+
live on that handle rather than on the session, so this step writes nothing
130+
to the session even though it can fail on a bad capsule or a duplicate id.
131+
3. **Resolve.** Every `__datafusion_session_planner__` runs, in argument order,
132+
against the completed chains, and each supplied planner is exported to a
133+
capsule. Everything that can raise has raised by the end of this step.
134+
4. **Commit.** The accumulated planner is bound, in a single `SessionState`
135+
rebuild. The bind is skipped entirely when the call installed nothing, so an
136+
empty call does not drag a planner sitting on another handle's codecs onto
137+
this one's.
138+
139+
Only step 4 touches the session. This is a rule for the next field added to
140+
`SessionExtensionComponents`, not only a description of the current code: a new
141+
kind of component must do its fallible work — importing a capsule, resolving a
142+
name — in step 3, so that step 4 cannot raise part-way through.
143+
144+
There is nothing to roll back to if it does. The returned handle shares one
145+
session with the receiver, so the damage is visible from every other handle;
146+
and undoing a registration is not the same as restoring what it displaced,
147+
because deregistering a function that shadowed a built-in removes the built-in
148+
too. The split is cheaper than an undo log that cannot be written correctly.
149+
150+
The extension-facing statement of this is
151+
{ref}`extension_bundles_transaction`, which says only that declaring a
152+
component is safe where registering one during the hook is not.
153+
114154
## Two argument kinds for one convention
115155

116156
`CapsuleGetterArg` in `crates/util/src/lib.rs` distinguishes three cases: no

‎docs/source/extension-guide/bundles.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,8 @@ for direct ones. The wrapper travels with the codec; the bundle does not.
281281
The query planner is exempt — it carries no wire id, so it may be an object or
282282
a capsule.
283283

284+
(extension_bundles_transaction)=
285+
284286
## Failure and rollback
285287

286288
Nothing is written to the session until every factory has returned and every
@@ -290,6 +292,12 @@ table, say — is **not** rolled back, which is why bundle objects must be
290292
configuration-only: create fresh components on each call, never cache bound
291293
components, and do not retain the context passed in.
292294

295+
Declaring a component is what buys you that guarantee. Anything you return from
296+
your hook is validated while a failure still costs nothing, and is written only
297+
after every bundle in the call has succeeded. Anything you register yourself is
298+
written immediately, before the other bundles have even run. The ordering that
299+
makes this hold is recorded at {ref}`ffi_internals_commit_order`.
300+
293301
Like every other derivation, the returned context is a handle on the *same*
294302
session as the receiver — see {ref}`extension_sessions`. Only the Python-side
295303
codec chains belong to the returned handle; the planner is installed on the

‎python/datafusion/__init__.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@
9393
from .dataframe_formatter import configure_formatter
9494
from .expr import Expr, WindowFrame
9595
from .extensions import (
96+
PhysicalOptimizerRuleExportable,
9697
QueryPlannerExportable,
9798
SessionComponentsExportable,
9899
SessionExtensionComponents,
@@ -139,6 +140,7 @@
139140
"MetricsSet",
140141
"ParquetColumnOptions",
141142
"ParquetWriterOptions",
143+
"PhysicalOptimizerRuleExportable",
142144
"PhysicalPartitioning",
143145
"QueryPlannerExportable",
144146
"RecordBatch",

‎python/datafusion/context.py‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
from datafusion.dataframe import DataFrame
7171
from datafusion.expr import sort_list_to_raw_sort_list
7272
from datafusion.extensions import (
73+
PhysicalOptimizerRuleExportable,
7374
QueryPlannerExportable,
7475
SessionComponentsExportable,
7576
SessionExtensionComponents,
@@ -151,16 +152,6 @@ class TableProviderExportable(Protocol):
151152
def __datafusion_table_provider__(self, session: Any) -> object: ... # noqa: D105
152153

153154

154-
class PhysicalOptimizerRuleExportable(Protocol):
155-
"""Type hint for object that has __datafusion_physical_optimizer_rule__ PyCapsule.
156-
157-
The method returns a PyCapsule wrapping an ``FFI_PhysicalOptimizerRule``,
158-
typically produced by a separate compiled extension.
159-
"""
160-
161-
def __datafusion_physical_optimizer_rule__(self) -> object: ... # noqa: D105
162-
163-
164155
class SessionConfig:
165156
"""Session configuration options."""
166157

@@ -1918,7 +1909,8 @@ def with_extensions(
19181909
every capsule has been validated, so a hook that raises leaves the
19191910
session as it was. A hook that *mutates* the context it is handed —
19201911
registering a table, say — is not rolled back, which is why bundle
1921-
objects must be configuration-only.
1912+
objects must be configuration-only. See
1913+
:ref:`extension_bundles_transaction`.
19221914
19231915
Shares its session with this context — see :py:class:`SessionContext`.
19241916
@@ -2030,6 +2022,11 @@ def with_extensions(
20302022
continue
20312023
planner = new.ctx._export_query_planner(supplied)
20322024

2025+
# The commit step. Everything above is allowed to raise; this is not.
2026+
# See docs/source/contributor-guide/ffi-internals.md, "Why
2027+
# `with_extensions` commits last", for what a new component kind has to
2028+
# do to keep that true.
2029+
#
20332030
# Rebinding the session's planner is a side effect on state shared with
20342031
# every other handle, so do not pay it for a call that installs nothing
20352032
# -- the same guard `with_python_udf_inlining` carries. With no codec

‎python/datafusion/extensions.py‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,12 @@
3232
implements either hook or both. Bundle order is significant for planners, which
3333
nest, and irrelevant for codecs, which accumulate.
3434
35-
Of the four names here, only the two bundle hooks are ``@runtime_checkable``,
35+
Of the five names here, only the two bundle hooks are ``@runtime_checkable``,
3636
because :py:meth:`~datafusion.context.SessionContext.with_extensions`
37-
dispatches on them from Python. :py:class:`QueryPlannerExportable` is a type
38-
hint only, matching the other capsule-getter protocols in
39-
:py:mod:`datafusion.user_defined` and :py:mod:`datafusion.catalog`.
37+
dispatches on them from Python. :py:class:`QueryPlannerExportable` and
38+
:py:class:`PhysicalOptimizerRuleExportable` are type hints only, matching the
39+
other capsule-getter protocols in :py:mod:`datafusion.user_defined` and
40+
:py:mod:`datafusion.catalog`.
4041
4142
See :ref:`extension_bundles` in the online documentation for why the phases are
4243
split and for a worked implementation.
@@ -57,13 +58,49 @@
5758
)
5859

5960
__all__ = [
61+
"PhysicalOptimizerRuleExportable",
6062
"QueryPlannerExportable",
6163
"SessionComponentsExportable",
6264
"SessionExtensionComponents",
6365
"SessionPlannerExportable",
6466
]
6567

6668

69+
class PhysicalOptimizerRuleExportable(Protocol):
70+
"""Type hint for object that has __datafusion_physical_optimizer_rule__ PyCapsule.
71+
72+
The method returns a PyCapsule wrapping an ``FFI_PhysicalOptimizerRule``,
73+
typically produced by a separate compiled extension. It takes **no
74+
argument**: a rule needs neither a codec nor a task-context provider, so
75+
there is nothing session-scoped to hand it.
76+
77+
Rules accumulate rather than replace. Install one with
78+
:py:meth:`~datafusion.context.SessionContext.add_physical_optimizer_rule`
79+
— see :ref:`extension_other_hooks`.
80+
81+
Examples:
82+
The getter is the whole protocol, and a capsule is what it must return
83+
— anything else is refused where it is installed rather than at plan
84+
time:
85+
86+
>>> from datafusion import SessionContext
87+
>>> ctx = SessionContext()
88+
>>> ctx.add_physical_optimizer_rule(object())
89+
Traceback (most recent call last):
90+
...
91+
RuntimeError: "Invalid datafusion_physical_optimizer_rule...
92+
93+
Real usage. Skipped here (needs a built extension library); run for
94+
real by ``test_ffi_physical_optimizer_rule_runs_during_planning`` in
95+
``datafusion-ffi-example``.
96+
97+
>>> from datafusion_ffi_example import MyPhysicalOptimizerRule # doctest: +SKIP
98+
>>> ctx.add_physical_optimizer_rule(MyPhysicalOptimizerRule()) # doctest: +SKIP
99+
"""
100+
101+
def __datafusion_physical_optimizer_rule__(self) -> object: ... # noqa: D105
102+
103+
67104
class QueryPlannerExportable(Protocol):
68105
"""Type hint for object that has a __datafusion_query_planner__ PyCapsule.
69106
@@ -110,7 +147,7 @@ def _not_a_codec_iterable(field: str, value: object) -> str:
110147
)
111148

112149

113-
@dataclass(frozen=True)
150+
@dataclass(frozen=True, kw_only=True)
114151
class SessionExtensionComponents:
115152
"""Components an extension contributes to a session context.
116153
@@ -121,6 +158,9 @@ class SessionExtensionComponents:
121158
components bound to a different session hold a task-context provider for
122159
that other session and cannot be rebound.
123160
161+
Construction is keyword-only, so later releases can add component kinds
162+
without changing what an existing call means.
163+
124164
Query planners are not listed here. They install in a second phase so each
125165
can wrap the one before it — see :py:class:`SessionPlannerExportable`.
126166
@@ -233,7 +273,8 @@ class SessionComponentsExportable(Protocol):
233273
retain that context or cache the components they bound to it, since the
234274
next call may install onto a different session. They should also avoid
235275
mutating the context they are handed — a registration made during binding
236-
is not rolled back if a later extension fails.
276+
is not rolled back if a later extension fails. See
277+
:ref:`extension_bundles_transaction`.
237278
238279
A bundle that also contributes a query planner implements
239280
:py:class:`SessionPlannerExportable` alongside this protocol.

0 commit comments

Comments
 (0)