Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
4facb0c
Add atomic SessionContext.with_extensions API
timsaucer Aug 7, 2026
a62e967
Add extension-bundle example and with_extensions FFI tests
timsaucer Aug 7, 2026
e8c0855
Document and test the context-outlives-DataFrame contract
timsaucer Aug 7, 2026
a4435bb
Skip private internal methods in wrapper coverage test
timsaucer Aug 7, 2026
25fee05
Test planner rebinding and codec ids in with_extensions
timsaucer Aug 7, 2026
56ca501
Fix duplicate attribute docs in SessionExtensionComponents
timsaucer Aug 8, 2026
8c9328a
Move session extension types into datafusion.extensions
timsaucer Aug 9, 2026
ceaf752
Run the with_extensions docstring example in CI
timsaucer Sep 4, 2026
bb26fc5
Share the session in with_extensions instead of forking it
timsaucer Sep 4, 2026
3529508
Name a bundle's bare capsules after the bundle
timsaucer Sep 4, 2026
d8f32cc
Stop claiming _install_extensions always writes state
timsaucer Sep 4, 2026
e06662a
Tidy the loose ends from review of with_extensions
timsaucer Sep 4, 2026
3924704
Require with_extensions codecs to be objects, not capsules
timsaucer Sep 4, 2026
31a4687
Install extension codecs and planners in two phases
timsaucer Sep 5, 2026
74ee466
Give the planner example a node only its own codec can carry
timsaucer Sep 5, 2026
54c8863
Drop the planner example's logical codec observer
timsaucer Sep 8, 2026
22d7c8b
Export QueryPlannerExportable and drop the empty-extensions guard
timsaucer Sep 8, 2026
3e7f13e
Describe both extension hooks in the upgrade guide
timsaucer Sep 8, 2026
734673a
Generate heading anchors for h4 so the FFI guide's links resolve
timsaucer Sep 8, 2026
f57feec
Skip the planner commit when with_extensions installs nothing
timsaucer Sep 8, 2026
7b6193e
Reject a lone codec where SessionExtensionComponents wants an iterable
timsaucer Sep 8, 2026
0868f39
Widen the with_extensions annotation to both hooks
timsaucer Sep 8, 2026
f7c448e
Correct three claims in the extension bundle docs
timsaucer Sep 8, 2026
0712eff
Rename _rebind_query_planner to _export_query_planner
timsaucer Sep 8, 2026
d13787b
Say which codec chains each extension hook's context carries
timsaucer Sep 8, 2026
07792ad
Make the planner-hook tests assert what their names claim
timsaucer Sep 8, 2026
4296c5e
Correct three more claims in the extension bundle docs
timsaucer Sep 8, 2026
781e980
docs: add a user-facing extensions page and split distributing-work
timsaucer Sep 9, 2026
7c86fb0
docs: split the FFI guide into one section per audience
timsaucer Sep 9, 2026
872afb6
docs: give each docstring claim one canonical home
timsaucer Sep 9, 2026
61ca5ff
docs: point the non-Sphinx consumers at the new pages
timsaucer Sep 9, 2026
5a1bfeb
docs: fix extension-guide review findings, rename phase-one bundle hook
timsaucer Sep 9, 2026
9372e1b
test: match the hook table against dispatch sites, not raw text
timsaucer Sep 9, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 45 additions & 2 deletions .ai/skills/ffi-capsule-protocol/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,33 @@ library reaches things only the session has.
session satisfies the protocol too — `ctx.__datafusion_query_planner__()` and
`ctx.__datafusion_query_planner__(ctx)` are both valid.

`__datafusion_session_planner__(ctx, fallback)` is the exception to the shape
above: it takes a second argument, the planner assembled so far. A session has
one planner slot, so planners compose by nesting rather than by chaining, and
the host hands each bundle the previous layer instead of letting it capture one.
Wrap `fallback` and delegate to it; returning a planner that ignores it discards
every layer beneath, including one the session already had. It runs after every
bundle's codecs are installed, so `ctx` carries the final chains.

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

