Conversation
This was referenced Sep 15, 2026
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
timsaucer
force-pushed
the
feat/bundle-optimizer-rules
branch
from
September 15, 2026 20:04
74edaff to
3a24768
Compare
timsaucer
marked this pull request as ready for review
September 16, 2026 14:44
`SessionExtensionComponents.physical_optimizer_rules` completes the group of components whose capsule getter takes no argument, so a library shipping a rule alongside anything else no longer asks the caller for a separate `add_physical_optimizer_rule` call. Rules are the one kind with no collision rule: they accumulate rather than replace, so two bundles contributing one each is the normal case and there is nothing to refuse. Installing them splits across the two new private primitives `_resolve_extension_physical_optimizer_rules` and `_install_extension_physical_optimizer_rules`, keeping the commit step infallible: the capsules are imported during resolution, so a rule that fails to import cannot leave the session with a planner already bound. All the rules in one call go on in a single `SessionState` rebuild. `add_physical_optimizer_rule` rebuilds per call, which for a bundle with several would clone the whole state that many times and leave the earlier ones installed if a later one failed. The session id is carried across the rebuild for the same reason that method carries it. `PhysicalOptimizerRuleExportable` moves from `datafusion.context` to `datafusion.extensions`, alongside the rest of the protocol family, and is now exported from the package root. It stays importable from `datafusion.context`. `MyRuleExtension` in `datafusion-ffi-example` declares two rules, which is what makes accumulation observable — each carries its own counter and both fire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A rule the importer refuses used to raise from Rust with nothing but the
capsule name. A caller who passed four bundles could not tell which one was at
fault, and the resolve step is the last place that is known.
`_resolve_declared_rules` looks for the getter in Python first, mirroring what
`_resolve_declared_functions` already does for a declared function:
TypeError A declared optimizer rule must expose
__datafusion_physical_optimizer_rule__, got <object ...> from
<RuleExtension ...>
Scoped to match the sibling rather than to go past it. A getter that is present
but returns a non-capsule still falls through to the importer's `RuntimeError`,
exactly as a declared function does, and `with_extensions` now documents that
case instead of listing only the two errors it raises itself.
`MyRuleExtension`'s two rules append to a run log they share, so the order they
installed in is observable. The counters cannot show it: each rule has its own,
so they say how often a rule ran but not when.
`ffi-internals.md` describes the four-step commit order as a rule for the next
field added to `SessionExtensionComponents`. `physical_optimizer_rules` is that
field, so steps three and four name it rather than leaving the enumeration
stale on the commit that invoked it.
Why rules never collide is now argued once, in the extension guide. The
protocol docstring and the field docstring state it and link there, and the
docstring naming the test that runs its skipped example names the whole test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Installing a physical optimizer rule rebuilds the session state, and `SessionStateBuilder::build` starts the new state with an empty prepared-plan map, so a session that has run PREPARE reports the statement missing afterwards. It hits every handle sharing the session, not just the one a call returned. There is no fix in this repo. `SessionState` exposes `physical_optimizers` read-only and only the builder can append to it, so the rebuild is the only public path, and `prepared_plans` has no builder setter. The canonical home for the claim is a new `extension_rule_rebuild` section in the extension guide. `add_physical_optimizer_rule` and `with_extensions` each state it in one sentence and link there. The enumeration already on `add_physical_optimizer_rule` -- "tables, UDFs, and catalogs are preserved" -- was incomplete and now names the exception. Three smaller corrections alongside it: `test_with_extensions_declaring_no_rules_leaves_the_session_id` claimed to pin the id surviving the rebuild, but it declares no rules, so the rebuild is skipped and the assertion cannot reach that guarantee. Its docstring now says it is the no-op control and names the FFI test that does cover the rebuild. `test_declared_rules_run_in_declaration_order` asserted `run_order() == [0, 1]`, which ties the ordering claim to the optimizer running exactly once per query -- a count its sibling FFI test deliberately avoids asserting. It now checks the first pass only. `PyPhysicalOptimizerRules` is `frozen`. Nothing mutates it between the resolve and commit steps, so it has no reason to carry a borrow flag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rules keep their separate resolve step -- importing the capsules must stay fallible-and-early -- but the install half was a second private pymethod whose only caller was the line after `_commit_extensions`. Make it a parameter instead: the commit stays a single call, the rules go on in the same one-rebuild block, and the boundary does not grow an installer per component kind. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
timsaucer
force-pushed
the
feat/bundle-optimizer-rules
branch
from
September 16, 2026 18:01
c7c4bbf to
bd43d97
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
1 participant
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?
Part 2 of 4 toward #1676.
Rationale for this change
#1679 added support for an easy way for users to add Python extensions with a single call to
SessionContext.with_extensions(). This supported codecs and query planners. Then in #1738 we extend this function to support scalar, aggregate, and window functions. This PR adds the next feature, physical optimizer rules.What changes are included in this PR?
SessionExtensionComponents.physical_optimizer_rulesobjects exposing__datafusion_physical_optimizer_rule__.PhysicalOptimizerRuleExportablemoves todatafusion.extensions._resolve_extension_physical_optimizer_rules); the commit is a new parameter on_commit_extensions, where all the rules go on in oneSessionStaterebuild.Are there any user-facing changes?
There are new additions to the
with_extensions()calls, but none have been prior released so this is all new code and not impacting to existing users.