Skip to content

Commit 7022baa

Browse files
timsaucerclaude
andauthored
Add method on SessionContext to add all extensions from one library (#1679)
* Add atomic SessionContext.with_extensions API Installing FFI extension codecs and query planners by chaining the existing with_* methods can bind task-context providers to intermediate contexts that are later collected, breaking the weak provider reference over the FFI boundary. with_extensions creates one destination context, passes it to each extension factory so components bind to that exact context, and installs everything in a single state write. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add extension-bundle example and with_extensions FFI tests MyPlannerExtension in the query-planner example crate implements the __datafusion_session_extension__ protocol from Rust: it extracts the destination context's task-context provider, binds fresh observing codecs and a planner to it, and returns SessionExtensionComponents. Its codecs record the max_rows config value resolved through the weak provider, letting tests prove the provider targets the returned context rather than the source. Documents with_extensions as the preferred API in the FFI guide. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Document and test the context-outlives-DataFrame contract A DataFrame does not keep its SessionContext alive. FFI components hold a weak task-context provider, so operations that reach an FFI codec after the context is collected fail with a clean out-of-scope error rather than crashing. Lock that behavior in with a test and document the ownership contract in the FFI guide and with_extensions docstring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Skip private internal methods in wrapper coverage test Single-underscore methods on internal pyo3 classes (such as SessionContext._install_extensions) are private support methods for the Python wrappers and do not require a public wrapper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Test planner rebinding and codec ids in with_extensions A codec-only bundle installed on a context that already holds an FFI planner must rebind that planner to the new chains, so the planner decodes through the bundle's codecs. Codec ids are derived from the exporting class, so two bundles shipping the same codec class collide and the install is refused. Declaring __datafusion_codec_id__ on the object a bundle hands over resolves it, and both chains then install. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fix duplicate attribute docs in SessionExtensionComponents The docs build runs Sphinx with --fail-on-warning. SessionExtensionComponents documented its fields in both a napoleon `Attributes:` section and the dataclass class-body annotations, so autoapi emitted each field twice and the build failed with six "duplicate object description" warnings. Move each field's description to a per-field docstring under its annotation so autoapi renders exactly one entry per field. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Move session extension types into datafusion.extensions QueryPlannerExportable, SessionExtensionComponents, and SessionExtensionExportable describe how an extension library plugs into a session, not how a SessionContext behaves. Give them their own module so context.py does not keep absorbing the extension surface as it grows. extensions.py imports SessionContext, the codec protocols, and CapsuleType under TYPE_CHECKING only, so context.py can import from it at runtime without a cycle. All three names remain importable from datafusion and datafusion.context; QueryPlannerExportable stays out of the top-level __all__ as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Run the with_extensions docstring example in CI The example was marked `+SKIP` because the main suite has no built FFI extension to import, which is exactly how such an example rots. Parse the statements out of the live docstring in the query-planner example suite, drop the skip, and execute each one against a real extension bundle. Only names are redirected: `my_extension` resolves to a stand-in combining this repository's provider codecs and planner, and `SessionContext` supplies the config that planner reads. A renamed method, a changed signature, or a wrong expected output now fails CI, which already runs this suite. Also drop the `extensions` Args entry's restatement of the type hint and say instead what the hint does not: install order is chain order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Share the session in with_extensions instead of forking it `_derive_for_extensions` minted a new `Arc<SessionContext>` via `new_with_state(self.ctx.state())`. Every other `with_*` method shares `Arc::clone(&self.ctx)`, and `new_with_state` carries the session id over, so `with_extensions` returned a second live session claiming the same `session_id()` as the source while holding independent `SessionState`. Configuration and the function registry diverged, catalogs stayed shared, and both handles reported the same `__datafusion_codec_id__` — which is `session:<session_id>` and exists precisely to distinguish codec chains, so installing both on a third session was refused as a duplicate id. The fork also bought nothing. It was introduced to keep components from binding to an intermediate context that could be collected, but there is one `Arc<SessionContext>` per session, so no such intermediate exists; deriving one is what creates the hazard. Rule 6 of the ffi-capsule-protocol skill already said to mutate `SessionState` in place rather than derive a replacement. Delete `_derive_for_extensions` and hand the receiver to the extension factories. `_install_extensions` already returned a handle sharing `Arc::clone(&slf.borrow().ctx)`, so removing the fork upstream of it is the whole change. Atomicity is unaffected: both codec chains are built as locals and state is written exactly once, at the end, in `set_session_query_planner`. Replace `test_with_extensions_provider_targets_returned_context`, which is vacuous once the session is shared, with `test_with_extensions_shares_the_session_with_the_source`. It asserts matching session ids and that a `SET` issued through the source after installation is visible to the provider the bundle bound. Reintroducing the fork fails it. Update the prose that described the fork-era design: the `with_extensions` docstring and `SessionExtensionComponents` / `SessionExtensionExportable` in `datafusion.extensions`, the `with_extensions` and "What a derived context shares" sections of the FFI guide, the query planner example's README and `extension.rs` comments, and two test docstrings. Note the shared-session mechanism in Rule 6 of the skill, since `with_extensions` is where it is easiest to get wrong. `enable_url_table` is once again the only method that mints a second `Arc<SessionContext>` for a session; its comment, the FFI guide, and the skill now also record that it forks state while keeping the session id, tracked as a bug in #1708. Also add the missing doctest to `SessionExtensionComponents` and a pointer to `with_extensions` from the upgrade guide, which described only the low-level install path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Name a bundle's bare capsules after the bundle A codec handed to `with_extensions` as a bare `PyCapsule` fell through to `anon:<uuid4>`, an id private to the session that installed it. Plans written through it are undecodable anywhere else, and `with_extensions` accepts no `codec_id=` to override that — so the workaround was to wrap the capsule in an object declaring `__datafusion_codec_id__`, which nothing documented. A distributed engine has to decode its plans in another process, so the shape it would naturally ship — a Rust bundle handing over capsules, as `MyPlannerExtension` does — was the one shape that could not work. The bundle is the stable name that was missing. It is a plain Python object, so its `module.QualName` is library-owned and exactly as stable across processes as an exporting codec class's, which arm 3 of `derive_codec_id` already trusts. The capsule was unnameable only because a capsule carries no type of its own, not because nothing stable was in reach. Resolve a capsule's id through the contributing bundle, using `derive_codec_id` itself so the bundle inherits the same `__datafusion_codec_id__` escape hatch against a class rename. The fallback applies only where randomness would have: an id declared on the handed-over object, or that object's own class, still wins, so an extension can name a codec directly. Two bare capsules of one kind from one bundle collide and are refused. Numbering them by position would be exactly the id `codec.rs` rejects for `anon:` — one another library can mint the same value from — and would break stored plans the first time the bundle reordered what it returns. `resolve_codec_id` gains the bundle argument, `_install_extensions` takes (codec, bundle) pairs, and the collision message now names both routes to a distinct identity; it previously offered only `codec_id=`, which is unreachable from `with_extensions`. Covered in `python/tests/test_context.py`, which reaches every arm without a built extension library: the bundle-derived name, an extension pinning its own id, an id on the handed-over object winning, an exporting object keeping its own, and the two-capsule collision. The cross-FFI case is pinned in the query planner example, where a Rust bundle's capsules must report `datafusion_ffi_query_planner_example.MyPlannerExtension` and no id may be `anon:`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Stop claiming _install_extensions always writes state The doc comment said "the final state is written through this context's own `state_ref()`", which overstates it. `set_session_query_planner` returns early when there is no planner to bind, and the codec chains live on the returned `PySessionContext` fields rather than in `SessionState` — so a codec-only install onto a session with no FFI planner writes nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Tidy the loose ends from review of with_extensions Mark `SessionExtensionExportable` `@runtime_checkable` and have `with_extensions` check it with `isinstance` rather than `hasattr`, so the annotation and the runtime check are the same statement, and callers can ask the question too. Covered by a doctest on the protocol. Replace the leading-underscore skip in `test_wrapper_coverage` with a named allowlist. The pattern also excused `DataFrame._repr_html_`, which a wrapper does have to provide, so a two-method need was weakening coverage for every private name. Removing `_install_extensions` from the allowlist fails the test, so the entry is load-bearing rather than decorative. Say in `_CodecOnlyExtension` that retaining the context is what the protocol tells real extensions not to do, and that it is kept only so a test can assert which context the factory was handed. Let the docstring-example shim in the query planner example accept a config positionally, the way the real constructor does. Editing the docstring to `SessionContext(config)` now fails as a doctest diff rather than as a `TypeError` inside the harness. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Require with_extensions codecs to be objects, not capsules Reverses "Name a bundle's bare capsules after the bundle". Deriving a capsule's id from the bundle that contributed it reads the identity off the wrong object: the bundle is whatever the caller passed to with_extensions, so an application that packages several libraries as one bundle of its own stamps its identity onto the inner libraries' codecs. Their pinned __datafusion_codec_id__ is discarded and there is nothing the inner library can do about it, since its object never reaches _install_extensions. Nothing fails at install time; the mismatch surfaces as an undecodable plan in the process that reads it, naming an id nobody wrote in source. So with_extensions now refuses a bare capsule and names the getter to implement. An id read off the handed-over object is composition-stable by construction, which the new tests pin at both layers. This also decouples a codec's wire identity from the bundle's Python class name, which is what __datafusion_codec_id__ exists for, and closes the case where a bundle built by a factory function contributed a wire id containing "<locals>". The low-level methods keep accepting capsules: they take codec_id=, so the random anon: arm still has an escape hatch. Query planners are unaffected, carrying no wire id. MyPlannerExtension gains BundledLogicalCodec and BundledPhysicalCodec, small pyclasses holding the bound FFI codec and declaring pinned ids, as the reference shape for a library whose plans leave the process. Also, unrelated to the above but adjacent in the docs: with_extensions never said that a bundle-supplied planner replaces an installed one rather than layering, and the SessionExtensionComponents example that showed a codec was fully skipped with undefined names. Both fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Install extension codecs and planners in two phases A session chains many codecs and dispatches between them by id, so codecs accumulate and their order does not affect decoding. A session holds exactly one query planner, so planners cannot accumulate — they compose by nesting, each wrapping the one before it. Collecting both from a single hook forced with_extensions to refuse more than one planner per call, because every factory ran before anything was installed and so no bundle could see another bundle's planner to wrap it. Two libraries that each ship a planner could not be installed together at all, and splitting them across two calls silently discarded the first. Codecs now come from __datafusion_session_extension__ and planners from a new __datafusion_session_planner__(ctx, fallback), which runs once per bundle in argument order after every codec is installed. Each receives the planner built so far; wrapping it nests this bundle outside the previous one, so the last bundle listed ends up outermost. A bundle implements either hook or both, which also lets a library that ships only an optimizing planner stop returning empty components. SessionExtensionComponents loses its query_planner field. Running the planner hooks after every codec is installed is what makes a nested planner safe. The rebuild that follows a later codec install reaches only the outermost layer, so a fallback captured against a partial chain would stay stale; there is now no "afterwards" within a call. Atomicity is unchanged. _install_extension_codecs writes nothing — the chains belong to the returned handle — so phase one is transactional for free, and the nest is built in memory with _install_extension_planner performing the single session write after the last hook returns. A hook that raises in either phase leaves the caller's context as it was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Give the planner example a node only its own codec can carry The bundle shipped a planner and codecs, but the halves never met: the planner emitted a stock GlobalLimitExec and the codecs delegated everything to the default codec. So the example asserted by structure that a planner and its codecs belong together without demonstrating why, and no test would have caught a bundle whose planner emits a node its own codec cannot encode. DistributedQueryPlanner now wraps its result in a DistributedExec, a type private to this library, and ObservingPhysicalExtensionCodec claims it by downcast and rebuilds it from its inputs. Nothing else in the session knows the type, which is the reason the two ship as one bundle. The observing codecs stop being dead weight in the process — they were previously never consulted, and decode_max_rows_seen had no caller. Also documents what codec order does and does not control, which building this surfaced. Decoding routes by id and is never order-dependent. Encoding stops at the first codec that claims the node, so a codec claiming a broad category — MyPhysicalExtensionCodec claims any ForeignExecutionPlan — takes nodes from any library installed after it. The query still succeeds; only the library that wrote the bytes changes, which breaks a plan that has to decode elsewhere. That gives a bundle two reasons to want different positions for its two halves. The guide now says to contribute each half at its own position with a small adapter rather than reordering, since the hooks are independent, and treats the low-level sequence as the last resort it is: it works, but it hands back responsibility for codec-before-planner ordering and leaves a hand-layered fallback holding the codecs it captured. No attempt is made to express every permutation from one call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drop the planner example's logical codec observer The physical observer earns its place now that it claims the bundle's own DistributedExec, but the logical one never did and cannot: it declines all four methods to the default codec, and this library defines no logical extension node for it to claim. The FFI logical codec does not carry arbitrary LogicalPlan::Extension nodes anyway, so there is no logical analogue to give it. Measuring a query confirms it: every record_task_ctx firing comes from the physical decode path and none from the logical one. BundledLogicalCodec now wraps DefaultLogicalExtensionCodec directly, which keeps what the logical half actually demonstrated -- a bundle contributing both codec kinds under ids it declares -- and drops 54 lines of trait impl and hand-written Debug that fed an accessor nothing could observe. Also narrows PlannerObservations::used_fallback back to private. It is read only through MyQueryPlanner::used_fallback in the same module; its neighbours need pub(crate) because extension.rs reads them, and it does not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Export QueryPlannerExportable and drop the empty-extensions guard Two loose ends from review. QueryPlannerExportable was the only member of the extension protocol family left in the submodule while SessionExtensionComponents, SessionExtensionExportable and SessionPlannerExportable were exported from the package root. It types the planner a __datafusion_session_planner__ hook returns, so a bundle author needs it just as much, and one family member importing differently from the rest is a papercut with no upside. Also fixes two doc references that stopped resolving when these classes moved out of context.py: a bare :class:`QueryPlannerExportable` and a bare :py:class:`SessionExtensionComponents`, both now spelled with their module the way the neighbouring datafusion.user_defined references are. with_extensions() with no arguments raised instead of installing nothing. That put it out of family: every sibling varargs method -- DataFrame.select, filter, sort, drop, window -- accepts zero arguments and returns a no-op result, and the two existing "at least one" guards in the codebase both cover cases with no meaningful identity element, which this is not. Installing no extensions has an obvious answer, and a caller assembling the list from a plugin registry should not have to special-case it being empty. Phase two still runs, so the empty case rebinds an existing FFI planner to unchanged chains; a test pins that the planner survives it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Describe both extension hooks in the upgrade guide The planner-install section still said a bundle exposes __datafusion_session_extension__, which stopped being the whole protocol when planners moved to a hook of their own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Generate heading anchors for h4 so the FFI guide's links resolve `myst_heading_anchors` was 3, but the extension-bundles section added in this branch cross-references its own `####` subsections. Sphinx warns `'myst' cross-reference target not found: 'when-codec-order-does-matter'` and renders that link as plain text; the docs build does not pass `-W`, so it went unnoticed. Bumping to 4 rather than promoting the heading keeps the four subsections nested under `### Extension bundles: with_extensions`, where they belong. Only one other `####` heading exists under `docs/source/`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Skip the planner commit when with_extensions installs nothing `with_extensions` ended every call with `_install_extension_planner`, which rebuilds `SessionState` to rebind an existing FFI planner to this handle's codec chains. When the call installed no codec there is nothing to rebind against, so the rebuild is at best churn — and at worst it drags a planner that is sitting on another handle's codecs onto this one's, silently undoing that install. `with_python_udf_inlining` already guards its no-op toggle for exactly this reason; `with_extensions` now guards the same way. `test_with_extensions_installing_nothing_leaves_the_planner_alone` covers both shapes of "installed nothing": no arguments at all, and a bundle whose hooks answer empty. Both fail without the guard, with the planner left on an empty logical chain and the query erroring out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Reject a lone codec where SessionExtensionComponents wants an iterable `logical_extension_codecs=codec` instead of `(codec,)` is the easy mistake to make, and it surfaced as `'MyCodec' object is not iterable` raised by an `extend` call inside `with_extensions` — naming neither the field nor the hook that built the value. `__post_init__` now checks it, so the error lands in the extension library's own frame and says which field is wrong and how to spell one codec. It also normalizes each field to a tuple. The declared type is a tuple and the class is frozen, so a list left in place would be a mutable member of an immutable value, and a generator would be exhausted by the first read. A str is refused rather than normalized: it is iterable, so it would otherwise become a tuple of characters and fail much later as that many bogus codecs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Widen the with_extensions annotation to both hooks `with_extensions` accepts a bundle implementing either hook — the runtime check tests against both protocols, `test_with_extensions_accepts_a_planner_only_extension` pins it, and the FFI guide's `PlannerOf` adapter recommends contributing only the planner half. The annotation named `SessionExtensionExportable` alone, so a type checker rejected the very shape the guide tells authors to write. Two doc comments also went stale. `test_with_extensions_no_extensions_keeps_an_installed_planner` still described phase two running and rebinding an existing planner, which the no-op guard now skips outright, and `__datafusion_codec_id__` listed a `_install_extensions` method that never existed under that name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Correct three claims in the extension bundle docs The `SessionExtensionComponents` doctest took its codec capsule off a `SessionContext()` that was dropped on the same line. An FFI codec holds its task-context provider weakly, so that capsule names a session that is already gone — the doctest only reads an id back so it passes, but it is the exact shape the FFI guide warns against. It now keeps the context in a name. `SessionPlannerExportable` called returning `fallback` a wrap that "contributes nothing". It is not: the capsule the first bundle receives wraps the session's planner for export, so handing it back installs it as a foreign planner and every later plan crosses an FFI boundary that was not there before. `None` is the no-op. Corrected in the protocol docstring, the FFI guide's canonical section, and the `_PlannerExtension` test helper that repeated the claim. `__post_init__` walked a written-out list of field names. It now walks `dataclasses.fields`, filtered on the `_codecs` suffix so a codec field added later is normalized without anyone remembering to name it, and a future field that is not a codec collection is left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Rename _rebind_query_planner to _export_query_planner The method rebinds nothing. It imports whatever a `__datafusion_session_planner__` hook returned — an object exposing the getter or a raw capsule — and hands back a capsule, so the next hook receives one either way; its own doc comment already said "re-export". Meanwhile "rebind" means something specific and different in this file: rebuilding an installed planner against a handle's codec chains, which is what `set_session_query_planner` does. Freeing the word keeps the two apart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Say which codec chains each extension hook's context carries `__datafusion_session_planner__` was documented as receiving a context that carries the final codec chains, and `MyPlannerExtension` relies on exactly that when it takes the host's codecs off `ctx` instead of minting its own. The other side was never stated: `__datafusion_session_extension__` runs before anything is installed, so its `ctx` is the same session with the chains the receiver already had — missing this call's codecs, including the bundle's own. Both hooks hand back a valid task-context provider, which is what components actually need, so the difference only bites an author who reads codec chains off the context. Recorded on `SessionExtensionExportable`, in the FFI guide's two-phase section, and in Rule 2 of the capsule-protocol skill. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Make the planner-hook tests assert what their names claim `test_with_extensions_threads_the_planner_through_in_order` never checked an order: it asserted each hook recorded one fallback and that the second was not None, both of which hold for a host that ran the hooks backwards. `test_with_extensions_skips_a_planner_hook_returning_none` asserted only that downstream ran, while its comment claimed the skipped hook had not become downstream's fallback. `_PlannerExtension` now takes an optional shared list the hooks append themselves to, so order is observable. The threading test asserts that list, plus that the second hook's fallback is not the object the first was handed -- the host re-exports every return value before passing it on. A capsule is opaque from Python, so that cannot separate a re-export of the first planner from a fresh read of the session's; the comment says so and points at the FFI suite's `test_with_extensions_nests_planners_in_argument_order`, which pins the nesting by asserting the outer planner delegated. The skip test records the skipped hook's fallback too, and asserts both hooks ran, that downstream was handed a different capsule, and that the resulting context still queries. Each new assertion was mutation-tested: reversing the planner loop, dropping the `if supplied is None: continue`, and replacing the re-export with a straight pass-through each fail exactly one of these tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * 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> * docs: add a user-facing extensions page and split distributing-work The user guide had no page for someone who installs an extension library and wants to run queries with it. `with_extensions` was described only in a 123-line docstring and in the contributor-guide FFI page, which opens by explaining that Rust has no stable ABI — the wrong altitude for a reader who just wants their queries to run somewhere else. Adds `user-guide/extensions.md`: what an extension library is, which kinds register directly versus needing `with_extensions`, the two failure modes that actually bite (a collected context, a version mismatch), and how to check what a session was taught. It names no capsule, no ABI, and no codec id; the only occurrences of `FFI` and `TaskContextProvider` are inside the error string a reader would be searching for. Splits `distributing-work.md` into a directory. The page was entirely about pickling expressions to worker pools and treated query-level distribution as two stubs at the end, so nothing on the site connected `with_extensions` to distribution at all — which is the road a data scientist is actually looking for. The index now asks who owns the partitioning decision and routes accordingly; `query-engines.md` carries the missing bridge and absorbs the two upstream work-in-progress sections. Wires `sphinx-reredirects`, which has been a declared dependency since #1578 without ever being enabled, so the old `distributing-work.html` URL keeps working. Also fixes three pointers that went stale in the MyST migration and still named `.rst` files, a malformed `ref:` role in udf-and-udfa.md that rendered as literal text, and a doubled "the" in data-sources.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: split the FFI guide into one section per audience `contributor-guide/ffi.md` had grown to 795 lines serving three different readers at once, filed under a section whose index says it is for people contributing to this repository. An engineer at delta-rs or a distributed-engine vendor is neither a contributor nor an end user; they consume a published, versioned protocol, and the only description of it lived behind a heading telling them the page was not for them. Adds a third top-level section, `extension-guide/`, so the sidebar reads User Guide / Extension Guide / Contributor Guide — one per audience. It carries the `(ffi)=` label, so every existing reference keeps resolving, including the `:ref:`ffi`` inside `context.py`'s docstring that ships in the wheel. The maintainer-facing rationale moves to `contributor-guide/ffi-internals.md`: the weak-`Arc` scheme, why repairing an orphaned provider cannot work, why planner codec rebinding is one level deep, and the two upstream issues. An extension vendor should not be reading "that is a bug rather than a design, do not copy the pattern" halfway down their integration guide. The PyO3 `frozen` policy moves to `contributor-guide/pyo3-guidelines.md`. It is project review policy with nothing to do with FFI, and extracting it repairs a prose bug: it had been spliced into the middle of "Implementation Details", so the sentence "If you were interfacing with a library that provided the above `FFI_TableProvider`" resumed 46 lines after the snippet it referred to. Those two halves are rejoined on `capsule-protocol.md`. Fills the coverage gap the split exposed. The codebase exports 18 capsule getters and the old page documented 7; the remaining 11 appeared on no page that even listed them. The section index now carries a table of all 18, and `table-providers.md` and `functions.md` document the catalog family, table functions, physical optimizer rules, and extension options for the first time. Corrects the argument rule while moving it. The old section asserted that capsule getters "receive the SessionContext they are being installed on", which is true for the codec, planner, and table-function hooks but not for the catalog family: `CapsuleGetterArg::LogicalCodec` passes the host's logical codec as a bare capsule, and `__datafusion_table_provider__` gets a session from `SessionContext.register_table` but a codec capsule from `Schema.register_table`. Nothing breaks, because every implementation passes the argument to `ffi_logical_codec_from_pycapsule`, which handles both — so the rule is now stated by capability rather than by type, and the upgrade guide says which hooks changed. Also converts the guide's five in-page heading links to labelled `{ref}` targets, since heading anchors rot silently on rewording, and redirects the old `contributor-guide/ffi.html` URL. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: give each docstring claim one canonical home The extension docstrings and the FFI guide had grown the same six claims 3-5 times each, in wording that had already started to drift. `with_extensions` was 123 lines — the longest docstring in the package by 30 — and most of it argued a design rather than stating a contract. Routes each recurring claim to one home and leaves a one-line pointer elsewhere. Session sharing and context lifetime move onto the `SessionContext` class docstring, since they are properties of the type and that is exactly why five methods had each reworded them. The `None`-vs-`fallback` contract moves onto `SessionPlannerExportable`, since it is a return-value contract of one method. The two-phase rationale, the objects-not-capsules argument, and the codec-order argument stay in the guide, which owns the "why". The duck-type-the-session rule moves from `QueryPlannerExportable` to `LogicalExtensionCodecExportable`, which is already the designated `session` reference for that family and where the codec protocols were pointing for it anyway. It also gains the fact the old text was missing: across the protocol this argument is not always a session, so duck-typing it is not a style preference. Not every docstring shrank. `__datafusion_query_planner__` was 4 lines with no example while `set_query_planner` told callers to capture the fallback through it, so it grew to 25. Several others grew by gaining the `Args`/`Returns`/ `Raises` sections they were missing, and by trading `+SKIP` examples for runnable ones — a `SessionContext` satisfies the capsule-getter protocols, so its own exported capsule stands in for a library's without a build step. The total across these sixteen docstrings is roughly flat, at 577 lines before and 629 after; what changed is that the rationale left and the contract arrived. The 21 lines of `dataclasses.fields` rationale on `SessionExtensionComponents.__post_init__` become comments in the method body. That is the fix rather than touching `autoapi_options`: `__post_init__` is a *special* member, so dropping `private-members` would not hide it, and dropping `special-members` would delete the entire `__datafusion_*__` reference surface. `conf.py` now records why that setting is deliberately left alone, and de-duplicates the four re-exported `extensions` classes the way it already did for `DataFrame` and `SessionContext`. Adds `python/tests/test_docstrings.py`, which is what would have caught the 123-liner: a 95-line ceiling with no waiver list, a doctest-presence check over the extension protocol, and a check that a docstring naming the guide actually links it. The last two each found a real defect on first run. Also fixes what this branch made newly load-bearing in the serialization surface: `plan.py`'s single-backtick RST rendered `LogicalExtensionCodec` and friends as italics rather than links into the new API, and the `global_ctx()` fallback in `Expr.from_bytes` now silently yields a context with no extension codecs, which before this branch lost only registrations. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: point the non-Sphinx consumers at the new pages Four links in the two example READMEs pointed into `docs/source/.../ffi.md` with heading anchors. Sphinx does not link-check those, so deleting the page would have left them silently 404ing. They now use published URLs, which also fixes a second problem: a relative path into `docs/source` only renders on github.com and is broken for anyone reading the README from crates.io, an sdist, or a vendored copy. `grep -rn "docs/source" examples/` is now empty. `.ai/skills/ffi-capsule-protocol/SKILL.md` named the deleted page as "where the truth is", and AGENTS.md sends agents to that skill before they touch a capsule getter. It now points at the specific extension-guide pages and separately at `ffi-internals.md`. `llms.txt` filed the whole subject under "Optional" as "extending the Python bindings" — the wrong shelf for the headline feature of a major release, since that section means "skippable on a tight context budget". It gains an Extensions and distribution section, `datafusion.extensions` and `datafusion.ipc` in the API list, both FFI example crates, and the corrected `distributing-work` URL. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * 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> * test: match the hook table against dispatch sites, not raw text `test_hook_reference_table_lists_every_hook` scanned every byte of `crates/**.rs` and `python/datafusion/**.py` for `__datafusion_*__`, so comments and docstrings counted as evidence a hook exists. A doc-comment contrasting a hook with one that was removed, or naming a hypothetical, would have had to be deleted or added to the guide's table, and neither is right. Count sites instead. On the Rust side a site is a string literal holding nothing but the hook name -- what `hasattr`, `getattr`, and `call_capsule_getter` are handed -- or a `fn` of that name, which is a hook the host implements itself. Error strings that merely embed a name no longer count; each already sits beside a real lookup. On the Python side, read the syntax tree rather than the text: a method being defined, an attribute being accessed, or a string standing alone. A docstring is one string node holding the whole docstring, so prose drops out without a rule of its own. The dispatched set is unchanged at 18, still matching the table exactly. Verified in both directions: a comment naming a removed hook now passes, while adding a real lookup for an undocumented name still fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent b6c6f5b commit 7022baa

53 files changed

Lines changed: 5444 additions & 785 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

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

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,33 @@ library reaches things only the session has.
6262
session satisfies the protocol too — `ctx.__datafusion_query_planner__()` and
6363
`ctx.__datafusion_query_planner__(ctx)` are both valid.
6464

65+
`__datafusion_session_planner__(ctx, fallback)` is the exception to the shape
66+
above: it takes a second argument, the planner assembled so far. A session has
67+
one planner slot, so planners compose by nesting rather than by chaining, and
68+
the host hands each bundle the previous layer instead of letting it capture one.
69+
Wrap `fallback` and delegate to it; returning a planner that ignores it discards
70+
every layer beneath, including one the session already had. It runs after every
71+
bundle's codecs are installed, so `ctx` carries the final chains.
72+
73+
That is also the only hook where it does. `__datafusion_session_components__`
74+
runs before anything is installed, so its `ctx` still carries the chains the
75+
receiver had — the same session, and the same task-context provider, but not
76+
this call's codecs, not even your own. Read the host's codec chains in the
77+
planner hook, never in the extension hook.
78+
79+
A *codec* must always be handed over as an object implementing its getter, never
80+
as the bare capsule the getter returns; `with_extensions` refuses a capsule.
81+
A codec's wire id — the string a payload names on decode, which has to mean the
82+
same thing in whichever process decodes — is read off the object it arrives as,
83+
and a capsule has no type to read one from. Deriving the id from the bundle that
84+
contributed the capsule is not the fix: the bundle is whatever object the caller
85+
passed, so an application packaging your library inside a bundle of its own
86+
would re-tag your payloads and they would stop decoding where they are read. If
87+
the object's class name is not the identity you want on the wire, declare
88+
`__datafusion_codec_id__` on it. `BundledLogicalCodec` in
89+
`examples/datafusion-ffi-query-planner-example/src/extension.rs` is the shape.
90+
This applies only to codecs — a query planner carries no wire id.
91+
6592
## Rule 3 — never construct a `SessionContext` in an extension library
6693

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

184+
`SessionContext.with_extensions` is where this rule is easiest to get wrong,
185+
because "bind the components to the context you are about to return" reads like
186+
an instruction to derive one first. It is not: the factories are handed the
187+
receiver, and the returned handle shares its allocation. There is nothing to
188+
keep alive separately and nothing to garbage-collect out from under a provider.
189+
157190
`SessionContext.enable_url_table` is the one method that mints a second
158-
allocation for a session. Its result must not outlive the receiver.
191+
allocation for a session. Its result must not outlive the receiver, and it also
192+
forks the session's `SessionState` while keeping its id, so two handles report
193+
one `session_id()` with divergent configuration. That is a bug rather than a
194+
design — tracked in
195+
[apache/datafusion-python#1708](https://github.com/apache/datafusion-python/issues/1708)
196+
— so do not cite it as precedent for deriving a replacement context.
159197

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

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

186224
## Where the truth is
187225

188-
- `docs/source/contributor-guide/ffi.md` — the protocol, the fork caveat.
226+
- `docs/source/extension-guide/` — the protocol, for the library author.
227+
`capsule-protocol.md` has the hook convention and what the getter argument
228+
actually is; `codecs.md`, `bundles.md`, and `query-planners.md` have the
229+
per-component rules; `index.md` lists all 18 hooks.
230+
- `docs/source/contributor-guide/ffi-internals.md` — why the framing is shaped
231+
this way, including the weak-`Arc` scheme and the one-level rebind.
189232
- `docs/source/user-guide/upgrade-guides.md` — every past migration.
190233
- `crates/core/src/codec.rs` — the codec chain: the envelope, identity dispatch,
191234
and the two unframed cases from Rule 8.

AGENTS.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,47 @@ Every Python function must include a docstring with usage examples.
133133
`array_sort`) only need a one-line description and a `See Also` reference to the
134134
primary function. They do not need their own examples.
135135

136+
### One canonical home per claim
137+
138+
A claim lives where the reader already is when they need it — **exactly once**.
139+
140+
- A property of one callable's arguments, return value, or errors → that
141+
callable's docstring.
142+
- A property of a *type* that several callables share → the class docstring.
143+
(Session sharing and context lifetime live on `SessionContext`, not on each
144+
of the five `with_*` methods.)
145+
- **Why** the API is shaped this way, a multi-library recipe, Rust-side code, a
146+
trade-off, or a limitation with an upstream issue → the narrative guide under
147+
`docs/source/`.
148+
149+
Everywhere else: one sentence plus one Sphinx role. A docstring may *state* a
150+
claim the guide also makes; it must not *argue* it — no "because", no "the
151+
reason is", no counter-argument.
152+
153+
**The test:** if a paragraph would survive being moved into the guide unchanged,
154+
move it. `python/tests/test_docstrings.py` enforces a length ceiling, which is
155+
the symptom this rule treats.
156+
157+
When you point at the guide, use a real `:ref:` to a specific label. Prose
158+
saying "see the extensions guide" with no role is a dead end in the rendered
159+
HTML.
160+
161+
### Examples that need a compiled extension
162+
163+
Some APIs cannot be demonstrated without a built FFI extension library, which
164+
this package does not ship. The convention is:
165+
166+
1. A **runnable** example block first, using only the wheel. A `SessionContext`
167+
satisfies the capsule-getter protocols, so its own exported capsule stands in
168+
for a real library's in a doctest.
169+
2. Then a `# doctest: +SKIP` block showing real usage, at most a few lines, with
170+
one line of prose naming the test that runs it for real.
171+
3. Every `+SKIP` block needs that mirror. See
172+
`test_with_extensions_docstring_example_still_runs` in
173+
`examples/datafusion-ffi-query-planner-example`, which parses the live
174+
docstring, drops the skip, and executes it — so a renamed method or a wrong
175+
expected output fails there.
176+
136177
## Aggregate and Window Function Documentation
137178

138179
When adding or updating an aggregate or window function, ensure the corresponding

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/core/src/codec.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,7 @@ impl PythonLogicalCodec {
660660
/// `cloudpickle.loads` on the inline `DFPY*` payload. It does
661661
/// **not** make `pickle.loads(untrusted_bytes)` safe; treat every
662662
/// `pickle.loads` on untrusted input as unsafe regardless of this
663-
/// setting. See `docs/source/user-guide/io/distributing_work.rst`
663+
/// setting. See `docs/source/user-guide/distributing-work/expressions.md`
664664
/// (Security section) for the full threat model, and Python's
665665
/// [pickle module security warning][1] for why `pickle.loads` is
666666
/// unsafe in general.

crates/core/src/context.rs

Lines changed: 171 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ use datafusion_python_util::{
6666
};
6767
use object_store::ObjectStore;
6868
use pyo3::IntoPyObjectExt;
69-
use pyo3::exceptions::{PyKeyError, PyRuntimeError, PyValueError};
69+
use pyo3::exceptions::{PyKeyError, PyRuntimeError, PyTypeError, PyValueError};
7070
use pyo3::prelude::*;
7171
use pyo3::types::{PyCapsule, PyDict, PyList, PyTuple};
7272
use url::Url;
@@ -424,10 +424,12 @@ impl PySessionContext {
424424

425425
pub fn enable_url_table(&self) -> PyResult<Self> {
426426
// Pre-existing caveat, unrelated to query planners: this is the one
427-
// method that mints a second `Arc<SessionContext>` for a session. Any
428-
// weak `FFI_TaskContextProvider` handed out by the receiver stays bound
429-
// to the receiver, so the returned context must not outlive it. See
427+
// method that mints a second `Arc<SessionContext>` for a session, and
428+
// it also forks the session's state while keeping its id. Any weak
429+
// `FFI_TaskContextProvider` handed out by the receiver stays bound to
430+
// the receiver, so the returned context must not outlive it. See
430431
// `set_session_query_planner` for why everything else mutates in place.
432+
// Tracked as a bug in <https://github.com/apache/datafusion-python/issues/1708>.
431433
Ok(PySessionContext {
432434
ctx: Arc::new(self.ctx.as_ref().clone().enable_url_table()),
433435
logical_codec: Arc::clone(&self.logical_codec),
@@ -1433,10 +1435,13 @@ impl PySessionContext {
14331435
/// decode. See [`SESSION_CODEC_ID_PREFIX`].
14341436
///
14351437
/// Handles derived from one session — `with_python_udf_inlining`,
1436-
/// `with_logical_extension_codec` — report the same id even though their
1437-
/// codec chains differ, so installing two of them on one target is
1438-
/// refused. That is the intended answer: their payloads would be
1439-
/// indistinguishable on decode.
1438+
/// `with_logical_extension_codec`, [`Self::_install_extension_codecs`] —
1439+
/// report the same id even though their codec chains differ, so installing
1440+
/// two of them on one target is refused. That is the intended answer: they
1441+
/// share a `state_ref`, so their payloads would resolve against the same
1442+
/// session and are indistinguishable on decode. Every derivation shares the
1443+
/// session for exactly this reason; `enable_url_table` is the one that does
1444+
/// not, and it is tracked as a bug.
14401445
#[getter]
14411446
pub fn __datafusion_codec_id__(&self) -> String {
14421447
format!("{SESSION_CODEC_ID_PREFIX}{}", self.ctx.session_id())
@@ -1608,6 +1613,118 @@ impl PySessionContext {
16081613
derived.set_session_query_planner(None);
16091614
derived
16101615
}
1616+
1617+
/// Build the codec chains for a `with_extensions` call.
1618+
///
1619+
/// Private support method for `SessionContext.with_extensions`, and the
1620+
/// first of the two phases that method runs. `self` is the context the
1621+
/// extensions bound their components against, and is also the
1622+
/// `Arc<SessionContext>` every FFI task-context provider they created
1623+
/// targets, so the returned handle shares it rather than deriving a new
1624+
/// one.
1625+
///
1626+
/// **Writes nothing.** The codec chains belong to the returned handle
1627+
/// rather than to `SessionState`, so this phase is transactional for free:
1628+
/// a codec that fails to import, or that collides with an installed id,
1629+
/// leaves the caller's context exactly as it was. Binding the planner is
1630+
/// the only step that touches the session, and it is deferred to
1631+
/// [`Self::_install_extension_planner`] so the planner hooks can run
1632+
/// against the final chains.
1633+
///
1634+
/// Codecs must arrive as objects exposing the capsule getter, never as
1635+
/// bare capsules — see [`resolve_bundle_codec_id`].
1636+
pub fn _install_extension_codecs<'py>(
1637+
slf: &Bound<'py, Self>,
1638+
logical_codecs: Vec<Bound<'py, PyAny>>,
1639+
physical_codecs: Vec<Bound<'py, PyAny>>,
1640+
) -> PyDataFusionResult<Self> {
1641+
// Chains are built as local values, so a codec that fails to import --
1642+
// or that collides with an id already installed -- leaves the session
1643+
// untouched. Nothing is borrowed across a call back into Python.
1644+
let (mut logical_codec, mut physical_codec) = {
1645+
let this = slf.borrow();
1646+
(
1647+
this.logical_codec.as_ref().clone(),
1648+
this.physical_codec.as_ref().clone(),
1649+
)
1650+
};
1651+
1652+
for codec in logical_codecs {
1653+
let id = resolve_bundle_codec_id(
1654+
&codec,
1655+
"__datafusion_logical_extension_codec__",
1656+
&logical_codec.codec_ids(),
1657+
)?;
1658+
let inner_ffi = ffi_logical_codec_from_pycapsule(codec, Some(slf.as_any()))?;
1659+
let inner: Arc<dyn LogicalExtensionCodec> = (&inner_ffi).into();
1660+
logical_codec = logical_codec.with_additional_codec(id, inner);
1661+
}
1662+
let logical_codec = Arc::new(logical_codec);
1663+
1664+
for codec in physical_codecs {
1665+
let id = resolve_bundle_codec_id(
1666+
&codec,
1667+
"__datafusion_physical_extension_codec__",
1668+
&physical_codec.codec_ids(),
1669+
)?;
1670+
let inner_ffi = ffi_physical_codec_from_pycapsule(codec, Some(slf.as_any()))?;
1671+
let inner: Arc<dyn PhysicalExtensionCodec> = (&inner_ffi).into();
1672+
physical_codec = physical_codec.with_additional_codec(id, inner);
1673+
}
1674+
let physical_codec = Arc::new(physical_codec);
1675+
1676+
Ok(Self {
1677+
ctx: Arc::clone(&slf.borrow().ctx),
1678+
logical_codec,
1679+
physical_codec,
1680+
})
1681+
}
1682+
1683+
/// Re-export a planner a `__datafusion_session_planner__` hook returned as
1684+
/// a capsule, so the next hook in the chain receives one either way.
1685+
///
1686+
/// A hook may hand back an object exposing `__datafusion_query_planner__`
1687+
/// or a raw capsule; the next hook wraps whatever it is given and should
1688+
/// not have to branch on which. Importing here also surfaces a malformed
1689+
/// planner at the hook that produced it rather than at the final install.
1690+
/// Writes nothing.
1691+
pub fn _export_query_planner<'py>(
1692+
slf: &Bound<'py, Self>,
1693+
planner: Bound<'py, PyAny>,
1694+
) -> PyDataFusionResult<Bound<'py, PyCapsule>> {
1695+
let ffi = ffi_query_planner_from_pycapsule(&planner, Some(slf.as_any()))?;
1696+
Ok(create_query_planner_capsule(slf.py(), &ffi)?)
1697+
}
1698+
1699+
/// Commit the query planner for a `with_extensions` call.
1700+
///
1701+
/// The second phase, run once every codec is installed and every planner
1702+
/// hook has returned, so the planner is bound against the final chains.
1703+
/// This is the one call in `with_extensions` that writes to the session,
1704+
/// and it goes through this context's own `state_ref()`, so providers
1705+
/// bound to it stay valid.
1706+
///
1707+
/// `None` means no bundle supplied a planner. That still rebuilds
1708+
/// whichever planner the session already holds against the new chains,
1709+
/// exactly as `with_logical_extension_codec` does, and writes nothing at
1710+
/// all if the session has no FFI planner to rebuild.
1711+
///
1712+
/// The caller skips this step entirely when the call installed no codec
1713+
/// and no planner, the same way [`Self::with_python_udf_inlining`] returns
1714+
/// early for a no-op toggle: there is nothing to rebind against, and the
1715+
/// rebuild would drag a planner sitting on another handle's codecs onto
1716+
/// this one's.
1717+
#[pyo3(signature = (planner=None))]
1718+
pub fn _install_extension_planner<'py>(
1719+
slf: &Bound<'py, Self>,
1720+
planner: Option<Bound<'py, PyAny>>,
1721+
) -> PyDataFusionResult<()> {
1722+
let planner = planner
1723+
.map(|planner| ffi_query_planner_from_pycapsule(&planner, Some(slf.as_any())))
1724+
.transpose()?;
1725+
slf.borrow().set_session_query_planner(planner);
1726+
Ok(())
1727+
}
16111728
}
16121729

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

1932+
/// Resolve the wire id for a codec contributed through `with_extensions`,
1933+
/// requiring an object that can name itself.
1934+
///
1935+
/// `with_extensions` takes no `codec_id=`, so the only naming channels are the
1936+
/// ones [`derive_codec_id`] reads off the handed-over object: a declared
1937+
/// `__datafusion_codec_id__`, or its class's `module.QualName`. A bare capsule
1938+
/// has neither. Naming it after the bundle that contributed it looks like an
1939+
/// answer and is not one: the bundle is whatever object the caller passed to
1940+
/// `with_extensions`, so a bundle that wraps another library's bundle — the
1941+
/// natural way for an application to package several libraries as one — would
1942+
/// stamp its own identity onto the inner library's codecs and silently change
1943+
/// the wire format. The inner library cannot defend against that no matter what
1944+
/// it declares, and the mismatch does not surface until a plan fails to decode
1945+
/// in another process.
1946+
///
1947+
/// So the capsule is refused here, where the author can fix it by wrapping it
1948+
/// in an object. Wrapping also decouples the codec's wire identity from the
1949+
/// bundle's Python class name, which is the whole point of
1950+
/// `__datafusion_codec_id__`.
1951+
fn resolve_bundle_codec_id(
1952+
codec: &Bound<'_, PyAny>,
1953+
getter: &str,
1954+
existing: &[&str],
1955+
) -> PyResult<String> {
1956+
if codec.is_instance_of::<PyCapsule>() {
1957+
return Err(PyTypeError::new_err(format!(
1958+
"A codec contributed through `with_extensions` must be an object exposing \
1959+
`{getter}`, not a bare PyCapsule. A capsule carries no type of its own, so \
1960+
there is nothing to name the codec by, and a payload names its codec by id \
1961+
when it is decoded — an id that has to mean the same thing in whichever \
1962+
process decodes. Wrap the capsule in an object that exposes `{getter}` and, \
1963+
if the class name is not the identity you want on the wire, declares \
1964+
`__datafusion_codec_id__`."
1965+
)));
1966+
}
1967+
resolve_codec_id(codec, None, existing)
1968+
}
1969+
18081970
fn derive_codec_id(codec: &Bound<'_, PyAny>, explicit: Option<String>) -> PyResult<String> {
18091971
if let Some(id) = explicit {
18101972
return Ok(id);

0 commit comments

Comments
 (0)