A *codec* must always be handed over as an object implementing its getter, never
as the bare capsule the getter returns; `with_extensions` refuses a capsule.
A codec's wire id — the string a payload names on decode, which has to mean the
same thing in whichever process decodes — is read off the object it arrives as,
and a capsule has no type to read one from. Deriving the id from the bundle that
contributed the capsule is not the fix: the bundle is whatever object the caller
passed, so an application packaging your library inside a bundle of its own
would re-tag your payloads and they would stop decoding where they are read. If
the object's class name is not the identity you want on the wire, declare
`__datafusion_codec_id__` on it. `BundledLogicalCodec` in
`examples/datafusion-ffi-query-planner-example/src/extension.rs` is the shape.
This applies only to codecs — a query planner carries no wire id.

## Rule 3 — never construct a `SessionContext` in an extension library

The FFI constructors ask for things a library does not have:
Expand Down Expand Up @@ -154,8 +181,19 @@ guards this. Its `WHERE` clause is load-bearing: filter pushdown upgrades the
weak handle during logical optimization, before plan serialization could fail
first for an unrelated reason.

`SessionContext.with_extensions` is where this rule is easiest to get wrong,
because "bind the components to the context you are about to return" reads like
an instruction to derive one first. It is not: the factories are handed the
receiver, and the returned handle shares its allocation. There is nothing to
keep alive separately and nothing to garbage-collect out from under a provider.

`SessionContext.enable_url_table` is the one method that mints a second
allocation for a session. Its result must not outlive the receiver.
allocation for a session. Its result must not outlive the receiver, and it also
forks the session's `SessionState` while keeping its id, so two handles report
one `session_id()` with divergent configuration. That is a bug rather than a
design — tracked in
[apache/datafusion-python#1708](https://github.com/apache/datafusion-python/issues/1708)
— so do not cite it as precedent for deriving a replacement context.

## Rule 7 — installing a planner mutates the session, and says so

Expand Down Expand Up @@ -185,7 +223,12 @@ pins that; changing it should be deliberate.

## Where the truth is

- `docs/source/contributor-guide/ffi.md` — the protocol, the fork caveat.
- `docs/source/extension-guide/` — the protocol, for the library author.
`capsule-protocol.md` has the hook convention and what the getter argument
actually is; `codecs.md`, `bundles.md`, and `query-planners.md` have the
per-component rules; `index.md` lists all 18 hooks.
- `docs/source/contributor-guide/ffi-internals.md` — why the framing is shaped
this way, including the weak-`Arc` scheme and the one-level rebind.
- `docs/source/user-guide/upgrade-guides.md` — every past migration.
- `crates/core/src/codec.rs` — the codec chain: the envelope, identity dispatch,
and the two unframed cases from Rule 8.
Expand Down
41 changes: 41 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,47 @@ Every Python function must include a docstring with usage examples.
`array_sort`) only need a one-line description and a `See Also` reference to the
primary function. They do not need their own examples.

### One canonical home per claim

A claim lives where the reader already is when they need it — **exactly once**.

- A property of one callable's arguments, return value, or errors → that
callable's docstring.
- A property of a *type* that several callables share → the class docstring.
(Session sharing and context lifetime live on `SessionContext`, not on each
of the five `with_*` methods.)
- **Why** the API is shaped this way, a multi-library recipe, Rust-side code, a
trade-off, or a limitation with an upstream issue → the narrative guide under
`docs/source/`.

Everywhere else: one sentence plus one Sphinx role. A docstring may *state* a
claim the guide also makes; it must not *argue* it — no "because", no "the
reason is", no counter-argument.

**The test:** if a paragraph would survive being moved into the guide unchanged,
move it. `python/tests/test_docstrings.py` enforces a length ceiling, which is
the symptom this rule treats.

When you point at the guide, use a real `:ref:` to a specific label. Prose
saying "see the extensions guide" with no role is a dead end in the rendered
HTML.

