Skip to content

Commit 5a1bfeb

Browse files
timsaucerclaude
andcommitted
docs: fix extension-guide review findings, rename phase-one bundle hook
Six findings from a read-through of the new extension guide, plus the API rename one of them turned into. The "process local tokens" note sat in the guide index with nothing around it to explain what a token was or why the reader should care. It moves to a new `extension_codec_durable_metadata` section in `codecs.md`, where the reader is already thinking about what goes in a payload: what to encode, then what the examples do instead, and the three consequences that follow from parking live objects in a process-local map — no double decode, no fan-out, and a leak for any plan that never reaches a decoder. The checklist item now points there instead of at the guide index. The hook reference loses its `Capsule name` column, which restated `datafusion_<thing>` for every row when the naming rule already derives it, and gains a `Contributes` column instead. The argument column stays: those four values are protocol, not a signature, and only 4 of the 18 hooks have a Python definition to link at all — the rest are host-side imports, so links into Rust source would rot faster than the table. Staleness is handled by `test_hook_reference_table_lists_every_hook` instead, which greps `crates/` and `python/datafusion/` for `__datafusion_*__` and diffs the set against the table rows. Verified it fails when a row is dropped. `capsule-protocol.md` described `abi_stable`, which datafusion-ffi no longer uses. It now describes `stabby` and the part that is not stabby: `FFI_Option` and `FFI_Result` are datafusion-ffi's own, because stabby's require `T: IStable` and the `FFI_*` structs hold self-referential function pointers. The conversion example converts to `Arc<dyn TableProvider>` rather than naming `ForeignTableProvider`, since the `From` impl compares library markers and returns the original `Arc` when both sides are the same library. Three other snippets on that page had gone stale with it: `FFI_TableProvider::new` with three arguments, `PyCapsule::new_bound`, and a receiving snippet whose variable was named `codec`. In `table-providers.md` all five `Registered with` cells now render `Receiver.method`, so the schema row reads `Catalog.register_schema` rather than a bare dotted path, with one sentence on reaching a `Catalog` first. `Other session components` was in `functions.md`, where an optimizer rule and a config struct are neither functions nor tables; it becomes its own page, `other-components.md`, carrying the `extension_other_hooks` label so the index rows still resolve. Three guide pages named individual tests, which invites exactly the divergence the reference is supposed to prevent. They name the suite now. Finally the phase-one bundle hook. `__datafusion_session_extension__` reused the name of the whole thing it is a hook on — `with_extensions` takes extensions and `SessionExtensionExportable` is the bundle protocol — while its sibling `__datafusion_session_planner__` is named for its content. It is now `__datafusion_session_components__`, matching both its sibling and the `SessionExtensionComponents` it returns, which leaves room for the UDF and provider fields that will join the codec fields later. The protocol class follows it to `SessionComponentsExportable`, since that file's convention is one class per hook name. Neither name has shipped, so the upgrade guide needs no before-and-after; it introduces both hooks as new in this release and names the new one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 61ca5ff commit 5a1bfeb

24 files changed

Lines changed: 268 additions & 165 deletions

File tree

.ai/skills/ffi-capsule-protocol/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ Wrap `fallback` and delegate to it; returning a planner that ignores it discards
7070
every layer beneath, including one the session already had. It runs after every
7171
bundle's codecs are installed, so `ctx` carries the final chains.
7272

73-
That is also the only hook where it does. `__datafusion_session_extension__`
73+
That is also the only hook where it does. `__datafusion_session_components__`
7474
runs before anything is installed, so its `ctx` still carries the chains the
7575
receiver had — the same session, and the same task-context provider, but not
7676
this call's codecs, not even your own. Read the host's codec chains in the

