Skip to content

Commit 0868f39

Browse files
timsaucerclaude
andcommitted
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>
1 parent 7b6193e commit 0868f39

3 files changed

Lines changed: 9 additions & 9 deletions

File tree

‎crates/core/src/context.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,11 +1435,11 @@ impl PySessionContext {
14351435
/// decode. See [`SESSION_CODEC_ID_PREFIX`].
14361436
///
14371437
/// Handles derived from one session — `with_python_udf_inlining`,
1438-
/// `with_logical_extension_codec`, `_install_extensions` — report the same
1439-
/// id even though their codec chains differ, so installing two of them on
1440-
/// one target is refused. That is the intended answer: they share a
1441-
/// `state_ref`, so their payloads would resolve against the same session
1442-
/// and are indistinguishable on decode. Every derivation shares the
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
14431443
/// session for exactly this reason; `enable_url_table` is the one that does
14441444
/// not, and it is tracked as a bug.
14451445
#[getter]

‎python/datafusion/context.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1812,7 +1812,7 @@ def set_query_planner(self, planner: QueryPlannerExportable | _PyCapsule) -> Non
18121812
self.ctx.set_query_planner(planner)
18131813

18141814
def with_extensions(
1815-
self, *extensions: SessionExtensionExportable
1815+
self, *extensions: SessionExtensionExportable | SessionPlannerExportable
18161816
) -> SessionContext:
18171817
"""Create a new session context with the given extension bundles.
18181818

‎python/tests/test_context.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -971,9 +971,9 @@ def test_with_extensions_accepts_no_extensions(ctx):
971971
def test_with_extensions_no_extensions_keeps_an_installed_planner(ctx):
972972
"""The empty case must not disturb a planner the session already has.
973973
974-
Phase two still runs with nothing to install, which rebinds an existing
975-
FFI planner to the codec chains — unchanged here, so the planner has to
976-
come through intact.
974+
A call that installs no codec and no planner skips the planner commit
975+
entirely, so the installed planner keeps the chains it was bound to and the
976+
session still plans through it.
977977
"""
978978
extension = _CodecOnlyExtension()
979979
installed = ctx.with_extensions(extension, _PlannerExtension())

0 commit comments

Comments
 (0)