### Examples that need a compiled extension

Some APIs cannot be demonstrated without a built FFI extension library, which
this package does not ship. The convention is:

1. A **runnable** example block first, using only the wheel. A `SessionContext`
satisfies the capsule-getter protocols, so its own exported capsule stands in
for a real library's in a doctest.
2. Then a `# doctest: +SKIP` block showing real usage, at most a few lines, with
one line of prose naming the test that runs it for real.
3. Every `+SKIP` block needs that mirror. See
`test_with_extensions_docstring_example_still_runs` in
`examples/datafusion-ffi-query-planner-example`, which parses the live
docstring, drops the skip, and executes it — so a renamed method or a wrong
expected output fails there.

## Aggregate and Window Function Documentation

When adding or updating an aggregate or window function, ensure the corresponding
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/core/src/codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ impl PythonLogicalCodec {
/// `cloudpickle.loads` on the inline `DFPY*` payload. It does
/// **not** make `pickle.loads(untrusted_bytes)` safe; treat every
/// `pickle.loads` on untrusted input as unsafe regardless of this
/// setting. See `docs/source/user-guide/io/distributing_work.rst`
/// setting. See `docs/source/user-guide/distributing-work/expressions.md`
/// (Security section) for the full threat model, and Python's
/// [pickle module security warning][1] for why `pickle.loads` is
/// unsafe in general.
Expand Down
180 changes: 171 additions & 9 deletions crates/core/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ use datafusion_python_util::{
};
use object_store::ObjectStore;
use pyo3::IntoPyObjectExt;
use pyo3::exceptions::{PyKeyError, PyRuntimeError, PyValueError};
use pyo3::exceptions::{PyKeyError, PyRuntimeError, PyTypeError, PyValueError};
use pyo3::prelude::*;
use pyo3::types::{PyCapsule, PyDict, PyList, PyTuple};
use url::Url;
Expand Down Expand Up @@ -424,10 +424,12 @@ impl PySessionContext {

pub fn enable_url_table(&self) -> PyResult<Self> {
// Pre-existing caveat, unrelated to query planners: this is the one
// method that mints a second `Arc<SessionContext>` for a session. Any
// weak `FFI_TaskContextProvider` handed out by the receiver stays bound
// to the receiver, so the returned context must not outlive it. See
// method that mints a second `Arc<SessionContext>` for a session, and
// it also forks the session's state while keeping its id. Any weak
// `FFI_TaskContextProvider` handed out by the receiver stays bound to
// the receiver, so the returned context must not outlive it. See
// `set_session_query_planner` for why everything else mutates in place.
// Tracked as a bug in <https://github.com/apache/datafusion-python/issues/1708>.
Ok(PySessionContext {
ctx: Arc::new(self.ctx.as_ref().clone().enable_url_table()),
logical_codec: Arc::clone(&self.logical_codec),
Expand Down Expand Up @@ -1433,10 +1435,13 @@ impl PySessionContext {
/// decode. See [`SESSION_CODEC_ID_PREFIX`].
///
/// Handles derived from one session — `with_python_udf_inlining`,
/// `with_logical_extension_codec` — report the same id even though their
/// codec chains differ, so installing two of them on one target is
/// refused. That is the intended answer: their payloads would be
/// indistinguishable on decode.
/// `with_logical_extension_codec`, [`Self::_install_extension_codecs`] —
/// report the same id even though their codec chains differ, so installing
/// two of them on one target is refused. That is the intended answer: they
/// share a `state_ref`, so their payloads would resolve against the same
/// session and are indistinguishable on decode. Every derivation shares the
/// session for exactly this reason; `enable_url_table` is the one that does
/// not, and it is tracked as a bug.
#[getter]
pub fn __datafusion_codec_id__(&self) -> String {
format!("{SESSION_CODEC_ID_PREFIX}{}", self.ctx.session_id())
Expand Down Expand Up @@ -1608,6 +1613,118 @@ impl PySessionContext {
derived.set_session_query_planner(None);
derived
}

/// Build the codec chains for a `with_extensions` call.
///
/// Private support method for `SessionContext.with_extensions`, and the
/// first of the two phases that method runs. `self` is the context the
/// extensions bound their components against, and is also the
/// `Arc<SessionContext>` every FFI task-context provider they created
/// targets, so the returned handle shares it rather than deriving a new
/// one.
///
/// **Writes nothing.** The codec chains belong to the returned handle
/// rather than to `SessionState`, so this phase is transactional for free:
/// a codec that fails to import, or that collides with an installed id,
/// leaves the caller's context exactly as it was. Binding the planner is
/// the only step that touches the session, and it is deferred to
/// [`Self::_install_extension_planner`] so the planner hooks can run
/// against the final chains.
///
/// Codecs must arrive as objects exposing the capsule getter, never as
/// bare capsules — see [`resolve_bundle_codec_id`].
pub fn _install_extension_codecs<'py>(
slf: &Bound<'py, Self>,
logical_codecs: Vec<Bound<'py, PyAny>>,
physical_codecs: Vec<Bound<'py, PyAny>>,
) -> PyDataFusionResult<Self> {
// Chains are built as local values, so a codec that fails to import --
// or that collides with an id already installed -- leaves the session
// untouched. Nothing is borrowed across a call back into Python.
let (mut logical_codec, mut physical_codec) = {
let this = slf.borrow();
(
this.logical_codec.as_ref().clone(),
this.physical_codec.as_ref().clone(),
)
};

for codec in logical_codecs {
let id = resolve_bundle_codec_id(
&codec,
"__datafusion_logical_extension_codec__",
&logical_codec.codec_ids(),
)?;
let inner_ffi = ffi_logical_codec_from_pycapsule(codec, Some(slf.as_any()))?;
let inner: Arc<dyn LogicalExtensionCodec> = (&inner_ffi).into();
logical_codec = logical_codec.with_additional_codec(id, inner);
}
let logical_codec = Arc::new(logical_codec);

for codec in physical_codecs {
let id = resolve_bundle_codec_id(
&codec,
"__datafusion_physical_extension_codec__",
&physical_codec.codec_ids(),
)?;
let inner_ffi = ffi_physical_codec_from_pycapsule(codec, Some(slf.as_any()))?;
let inner: Arc<dyn PhysicalExtensionCodec> = (&inner_ffi).into();
physical_codec = physical_codec.with_additional_codec(id, inner);
}
let physical_codec = Arc::new(physical_codec);

Ok(Self {
ctx: Arc::clone(&slf.borrow().ctx),
logical_codec,
physical_codec,
})
}

/// Re-export a planner a `__datafusion_session_planner__` hook returned as
/// a capsule, so the next hook in the chain receives one either way.
///
/// A hook may hand back an object exposing `__datafusion_query_planner__`
/// or a raw capsule; the next hook wraps whatever it is given and should
/// not have to branch on which. Importing here also surfaces a malformed
/// planner at the hook that produced it rather than at the final install.
/// Writes nothing.
pub fn _export_query_planner<'py>(
slf: &Bound<'py, Self>,
planner: Bound<'py, PyAny>,
) -> PyDataFusionResult<Bound<'py, PyCapsule>> {
let ffi = ffi_query_planner_from_pycapsule(&planner, Some(slf.as_any()))?;
Ok(create_query_planner_capsule(slf.py(), &ffi)?)
}

/// Commit the query planner for a `with_extensions` call.
///
/// The second phase, run once every codec is installed and every planner
/// hook has returned, so the planner is bound against the final chains.
/// This is the one call in `with_extensions` that writes to the session,
/// and it goes through this context's own `state_ref()`, so providers
/// bound to it stay valid.
///
/// `None` means no bundle supplied a planner. That still rebuilds
/// whichever planner the session already holds against the new chains,
/// exactly as `with_logical_extension_codec` does, and writes nothing at
/// all if the session has no FFI planner to rebuild.
///
/// The caller skips this step entirely when the call installed no codec
/// and no planner, the same way [`Self::with_python_udf_inlining`] returns
/// early for a no-op toggle: there is nothing to rebind against, and the
/// rebuild would drag a planner sitting on another handle's codecs onto
/// this one's.
#[pyo3(signature = (planner=None))]
pub fn _install_extension_planner<'py>(
slf: &Bound<'py, Self>,
planner: Option<Bound<'py, PyAny>>,
) -> PyDataFusionResult<()> {
let planner = planner
.map(|planner| ffi_query_planner_from_pycapsule(&planner, Some(slf.as_any())))
.transpose()?;
slf.borrow().set_session_query_planner(planner);
Ok(())
}
}

impl PySessionContext {
Expand Down Expand Up @@ -1783,6 +1900,10 @@ impl PySessionContext {
/// pointed error everywhere else. Randomness is the point: an id drawn from
/// a namespace another session can mint the same value from — a counter, a
/// chain position — would let an unrelated codec answer for these bytes.
/// Reachable only from `with_logical_extension_codec` and
/// `with_physical_extension_codec`, where `codec_id=` is the way out;
/// `with_extensions` refuses bare capsules outright rather than naming them
/// after something that is not the codec. See [`resolve_bundle_codec_id`].
///
/// An id already in use is rejected rather than shadowed. Two codecs sharing an
/// id are indistinguishable on decode, and the API cannot tell whether two
Expand All @@ -1799,12 +1920,53 @@ fn resolve_codec_id(
return Err(PyValueError::new_err(format!(
"An extension codec with id '{id}' is already installed on this session. Two \
codecs cannot share an id, because a payload names its codec by id when it is \
decoded. Pass `codec_id=` to give this one a distinct identity."
decoded. Give this one a distinct identity: declare \
`__datafusion_codec_id__` on the object being installed, or pass `codec_id=` \
if you are calling `with_logical_extension_codec` or \
`with_physical_extension_codec` directly."
)));
}
Ok(id)
}

/// Resolve the wire id for a codec contributed through `with_extensions`,
/// requiring an object that can name itself.
///
/// `with_extensions` takes no `codec_id=`, so the only naming channels are the
/// ones [`derive_codec_id`] reads off the handed-over object: a declared
/// `__datafusion_codec_id__`, or its class's `module.QualName`. A bare capsule
/// has neither. Naming it after the bundle that contributed it looks like an
/// answer and is not one: the bundle is whatever object the caller passed to
/// `with_extensions`, so a bundle that wraps another library's bundle — the
/// natural way for an application to package several libraries as one — would
/// stamp its own identity onto the inner library's codecs and silently change
/// the wire format. The inner library cannot defend against that no matter what
/// it declares, and the mismatch does not surface until a plan fails to decode
/// in another process.
///
/// So the capsule is refused here, where the author can fix it by wrapping it
/// in an object. Wrapping also decouples the codec's wire identity from the
/// bundle's Python class name, which is the whole point of
/// `__datafusion_codec_id__`.
fn resolve_bundle_codec_id(
codec: &Bound<'_, PyAny>,
getter: &str,
existing: &[&str],
) -> PyResult<String> {
if codec.is_instance_of::<PyCapsule>() {
return Err(PyTypeError::new_err(format!(
"A codec contributed through `with_extensions` must be an object exposing \
`{getter}`, not a bare PyCapsule. A capsule carries no type of its own, so \
there is nothing to name the codec by, and a payload names its codec by id \
when it is decoded — an id that has to mean the same thing in whichever \
process decodes. Wrap the capsule in an object that exposes `{getter}` and, \
if the class name is not the identity you want on the wire, declares \
`__datafusion_codec_id__`."
)));
}
resolve_codec_id(codec, None, existing)
}

fn derive_codec_id(codec: &Bound<'_, PyAny>, explicit: Option<String>) -> PyResult<String> {
if let Some(id) = explicit {
return Ok(id);
Expand Down
Loading
Loading