Skip to content

Commit 90f8709

Browse files
timsaucerclaude
andauthored
chore: harden the extension API before it ships (#1759)
* 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> * refactor: narrow the extension protocol surface The capsule-getter protocols are annotations, never arguments: a bundle author constructs a SessionExtensionComponents but only ever names SessionComponentsExportable in a type hint. Exporting the hints from the package root made this one family the exception among sixteen such protocols, every other one of which is reached through its defining module. Drop PhysicalOptimizerRuleExportable, QueryPlannerExportable, SessionComponentsExportable, and SessionPlannerExportable from the root (__all__ 58 -> 54), keeping SessionExtensionComponents, which is the one name a bundle constructs. The three bundle protocols are new in 55.0.0, so no import path is lost. PhysicalOptimizerRuleExportable shipped in 54.0.0 from datafusion.context, so its move is a break: context.py now imports it under TYPE_CHECKING only, and the upgrade guide records the new path. Nothing else changes for a rule author -- the protocol is structural and not runtime-checkable, and add_physical_optimizer_rule is untouched. Also removes three now-dead autoapi skip entries, repoints three doctests that imported from the root, and fixes the add_physical_optimizer_rule cross-reference, which stopped resolving once the class left context.py. test_extension_protocols_are_exported_together asserted the premise this reverses, so it goes. The five doctests in extensions.py already prove the classes exist there, and SessionExtensionComponents' own docstring pins the remaining root export. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: state the commit-order invariant correctly The ordering rule said step 4 "cannot raise part-way through", and the comment in `with_extensions` said "everything above is allowed to raise; this is not". Neither holds: `_install_extension_planner` runs `ffi_query_planner_from_pycapsule` before it calls `set_session_query_planner`, so the commit step has fallible work of its own. The guarantee survives, because that import happens before the write. But the passage is written as a rule for whoever adds the next component kind, and as phrased it asks them to preserve a property the code does not have. Restate it as what actually holds: every fallible operation, including the ones inside the commit, completes before the first write. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: verify the physical optimizer rule docstring example The `+SKIP` block in `PhysicalOptimizerRuleExportable` named `test_ffi_physical_optimizer_rule_runs_during_planning` as the test that runs it for real, but that test never reads the docstring. It is a separately written test that happens to call the same two APIs, so it catches a renamed method or module only by coincidence, and cannot see an edit to the docstring at all. Add the mirror the convention actually asks for, in the shape of `test_with_extensions_docstring_example_still_runs`: parse the live docstring, keep only the skipped statements, drop the skip, and run them. Only `ctx` is supplied, because the skipped statements go on using the context the runnable block above them opened. Verified by mutation. Renaming the imported class in the docstring fails with `NameError: name 'MyPhysicalOptimizerRule' is not defined`, and deleting the block fails the `assert examples` guard rather than passing vacuously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: keep bundle protocols out of datafusion.context datafusion.context imported QueryPlannerExportable, SessionComponentsExportable and SessionPlannerExportable at runtime, so they were reachable as datafusion.context.* despite datafusion.extensions being their one home. Move them under TYPE_CHECKING and route the runtime isinstance checks through a private _extensions module alias. The protocols are new in 55.0.0, so no released import path is dropped. Add a test pinning that all four capsule-getter protocols live only in datafusion.extensions, and list PhysicalOptimizerRuleExportable in llms.txt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: pin keyword-only construction; scope the rollback promise Add a test that SessionExtensionComponents rejects positional arguments, so dropping kw_only=True fails the suite. Every existing caller passes keywords and would stay green without it. Drop the absence assertions from the extension protocol import test. Re-exporting a name is additive and breaks no caller, so asserting a name is missing only adds friction for a later deliberate export. Keep the positive check that each protocol imports from datafusion.extensions. The contributor guide promised that a raising bundle leaves the session as it was without the carve-out for writes a hook makes to the context it is handed. Add it with a ref to the bundles guide, which is where the exception is explained. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 516d20d commit 90f8709

11 files changed

Lines changed: 246 additions & 65 deletions

File tree

‎docs/source/conf.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,7 @@ def autoapi_skip_member_fn(app, what, name, obj, skip, options) -> bool: # noqa
122122
# Re-exports
123123
("class", "datafusion.DataFrame"),
124124
("class", "datafusion.SessionContext"),
125-
("class", "datafusion.QueryPlannerExportable"),
126125
("class", "datafusion.SessionExtensionComponents"),
127-
("class", "datafusion.SessionComponentsExportable"),
128-
("class", "datafusion.SessionPlannerExportable"),
129126
("module", "datafusion.common"),
130127
# Duplicate modules (skip module-level docs to avoid duplication)
131128
("module", "datafusion.col"),

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,54 @@ 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, apart from anything a hook writes to the context it is handed
120+
({ref}`extension_bundles_transaction`). Keeping that promise is an ordering constraint on the implementation, not
121+
a property of any one step, because the planner is bound on the shared
122+
`SessionState` rather than on the returned handle.
123+
124+
A call therefore splits into four steps, of which only the last writes:
125+
126+
1. **Collect.** Every `__datafusion_session_components__` runs and its codecs
127+
are gathered. Nothing is installed yet, so a hook that raises here has
128+
touched nothing.
129+
2. **Chains.** The codecs are assembled into the returned handle. Codec chains
130+
live on that handle rather than on the session, so this step writes nothing
131+
to the session even though it can fail on a bad capsule or a duplicate id.
132+
3. **Resolve.** Every `__datafusion_session_planner__` runs, in argument order,
133+
against the completed chains, and each supplied planner is exported to a
134+
capsule. Every hook that can raise has run by the end of this step.
135+
4. **Commit.** The accumulated planner is re-imported from its capsule and
136+
bound, in a single `SessionState` rebuild. The bind is skipped entirely when
137+
the call installed nothing, so an empty call does not drag a planner sitting
138+
on another handle's codecs onto this one's.
139+
140+
Only step 4 touches the session, and it is not itself infallible:
141+
`_install_extension_planner` runs `ffi_query_planner_from_pycapsule` before it
142+
calls `set_session_query_planner`, which cannot fail. The property that keeps
143+
the promise is therefore about order, not about any step being incapable of
144+
raising — every fallible operation, including the ones inside the commit,
145+
completes before the first write.
146+
147+
That is the rule for the next field added to `SessionExtensionComponents`, not
148+
only a description of the current code: a new kind of component must do its
149+
fallible work — importing a capsule, resolving a name — before anything is
150+
written, so no failure can leave the session half-updated.
151+
152+
There would be nothing to roll back to if one did. The returned handle shares one
153+
session with the receiver, so the damage is visible from every other handle;
154+
and undoing a registration is not the same as restoring what it displaced,
155+
because deregistering a function that shadowed a built-in removes the built-in
156+
too. The split is cheaper than an undo log that cannot be written correctly.
157+
158+
The extension-facing statement of this is
159+
{ref}`extension_bundles_transaction`, which says only that declaring a
160+
component is safe where registering one during the hook is not.
161+
114162
## Two argument kinds for one convention
115163

116164
`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

‎docs/source/llms.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
- [`datafusion.expr`](https://datafusion.apache.org/python/autoapi/datafusion/expr/index.html): expression tree nodes (`Expr`, `Window`, `WindowFrame`, `GroupingSet`).
3232
- [`datafusion.functions`](https://datafusion.apache.org/python/autoapi/datafusion/functions/index.html): 290+ scalar, aggregate, and window functions.
3333
- [`datafusion.context.SessionContext`](https://datafusion.apache.org/python/autoapi/datafusion/context/index.html): session entry point, data loading, SQL execution.
34-
- [`datafusion.extensions`](https://datafusion.apache.org/python/autoapi/datafusion/extensions/index.html): `SessionExtensionComponents`, `SessionComponentsExportable`, `SessionPlannerExportable`, `QueryPlannerExportable` — the extension-bundle protocol.
34+
- [`datafusion.extensions`](https://datafusion.apache.org/python/autoapi/datafusion/extensions/index.html): `SessionExtensionComponents`, `SessionComponentsExportable`, `SessionPlannerExportable`, `QueryPlannerExportable`, `PhysicalOptimizerRuleExportable` — the extension-bundle and capsule-getter protocols.
3535
- [`datafusion.ipc`](https://datafusion.apache.org/python/autoapi/datafusion/ipc/index.html): worker and sender context slots for shipping expressions between processes.
3636

3737
## Examples

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,34 @@ installed produces the same bytes as before, as do functions encoded by name.
159159
Regenerate any plan you serialized with an earlier release and stored for later
160160
use, if it was produced by a session with an extension codec installed.
161161

162+
### Capsule-getter protocols moved to `datafusion.extensions`
163+
164+
`PhysicalOptimizerRuleExportable` now lives in `datafusion.extensions`, next to
165+
the other protocols an extension library implements against. It was previously
166+
importable from `datafusion.context`, and that path is gone.
167+
168+
```python
169+
from datafusion.context import PhysicalOptimizerRuleExportable # before
170+
from datafusion.extensions import PhysicalOptimizerRuleExportable # after
171+
```
172+
173+
This affects type annotations only. The protocol is structural and not
174+
`@runtime_checkable`, so nothing imports it to call `isinstance`, and
175+
`SessionContext.add_physical_optimizer_rule` is unchanged — a rule object that
176+
worked before still works, whether or not its library names the protocol
177+
anywhere.
178+
179+
The bundle protocols added in this release — `QueryPlannerExportable`,
180+
`SessionComponentsExportable`, and `SessionPlannerExportable` — are reached the
181+
same way, through `datafusion.extensions` rather than the package root. They are
182+
new in 55.0.0, so no earlier import path existed. `SessionExtensionComponents`
183+
stays at the root, because a bundle constructs one rather than merely naming it:
184+
185+
```python
186+
from datafusion import SessionExtensionComponents
187+
from datafusion.extensions import SessionComponentsExportable
188+
```
189+
162190
### `SessionContext.execute` renamed its second parameter
163191

164192
The parameter is a single partition index, not a count, and is now named

‎examples/datafusion-ffi-example/python/tests/_test_physical_optimizer_rule.py‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,13 @@
1717

1818
from __future__ import annotations
1919

20+
import doctest
21+
import inspect
22+
import io
23+
2024
import pyarrow as pa
2125
from datafusion import SessionContext
26+
from datafusion.extensions import PhysicalOptimizerRuleExportable
2227
from datafusion_ffi_example import MyPhysicalOptimizerRule
2328

2429

@@ -43,3 +48,42 @@ def test_ffi_physical_optimizer_rule_runs_during_planning():
4348
f"before={before} after={after}"
4449
)
4550
assert result[0].column(0).to_pylist() == [1, 2, 3]
51+
52+
53+
def test_physical_optimizer_rule_docstring_example_still_runs():
54+
"""Run the ``PhysicalOptimizerRuleExportable`` docstring example verbatim.
55+
56+
The example is marked ``+SKIP`` because the main suite has no built FFI
57+
extension to import, which is exactly how such an example rots. Here the
58+
statements are parsed out of the live docstring, the skip is dropped, and
59+
each one is executed.
60+
61+
Only the ``+SKIP`` statements are taken. The docstring also carries a
62+
runnable example above them, which the main suite already executes under
63+
``--doctest-modules``; running it again here would prove nothing.
64+
65+
Only ``ctx`` is supplied, because the skipped statements go on using the
66+
context the runnable block opened. Everything else resolves for real: a
67+
renamed class, a changed signature, or a wrong expected output in the
68+
docstring fails here.
69+
"""
70+
examples = [
71+
example
72+
for example in doctest.DocTestParser().get_examples(
73+
inspect.getdoc(PhysicalOptimizerRuleExportable)
74+
)
75+
if example.options.pop(doctest.SKIP, False)
76+
]
77+
assert examples, "docstring has no skipped examples to check"
78+
79+
test = doctest.DocTest(
80+
examples,
81+
{"ctx": SessionContext()},
82+
"PhysicalOptimizerRuleExportable",
83+
None,
84+
None,
85+
None,
86+
)
87+
output = io.StringIO()
88+
results = doctest.DocTestRunner().run(test, out=output.write)
89+
assert results.failed == 0, output.getvalue()

‎python/datafusion/__init__.py‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,7 @@
9292
)
9393
from .dataframe_formatter import configure_formatter
9494
from .expr import Expr, WindowFrame
95-
from .extensions import (
96-
QueryPlannerExportable,
97-
SessionComponentsExportable,
98-
SessionExtensionComponents,
99-
SessionPlannerExportable,
100-
)
95+
from .extensions import SessionExtensionComponents
10196
from .io import read_avro, read_csv, read_json, read_parquet
10297
from .options import CsvReadOptions
10398
from .plan import (
@@ -140,17 +135,14 @@
140135
"ParquetColumnOptions",
141136
"ParquetWriterOptions",
142137
"PhysicalPartitioning",
143-
"QueryPlannerExportable",
144138
"RecordBatch",
145139
"RecordBatchStream",
146140
"RuntimeEnvBuilder",
147141
"SQLOptions",
148142
"ScalarUDF",
149-
"SessionComponentsExportable",
150143
"SessionConfig",
151144
"SessionContext",
152145
"SessionExtensionComponents",
153-
"SessionPlannerExportable",
154146
"Table",
155147
"TableFunction",
156148
"TableProviderFactory",

‎python/datafusion/context.py‎

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858

5959
import pyarrow as pa
6060

61+
from datafusion import extensions as _extensions
6162
from datafusion.catalog import (
6263
Catalog,
6364
CatalogList,
@@ -69,12 +70,6 @@
6970
)
7071
from datafusion.dataframe import DataFrame
7172
from datafusion.expr import sort_list_to_raw_sort_list
72-
from datafusion.extensions import (
73-
QueryPlannerExportable,
74-
SessionComponentsExportable,
75-
SessionExtensionComponents,
76-
SessionPlannerExportable,
77-
)
7873
from datafusion.options import (
7974
DEFAULT_MAX_INFER_SCHEMA,
8075
CsvReadOptions,
@@ -99,6 +94,18 @@
9994
from datafusion.catalog import CatalogProvider, Table
10095
from datafusion.common import DFSchema
10196
from datafusion.expr import Expr, SortKey
97+
98+
# Type-only on purpose. `datafusion.extensions` is the one home for the
99+
# capsule-getter protocols; importing these at runtime would make them
100+
# reachable as `datafusion.context.*`, and for
101+
# `PhysicalOptimizerRuleExportable` would restore the 54.0.0 path that
102+
# 55.0.0 drops. Runtime checks go through the private `_extensions` alias.
103+
from datafusion.extensions import (
104+
PhysicalOptimizerRuleExportable,
105+
QueryPlannerExportable,
106+
SessionComponentsExportable,
107+
SessionPlannerExportable,
108+
)
102109
from datafusion.plan import ExecutionPlan, LogicalPlan
103110
from datafusion.user_defined import (
104111
AggregateUDF,
@@ -151,16 +158,6 @@ class TableProviderExportable(Protocol):
151158
def __datafusion_table_provider__(self, session: Any) -> object: ... # noqa: D105
152159

153160

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-
164161
class SessionConfig:
165162
"""Session configuration options."""
166163

@@ -1830,7 +1827,8 @@ def add_physical_optimizer_rule(
18301827
18311828
Args:
18321829
rule: Object exposing ``__datafusion_physical_optimizer_rule__``,
1833-
a :class:`PhysicalOptimizerRuleExportable`.
1830+
a
1831+
:py:class:`~datafusion.extensions.PhysicalOptimizerRuleExportable`.
18341832
18351833
Examples:
18361834
>>> from datafusion import SessionContext
@@ -1918,7 +1916,8 @@ def with_extensions(
19181916
every capsule has been validated, so a hook that raises leaves the
19191917
session as it was. A hook that *mutates* the context it is handed —
19201918
registering a table, say — is not rolled back, which is why bundle
1921-
objects must be configuration-only.
1919+
objects must be configuration-only. See
1920+
:ref:`extension_bundles_transaction`.
19221921
19231922
Shares its session with this context — see :py:class:`SessionContext`.
19241923
@@ -1977,7 +1976,11 @@ def with_extensions(
19771976
"""
19781977
for extension in extensions:
19791978
if not isinstance(
1980-
extension, (SessionComponentsExportable, SessionPlannerExportable)
1979+
extension,
1980+
(
1981+
_extensions.SessionComponentsExportable,
1982+
_extensions.SessionPlannerExportable,
1983+
),
19811984
):
19821985
msg = (
19831986
"Extension implements neither "
@@ -1993,10 +1996,10 @@ def with_extensions(
19931996
logical_codecs: list[LogicalExtensionCodecExportable] = []
19941997
physical_codecs: list[PhysicalExtensionCodecExportable] = []
19951998
for extension in extensions:
1996-
if not isinstance(extension, SessionComponentsExportable):
1999+
if not isinstance(extension, _extensions.SessionComponentsExportable):
19972000
continue
19982001
components = extension.__datafusion_session_components__(self)
1999-
if not isinstance(components, SessionExtensionComponents):
2002+
if not isinstance(components, _extensions.SessionExtensionComponents):
20002003
msg = (
20012004
"__datafusion_session_components__ must return "
20022005
"SessionExtensionComponents, got "
@@ -2018,7 +2021,7 @@ def with_extensions(
20182021
# rather than wrapping the session's default in an FFI hop.
20192022
planner: _PyCapsule | None = None
20202023
for extension in extensions:
2021-
if not isinstance(extension, SessionPlannerExportable):
2024+
if not isinstance(extension, _extensions.SessionPlannerExportable):
20222025
continue
20232026
fallback = (
20242027
planner
@@ -2030,6 +2033,14 @@ def with_extensions(
20302033
continue
20312034
planner = new.ctx._export_query_planner(supplied)
20322035

2036+
# The commit step, and the only one that writes to the session. It can
2037+
# still raise -- `_install_extension_planner` re-imports the capsule
2038+
# before binding it -- so what keeps the promise is the order, not any
2039+
# step being incapable of failing: every fallible operation finishes
2040+
# before the first write. See docs/source/contributor-guide/
2041+
# ffi-internals.md, "Why `with_extensions` commits last", for what a new
2042+
# component kind has to do to keep that true.
2043+
#
20332044
# Rebinding the session's planner is a side effect on state shared with
20342045
# every other handle, so do not pay it for a call that installs nothing
20352046
# -- the same guard `with_python_udf_inlining` carries. With no codec

0 commit comments

Comments
 (0)