Skip to content

Commit b22387f

Browse files
timsaucerclaude
andcommitted
docs: note that installing a rule drops prepared statements
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>
1 parent e125321 commit b22387f

5 files changed

Lines changed: 55 additions & 9 deletions

File tree

‎crates/core/src/context.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1861,7 +1861,15 @@ impl PySessionContext {
18611861
/// to carry imported rules from the resolve step to the commit step, so the
18621862
/// import can fail before anything is written. `with_extensions` is its only
18631863
/// producer and its only consumer.
1864-
#[pyclass(name = "PhysicalOptimizerRules", module = "datafusion._internal")]
1864+
///
1865+
/// `frozen` because nothing mutates it between those two steps: the commit
1866+
/// only reads the rules back out, so there is no reason to pay for the runtime
1867+
/// borrow flag a mutable pyclass carries.
1868+
#[pyclass(
1869+
frozen,
1870+
name = "PhysicalOptimizerRules",
1871+
module = "datafusion._internal"
1872+
)]
18651873
pub struct PyPhysicalOptimizerRules {
18661874
rules: Vec<Arc<dyn PhysicalOptimizerRule + Send + Sync>>,
18671875
}

‎docs/source/extension-guide/other-components.md‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,32 @@ a bundle contributing several would clone the whole state that many times, and
6363
would leave the earlier ones installed if a later one failed. See
6464
{ref}`extension_bundles_transaction`.
6565

66+
(extension_rule_rebuild)=
67+
68+
### Installing a rule rebuilds the session state
69+
70+
There is no way to append to a live `SessionState`: DataFusion exposes
71+
`physical_optimizers` on one read-only, and only `SessionStateBuilder` can add
72+
to the list. Installing a rule therefore rebuilds the state in place, and the
73+
rebuild carries over the tables, functions, catalogs, and session id the old
74+
one held.
75+
76+
**Prepared statements are the exception.** `SessionStateBuilder::build` starts
77+
the new state with an empty prepared-plan map, so a session that has run
78+
`PREPARE` reports the statement missing once a rule is installed:
79+
80+
```python
81+
ctx.sql("PREPARE p AS SELECT a FROM t").collect()
82+
ctx.with_extensions(MyRuleBundle())
83+
ctx.sql("EXECUTE p") # ValueError: Prepared statement 'p' does not exist
84+
```
85+
86+
This is not specific to bundles — `add_physical_optimizer_rule` drops them the
87+
same way, and both hit every handle sharing the session rather than only the
88+
one the call returned. Install your rules before preparing anything. Batching a
89+
bundle's rules into one rebuild is what keeps the cost to once per call instead
90+
of once per rule.
91+
6692
## Typed configuration
6793

6894
**`__datafusion_extension_options__`** contributes typed configuration entries

‎examples/datafusion-ffi-example/python/tests/_test_session_extension.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,13 +139,17 @@ def test_declared_rules_run_in_declaration_order():
139139
Rules rewrite the plan one after another, so the order is part of what a
140140
bundle declares. The counters cannot show it — each rule has its own — so
141141
the two here append to a log they share.
142+
143+
Only the first pass is asserted on. How many times a query optimizes is a
144+
separate claim from what order the rules run in, and pinning both here
145+
would report a changed pass count as an ordering bug.
142146
"""
143147
extension = MyRuleExtension()
144148
ctx = SessionContext().with_extensions(extension)
145149

146150
_query(ctx)
147151

148-
assert extension.run_order() == [0, 1]
152+
assert extension.run_order()[:2] == [0, 1]
149153

150154

151155
def test_rules_install_without_changing_the_session_id():

‎python/datafusion/context.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2051,7 +2051,8 @@ def add_physical_optimizer_rule(
20512051
PyCapsule, typically produced by a separate compiled extension. The
20522052
underlying :class:`SessionState` is rebuilt from its current state
20532053
with the new rule appended, so previously registered tables, UDFs,
2054-
and catalogs are preserved.
2054+
and catalogs are preserved. Prepared statements are not — see
2055+
:ref:`extension_rule_rebuild`.
20552056
20562057
Args:
20572058
rule: Object exposing ``__datafusion_physical_optimizer_rule__``,
@@ -2147,6 +2148,10 @@ def with_extensions(
21472148
table, say — is not rolled back, which is why bundle objects must be
21482149
configuration-only.
21492150
2151+
A call that installs optimizer rules rebuilds the session state, which
2152+
drops the session's prepared statements — see
2153+
:ref:`extension_rule_rebuild`.
2154+
21502155
Shares its session with this context — see :py:class:`SessionContext`.
21512156
21522157
See :ref:`extension_bundles` in the online documentation for why the

‎python/tests/test_context.py‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1725,12 +1725,15 @@ def __datafusion_session_components__(self, ctx):
17251725

17261726

17271727
def test_with_extensions_declaring_no_rules_leaves_the_session_id(ctx):
1728-
"""Installing rules rebuilds ``SessionState``; the id has to survive it.
1729-
1730-
The rebuild mints a fresh id unless it is carried over, and a changed id
1731-
would break every ``TaskContext`` the session has handed out. Asserted for
1732-
a call declaring no rules as well, so the guarantee does not depend on
1733-
whether the rebuild was skipped.
1728+
"""A call declaring no rules leaves the session id alone.
1729+
1730+
This is the control for the no-op path: with nothing to install the state
1731+
rebuild is skipped, so the id is untouched rather than carried over. The
1732+
carry-over itself is not reachable from here — the rebuild needs a real
1733+
rule capsule, which only a compiled extension can hand over. That half is
1734+
pinned by ``test_rules_install_without_changing_the_session_id`` in
1735+
``datafusion-ffi-example``, where a fresh id would leave ``session_id()``
1736+
disagreeing with every ``TaskContext`` the session has handed out.
17341737
"""
17351738
before = ctx.session_id()
17361739
result = ctx.with_extensions(_FunctionExtension(udfs=(_doubler(),)))

0 commit comments

Comments
 (0)