Conversation
timsaucer
added this pull request to stack #1742
September 15, 2026 14:28
timsaucer
marked this pull request as draft
September 15, 2026 14:28
This was referenced Sep 15, 2026
`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>
timsaucer
force-pushed
the
feat/bundle-catalogs
branch
from
September 15, 2026 20:04
04a9650 to
6f56b5f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #1676.
Part 4 of 4.
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_providerdid 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 ofregister_catalog_providermoves to a private helper that both it and_resolve_extension_catalogscall, so the two paths accept exactly the same shapes — an object exposing__datafusion_catalog_provider__, a bare capsule, aPyCatalog, or a Python object implementing the provider interface.PyCatalog::newwas not a substitute: it wraps unconditionally inRustWrappedPyCatalogProviderand 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_catalogreturns whichever provider it displaced rather than refusing, and the defaultdatafusioncatalog 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.mdstates the contrast where a reader meets it._install_extension_catalogsreturns(). Once the providers are imported there is nothing left that can fail.The capstone test.
test_four_libraries_install_in_one_callinstalls four independently declared bundles — functions, rules, a table plus a table function, a catalog — in onewith_extensionscall, 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.mdnow carries the split this stack is built on: which components a bundle hands over unwrapped and why, pointing atRecordingTableFunctionas 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 commit step returning(), 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_provideris 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 noapi changelabel.Taking the stack as a whole,
SessionExtensionComponentsgrows from two fields to nine, all optional and all additive, andPhysicalOptimizerRuleExportablebecomes importable fromdatafusionas well asdatafusion.context.Review notes
The replace-vs-refuse asymmetry is inherited, not chosen. Both rules are DataFusion's. If you would rather
with_extensionsnormalized them — refusing both, or replacing both — that is a defensible position, but it would mean the bundle path behaving differently from theregister_*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_extensionsstill cannot install everything a library provides. A config extension has to reachSessionConfigbefore the context exists, which no bundle hook can reach —dfx_enginein #1721 needs exactly that, and itsbuild_sessionwill still have a step for it.Follow-up, once #1721 and this stack have both landed.
dfx_udfsdeliberately ships no bundle hook and carriestest_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 touchesexamples/distributed/.🤖 Generated with Claude Code