Skip to content

Let extension bundles declare catalog providers - #1741

Draft
timsaucer wants to merge 4 commits into
feat/bundle-tablesfrom
feat/bundle-catalogs
Draft

timsaucer wants to merge 4 commits into
feat/bundle-tablesfrom
feat/bundle-catalogs

Conversation

@timsaucer

@timsaucer timsaucer commented Sep 15, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #1676.

Part 4 of 4.

  1. scalar, aggregate, and window functions
  2. physical optimizer rules
  3. tables and table functions
  4. This PR -> catalog providers

Rationale for this change

The last field, and the last of the three getters that take the session. It also removes a small structural wart: register_catalog_provider did its capsule import and its insert in one function with no split point, so the bundle path could not reuse it without duplicating the four-way branch that decides what a caller actually handed over.

What changes are included in this PR?

SessionExtensionComponents.catalog_providers, as (name, provider) pairs.

resolve_catalog_provider, shared. The import half of register_catalog_provider moves to a private helper that both it and _resolve_extension_catalogs call, so the two paths accept exactly the same shapes — an object exposing __datafusion_catalog_provider__, a bare capsule, a PyCatalog, or a Python object implementing the provider interface. PyCatalog::new was not a substitute: it wraps unconditionally in RustWrappedPyCatalogProvider and never checks for a capsule, so routing the bundle path through it would have silently bypassed the FFI fast path.

Catalogs replace; tables do not. This is the one place in the stack where two neighbouring fields take opposite rules, so it is worth being explicit. register_catalog returns whichever provider it displaced rather than refusing, and the default datafusion catalog always exists — so replacing one is the usual way a library backs a session with its own metadata, and refusing it would refuse the main use case. A duplicate table, by contrast, is an error, because DataFusion refuses one. Only two bundles claiming the same catalog name within a single call is refused, with both named. table-providers.md states the contrast where a reader meets it.

The catalogs commit is a _commit_extensions parameter. Once the providers are imported there is nothing left that can fail, so the commit step needs no primitive of its own.

The capstone test. test_four_libraries_install_in_one_call installs four independently declared bundles — functions, rules, a table plus a table function, a catalog — in one with_extensions call, then runs a single query that reads the declared table and the declared catalog through the declared scalar function, and asserts both optimizer rules fired while planning it. That is the thing #1676 asked for, exercised across a real FFI boundary.

Skill update. Rule 2 of .ai/skills/ffi-capsule-protocol/SKILL.md now carries the split this stack is built on: which components a bundle hands over unwrapped and why, pointing at RecordingTableFunction as the assertion rather than the description. Rule 6 gains the other convention — a new kind of component means a new resolve step returning an opaque carrier and a new _commit_extensions parameter whose commit cannot fail, never a fallible commit — along with the single table exception and a note not to add a second without the same justification.

Are there any user-facing changes?

One new optional field on SessionExtensionComponents, defaulting to (). register_catalog_provider is refactored but behaviourally identical — same accepted shapes, same result. No hook signatures change and no wire format changes, so there is no upgrade-guide entry and no api change label.

Taking the stack as a whole, SessionExtensionComponents grows from two fields to nine, all optional and all additive, and PhysicalOptimizerRuleExportable becomes importable from datafusion as well as datafusion.context.

Review notes

The replace-vs-refuse asymmetry is inherited, not chosen. Both rules are DataFusion's. If you would rather with_extensions normalized them — refusing both, or replacing both — that is a defensible position, but it would mean the bundle path behaving differently from the register_* method it is meant to replace, which seems worse than the asymmetry.

What this stack does not do, stated once so it is on the record: with_extensions still cannot install everything a library provides. A config extension has to reach SessionConfig before the context exists, which no bundle hook can reach — dfx_engine in #1721 needs exactly that, and its build_session will still have a step for it.

Follow-up, once #1721 and this stack have both landed. dfx_udfs deliberately ships no bundle hook and carries test_this_library_cannot_be_installed_as_a_bundle, whose premise #1738 inverts. Whichever lands second should convert it and flip that test. Nothing in this stack touches examples/distributed/.

🤖 Generated with Claude Code

timsaucer and others added 4 commits September 16, 2026 14:02
`SessionExtensionComponents.catalog_providers` completes the set #1676 asked
for, minus object stores, which have no FFI type upstream and are tracked
separately in #1737.

`register_catalog_provider` had its import and its insert in one function with
no split point, so the import half moves to a shared `resolve_catalog_provider`
that both it and the new `_resolve_extension_catalogs` call. Both therefore
accept exactly the same shapes, and the bundle path gets the same treatment as
tables: imported against the handle carrying the finished codec chains, since
the getter is handed the logical codec its provider will serialize through.

Catalogs replace rather than collide. `register_catalog` returns whichever
provider it displaced, and the default `datafusion` catalog always exists, so a
library backing a session with its own metadata has to be able to replace one.
Only two bundles claiming a name in the same call is refused. That is the
opposite of tables, where a duplicate is an error, and both now say so where a
reader meets them.

`_install_extension_catalogs` returns `()`: nothing is left that can fail once
the providers are imported.

Rules 2 and 6 of the capsule-protocol skill now carry the two conventions the
stack established — which components a bundle hands over unwrapped and why, and
that a new component means a new resolve step rather than a fallible commit
step.

Closes #1676.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The same repair the tables got: a resolve failure surfaced the
importer's message, which names neither the catalog nor the bundle.
Wrapping it with the declared name — unique within the call — points at
one declaration. For a catalog the failure that remains is a capsule of
the wrong kind: a non-capsule return falls through to the duck-typed
wrap, the same fall-through every getter in the family has.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Nothing mutates it between resolve and commit, so there is no reason to
pay for the runtime borrow flag a mutable pyclass carries — the same
reasoning that froze PhysicalOptimizerRules and ResolvedTables.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Catalogs resolve alongside everything else and commit with the rest;
register_catalog replaces rather than refuses, so their commit step
cannot fail and needs no exception of its own.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extend SessionContext.with_extensions to cover additional extension points (functions, catalogs, object stores)

1 participant