docs/source/conf.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ def autoapi_skip_member_fn(app, what, name, obj, skip, options) -> bool: # noqa
123123
("class", "datafusion.SessionContext"),
124124
("class", "datafusion.QueryPlannerExportable"),
125125
("class", "datafusion.SessionExtensionComponents"),
126-
("class", "datafusion.SessionExtensionExportable"),
126+
("class", "datafusion.SessionComponentsExportable"),
127127
("class", "datafusion.SessionPlannerExportable"),
128128
("module", "datafusion.common"),
129129
# Duplicate modules (skip module-level docs to avoid duplication)

docs/source/extension-guide/bundles.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ bundle object implementing one or both of two hooks:
4040

4141
```python
4242
class MyEngineExtension:
43-
def __datafusion_session_extension__(self, ctx: SessionContext) -> SessionExtensionComponents:
43+
def __datafusion_session_components__(self, ctx: SessionContext) -> SessionExtensionComponents:
4444
# Phase one. Create fresh components bound to `ctx` on every call.
4545
return SessionExtensionComponents(
4646
logical_extension_codecs=(self._make_logical_codec(ctx),),
@@ -95,7 +95,7 @@ class Bundle:
9595
def __init__(self, codec_id):
9696
self.codec_id = codec_id
9797

98-
def __datafusion_session_extension__(self, ctx):
98+
def __datafusion_session_components__(self, ctx):
9999
# Fresh components on every call, bound to the `ctx` handed in.
100100
# Never cache these, and never retain `ctx`.
101101
return SessionExtensionComponents(
@@ -133,7 +133,7 @@ routes to the codec that wrote the payload. A session holds exactly **one**
133133
query planner, so planners cannot accumulate — they compose by *nesting*, each
134134
wrapping the one before it and delegating to it for work it does not handle.
135135

136-
So `with_extensions` runs every `__datafusion_session_extension__` and installs
136+
So `with_extensions` runs every `__datafusion_session_components__` and installs
137137
all the codecs, and only then runs each `__datafusion_session_planner__`, in
138138
argument order, handing each the planner built so far. Two consequences worth
139139
holding onto:
@@ -204,8 +204,8 @@ class CodecsOf:
204204
def __init__(self, inner):
205205
self.inner = inner
206206

207-
def __datafusion_session_extension__(self, ctx):
208-
return self.inner.__datafusion_session_extension__(ctx)
207+
def __datafusion_session_components__(self, ctx):
208+
return self.inner.__datafusion_session_components__(ctx)
209209

210210

211211
class PlannerOf:

docs/source/extension-guide/capsule-protocol.md

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,17 @@ review the code and documentation in the [datafusion-ffi] crate.
3535

3636
Our FFI implementation is narrowly focused on sharing data and functions with
3737
Rust backed libraries. This allows us to use the
38-
[abi_stable crate](https://crates.io/crates/abi_stable). This is an excellent
39-
crate that allows for easy conversion between Rust native types and FFI-safe
40-
alternatives. For example, if you needed to pass a `Vec<String>` via FFI, you
41-
can simply convert it to an `RVec<RString>` in an intuitive manner. It also
42-
supports features like `RResult` and `ROption` that do not have an obvious
43-
translation to a C equivalent.
38+
[stabby crate](https://crates.io/crates/stabby), which converts between Rust
39+
native types and FFI-safe alternatives. For example, if you needed to pass a
40+
`Vec<String>` via FFI, you can convert it to a
41+
`stabby::vec::Vec<stabby::string::String>` — the crate's own examples alias
42+
these as `SVec` and `SString`, which is the convention [datafusion-ffi] follows
43+
too.
44+
45+
For `Option` and `Result`, [datafusion-ffi] defines its own `FFI_Option<T>` and
46+
`FFI_Result<T>` rather than using stabby's. Stabby's versions require
47+
`T: IStable` for niche optimization, and many of the `FFI_*` structs hold
48+
self-referential function pointers that cannot implement it.
4449

4550
## `FFI_` on the provider, `Foreign` on the receiver
4651

@@ -51,17 +56,19 @@ defined a custom
5156
and you want to create a sharable FFI counterpart, you could write:
5257

5358
```rust
54-
let my_provider = MyTableProvider::default();
55-
let ffi_provider = FFI_TableProvider::new(Arc::new(my_provider), false, None);
59+
let my_provider = Arc::new(MyTableProvider::default());
60+
let ffi_provider = FFI_TableProvider::new_with_ffi_codec(my_provider, false, None, codec);
5661
```
5762

63+
where `codec` is the host's logical codec, read off the argument your getter
64+
was handed — see {ref}`extension_getter_argument`.
65+
5866
If you were interfacing with a library that provided the above
59-
`FFI_TableProvider` and you needed to turn it back into a `TableProvider`, you
60-
can turn it into a `ForeignTableProvider`, which implements the `TableProvider`
61-
trait:
67+
`FFI_TableProvider` and you needed a usable `TableProvider` back, you convert
68+
it into an `Arc<dyn TableProvider>`:
6269

6370
```rust
64-
let foreign_provider: ForeignTableProvider = ffi_provider.into();
71+
let provider: Arc<dyn TableProvider> = (&ffi_provider).into();
6572
```
6673

6774
If you review the code in [datafusion-ffi] you will find that each of the
@@ -74,6 +81,13 @@ example we're showing, this means the code that has written the underlying
7481
structures with the `Foreign` prefix are to be used by the receiver. In this
7582
case, it is the `datafusion-python` library.
7683

84+
Convert to the trait object rather than naming `ForeignTableProvider` yourself.
85+
The conversion compares the provider's library marker against the receiver's:
86+
when both sides turn out to be the same shared library it hands back the
87+
original `Arc` and skips the boundary entirely, and only otherwise wraps it in
88+
a `ForeignTableProvider`. Which one you get is an implementation detail, and
89+
both implement `TableProvider`.
90+
7791
## Wrapping it in a capsule
7892

7993
In order to share these FFI structures, we need to wrap them in some kind of
@@ -82,20 +96,20 @@ described in {ref}`extension_why_ffi`, we use `PyCapsule`. We can create a
8296
`PyCapsule` for our provider thusly:
8397

8498
```rust
85-
let name = CString::new("datafusion_table_provider")?;
86-
let my_capsule = PyCapsule::new_bound(py, provider, Some(name))?;
99+
PyCapsule::new_with_value(py, ffi_provider, cr"datafusion_table_provider")
87100
```
88101

89-
On the receiving side, turn this pycapsule object into the
90-
`FFI_TableProvider`, which can then be turned into a `ForeignTableProvider`;
91-
the associated code is:
102+
On the receiving side, read the `FFI_TableProvider` back out of the capsule and
103+
convert it, which is what `table_provider_from_pycapsule` in `crates/util` does:
92104

93105
```rust
94-
let capsule = capsule.cast::<PyCapsule>()?;
106+
validate_pycapsule(capsule, "datafusion_table_provider")?;
95107
let data: NonNull<FFI_TableProvider> = capsule
96-
.pointer_checked(Some(name))?
108+
.pointer_checked(Some(c"datafusion_table_provider"))?
97109
.cast();
98-
let codec = unsafe { data.as_ref() };
110+
let ffi_provider = unsafe { data.as_ref() };
111+
check_ffi_version("table provider", unsafe { (ffi_provider.version)() })?;
112+
let provider: Arc<dyn TableProvider> = ffi_provider.into();
99113
```
100114

101115
## The naming rule
@@ -112,7 +126,7 @@ must return a capsule named `datafusion_table_provider`. Return a capsule with
112126
the wrong name and the import fails with an error naming both the name found
113127
and the name expected, rather than reading the pointer as the wrong type.
114128

115-
The full list of hooks and their capsule names is in the
129+
The full list of hooks is in the
116130
{ref}`hook reference <extension_guide>`. `TableProvider` was the first
117131
extension written this way and is the most thoroughly implemented; every hook
118132
added since follows the same pattern.
@@ -196,7 +210,7 @@ getter and `__datafusion_codec_id__` — everything the protocol asks of it —
196210
`isinstance(session, SessionContext)` is `False` in Python even though its
197211
`repr` reads `datafusion.SessionContext`.
198212

199-
The two bundle hooks are the exception: `__datafusion_session_extension__` and
213+
The two bundle hooks are the exception: `__datafusion_session_components__` and
200214
`__datafusion_session_planner__` are dispatched from Python by
201215
{py:meth}`~datafusion.SessionContext.with_extensions`, so they receive the
202216
wrapper. See {doc}`bundles`.

docs/source/extension-guide/checklist.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ publish. Each links to the page that explains it.
9393
- [ ] **Your production codec serializes durable metadata**, not a
9494
process-local token. The examples in this repository use tokens to make
9595
ownership observable; that is a demonstration, not a pattern.
96-
→ {ref}`extension_guide`
96+
→ {ref}`extension_codec_durable_metadata`
9797
- [ ] **You have integration tests across a real FFI boundary.** The two
9898
example crates in this repository are the pattern: build the cdylib,
9999
install the wheel, then exercise it from Python.

docs/source/extension-guide/codecs.md

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,24 @@ A codec that also ships to hosts which dispatch differently may still want its
5050
own guard against foreign payloads. Keeping one is fine; it is simply not
5151
needed for the datafusion-python path.
5252

53+
(extension_codec_durable_metadata)=
54+
55+
## Encode metadata, not a handle to a live object
56+
57+
Your payload has to be enough to rebuild the object somewhere your process is
58+
not. Write the metadata a fresh instance can be constructed from — a path, a
59+
connection string, a schema, the options the object was created with.
60+
61+
The example codecs in this repository do not do this, and it is worth knowing
62+
before copying them. They keep a process-local `HashMap` of live providers and
63+
encode an integer token into it: encoding inserts, decoding removes. That makes
64+
Rust type identity observable across three separately loaded libraries in one
65+
test, which is what the examples exist to show. It also means a decode consumes
66+
its token, so the same bytes cannot be decoded twice, one encoded plan cannot
67+
fan out to several readers, and a plan that never reaches a decoder keeps its
68+
provider alive for the life of the process. A real codec has none of those
69+
properties because it does not park the object anywhere.
70+
5371
(extension_codec_ids)=
5472

5573
## Codec ids
@@ -133,8 +151,7 @@ after it. The query still succeeds. What changes is which library wrote the
133151
bytes — so a plan that has to decode in another process now needs whichever
134152
library happened to win, not the one whose node it is.
135153
`MyPhysicalExtensionCodec` in [`datafusion-ffi-example`] claims this way, and
136-
`test_a_greedy_codec_installed_first_claims_another_librarys_node` pins the
137-
consequence.
154+
the query-planner example's test suite pins the consequence.
138155

139156
Two rules of thumb:
140157

docs/source/extension-guide/functions.md

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -103,51 +103,4 @@ on your decoder — with an empty payload there is no id to route on, so your
103103

104104
`NameOnlyUdfCodec` in [`datafusion-ffi-example`] is the worked case.
105105

106-
(extension_other_hooks)=
107-
108-
## Other session components
109-
110-
Two further hooks contribute things that are neither data nor functions. Both
111-
take no argument and both are implemented in [`datafusion-ffi-example`].
112-
113-
**`__datafusion_physical_optimizer_rule__`** contributes a rule that rewrites
114-
physical plans, installed with
115-
{py:meth}`~datafusion.SessionContext.add_physical_optimizer_rule`. Reach for
116-
this rather than a {doc}`query planner <query-planners>` when you want to
117-
adjust the plan DataFusion produced rather than produce it yourself — it is
118-
much the smaller commitment, and rules accumulate where planners nest.
119-
120-
```rust
121-
fn __datafusion_physical_optimizer_rule__<'py>(
122-
&self,
123-
py: Python<'py>,
124-
) -> PyResult<Bound<'py, PyCapsule>> {
125-
let rule: Arc<dyn PhysicalOptimizerRule + Send + Sync> = Arc::new(self.clone());
126-
let runtime = get_tokio_runtime().handle().clone();
127-
let ffi = FFI_PhysicalOptimizerRule::new(rule, Some(runtime));
128-
129-
PyCapsule::new_with_value(py, ffi, cr"datafusion_physical_optimizer_rule")
130-
}
131-
```
132-
133-
**`__datafusion_extension_options__`** contributes typed configuration entries
134-
that your components can read back out of the session config, installed with
135-
{py:meth}`SessionConfig.with_extension <datafusion.SessionConfig.with_extension>`.
136-
`FFI_ExtensionOptions` carries no version field, so it is one of the three
137-
components that cannot be version-checked on import.
138-
139-
```rust
140-
fn __datafusion_extension_options__<'py>(
141-
&self,
142-
py: Python<'py>,
143-
) -> PyResult<Bound<'py, PyCapsule>> {
144-
let mut config = FFI_ExtensionOptions::default();
145-
config
146-
.add_config(self)
147-
.map_err(|e| PyRuntimeError::new_err(e.to_string()))?;
148-
149-
PyCapsule::new_with_value(py, config, cr"datafusion_extension_options")
150-
}
151-
```
152-
153106
[`datafusion-ffi-example`]: https://github.com/apache/datafusion-python/tree/main/examples/datafusion-ffi-example

docs/source/extension-guide/index.md

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,6 @@ The session owns the codecs used for the exchange and supplies them to the
6969
foreign planner. That is what lets the planner decode provider-owned objects,
7070
and lets datafusion-python decode the physical plan the planner returns.
7171

72-
:::{note}
73-
The example codecs use process-local tokens to demonstrate ownership.
74-
A production codec should serialize durable metadata instead.
75-
:::
76-
7772
## Hook reference
7873

7974
Every integration point is a dunder method named `__datafusion_*__`. The
@@ -83,36 +78,45 @@ FFI-safe struct. See {doc}`capsule-protocol` for what that means and
8378
{ref}`extension_getter_argument` for the argument every getter in the middle
8479
group receives.
8580

86-
| Hook | Capsule name | Argument | Documented on |
81+
Each hook's capsule name follows from its own name by
82+
{ref}`the naming rule <extension_capsule_protocol>`, so it is not repeated
83+
here.
84+
85+
| Hook | Contributes | Argument | Documented on |
8786
| --- | --- | --- | --- |
88-
| `__datafusion_table_provider__` | `datafusion_table_provider` | codec source | {doc}`table-providers` |
89-
| `__datafusion_table_provider_factory__` | `datafusion_table_provider_factory` | codec source | {doc}`table-providers` |
90-
| `__datafusion_catalog_provider__` | `datafusion_catalog_provider` | codec source | {doc}`table-providers` |
91-
| `__datafusion_catalog_provider_list__` | `datafusion_catalog_provider_list` | codec source | {doc}`table-providers` |
92-
| `__datafusion_schema_provider__` | `datafusion_schema_provider` | codec source | {doc}`table-providers` |
93-
| `__datafusion_table_function__` | `datafusion_table_function` | session | {doc}`functions` |
94-
| `__datafusion_scalar_udf__` | `datafusion_scalar_udf` | none | {doc}`functions` |
95-
| `__datafusion_aggregate_udf__` | `datafusion_aggregate_udf` | none | {doc}`functions` |
96-
| `__datafusion_window_udf__` | `datafusion_window_udf` | none | {doc}`functions` |
97-
| `__datafusion_logical_extension_codec__` | `datafusion_logical_extension_codec` | session | {doc}`codecs` |
98-
| `__datafusion_physical_extension_codec__` | `datafusion_physical_extension_codec` | session | {doc}`codecs` |
99-
| `__datafusion_codec_id__` | *not a capsule — a string attribute* || {doc}`codecs` |
100-
| `__datafusion_query_planner__` | `datafusion_query_planner` | session | {doc}`query-planners` |
101-
| `__datafusion_session_extension__` | *not a capsule — returns components* | `ctx` | {doc}`bundles` |
102-
| `__datafusion_session_planner__` | `datafusion_query_planner`, or `None` | `ctx`, `fallback` | {doc}`bundles` |
103-
| `__datafusion_physical_optimizer_rule__` | `datafusion_physical_optimizer_rule` | none | {ref}`extension_other_hooks` |
104-
| `__datafusion_extension_options__` | `datafusion_extension_options` | none | {ref}`extension_other_hooks` |
105-
| `__datafusion_task_context_provider__` | `datafusion_task_context_provider` | none | {ref}`extension_task_context_provider` |
106-
107-
Two rows are not like the others. `__datafusion_task_context_provider__` is
87+
| `__datafusion_table_provider__` | one table | codec source | {doc}`table-providers` |
88+
| `__datafusion_table_provider_factory__` | tables built by `CREATE EXTERNAL TABLE` | codec source | {doc}`table-providers` |
89+
| `__datafusion_catalog_provider__` | a named set of schemas | codec source | {doc}`table-providers` |
90+
| `__datafusion_catalog_provider_list__` | the whole catalog namespace | codec source | {doc}`table-providers` |
91+
| `__datafusion_schema_provider__` | a named set of tables | codec source | {doc}`table-providers` |
92+
| `__datafusion_table_function__` | a table-valued function | session | {doc}`functions` |
93+
| `__datafusion_scalar_udf__` | a scalar function | none | {doc}`functions` |
94+
| `__datafusion_aggregate_udf__` | an aggregate function | none | {doc}`functions` |
95+
| `__datafusion_window_udf__` | a window function | none | {doc}`functions` |
96+
| `__datafusion_logical_extension_codec__` | a logical codec | session | {doc}`codecs` |
97+
| `__datafusion_physical_extension_codec__` | a physical codec | session | {doc}`codecs` |
98+
| `__datafusion_codec_id__` | the wire id a codec's payloads carry || {ref}`extension_codec_ids` |
99+
| `__datafusion_query_planner__` | a query planner | session | {doc}`query-planners` |
100+
| `__datafusion_session_components__` | a bundle's codecs | `ctx` | {doc}`bundles` |
101+
| `__datafusion_session_planner__` | a bundle's planner, wrapping `fallback` | `ctx`, `fallback` | {doc}`bundles` |
102+
| `__datafusion_physical_optimizer_rule__` | a physical optimizer rule | none | {ref}`extension_other_hooks` |
103+
| `__datafusion_extension_options__` | typed entries in the session config | none | {ref}`extension_other_hooks` |
104+
| `__datafusion_task_context_provider__` | the host's task context | none | {ref}`extension_task_context_provider` |
105+
106+
Three rows are not like the others. `__datafusion_task_context_provider__` is
108107
implemented by the **host**, not by your library — you read it off the session
109-
you are handed. And `__datafusion_codec_id__` is a plain string attribute
110-
rather than a method returning a capsule.
108+
you are handed. `__datafusion_codec_id__` is a plain string attribute rather
109+
than a method returning a capsule. And the two `session_` hooks are dispatched
110+
from Python and return objects rather than capsules.
111111

112112
"codec source" in the argument column means the value is something you can
113113
read the host's logical extension codec off, which is not always a session.
114114
{ref}`extension_getter_argument` explains why, and what to do with it.
115115

116+
`python/tests/test_docstrings.py` compares this table against the hook names
117+
the package actually dispatches, so a hook added or renamed without touching
118+
this page fails the suite.
119+
116120
```{toctree}
117121
:maxdepth: 2
118122
@@ -123,6 +127,7 @@ functions
123127
codecs
124128
bundles
125129
query-planners
130+
other-components
126131
sessions
127132
checklist
128133
```

0 commit comments

Comments
 (0)