(datafusion-python) 01/04: refactor: let _commit_extensions trust its caller's filter

[ti...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

timsaucer pushed a commit to branch feat/bundle-functions
in repository https://gitbox.apache.org/repos/asf/datafusion-python.git

commit 4f3089738a6bee357513a87f6a41745f5096bbb3
Author: Tim Saucer <ti...@gmail.com>
AuthorDate: Thu Oct 8 09:01:23 2026 -0400

    refactor: let _commit_extensions trust its caller's filter
    
    The Rust hook loop re-checked each entry with hasattr, which the Python
    caller had already narrowed with isinstance. The two disagree on an
    attribute set to None, so keeping both left a second, weaker predicate
    that only stayed harmless because the stronger one ran first. Rust now
    calls every entry it is handed, and the isinstance filter is the only
    check.
    
    Co-Authored-By: Claude Opus 5.5 (1M context) <no...@anthropic.com>
---
 crates/core/src/context.rs   | 14 ++++++++------
 python/tests/test_context.py |  7 +++----
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/crates/core/src/context.rs b/crates/core/src/context.rs
index 63b7a6c8..af627d5f 100644
--- a/crates/core/src/context.rs
+++ b/crates/core/src/context.rs
@@ -1720,9 +1720,14 @@ impl PySessionContext {
     ///
     /// The second phase, run on the handle carrying the completed chains —
     /// `session` is that same handle as the Python-level wrapper, which is
-    /// what each `__datafusion_session_planner__` hook receives. The hooks
-    /// run first, **in argument order**, each handed the planner built so
-    /// far as a capsule; a hook may hand back an object exposing
+    /// what each `__datafusion_session_planner__` hook receives.
+    ///
+    /// `extensions` must already be narrowed to the planner hooks: every entry
+    /// is called, with no check of its own. The Python caller decides which
+    /// arguments are hooks.
+    ///
+    /// The hooks run first, **in argument order**, each handed the planner
+    /// built so far as a capsule; a hook may hand back an object exposing
     /// `__datafusion_query_planner__` or a raw capsule, and each return is
     /// imported here so a malformed planner surfaces at the hook that
     /// produced it rather than at the install. Returning `None` contributes
@@ -1757,9 +1762,6 @@ impl PySessionContext {
         // place rather than wrapping the session's default in an FFI hop.
         let mut planner: Option<FFI_QueryPlanner> = None;
         for extension in &extensions {
-            if !extension.hasattr("__datafusion_session_planner__")? {
-                continue;
-            }
             let fallback = match &planner {
                 Some(ffi) => create_query_planner_capsule(py, ffi)?,
                 None => slf.borrow().__datafusion_query_planner__(py, None)?,
diff --git a/python/tests/test_context.py b/python/tests/test_context.py
index f9575785..580ccea6 100644
--- a/python/tests/test_context.py
+++ b/python/tests/test_context.py
@@ -1216,10 +1216,9 @@ def test_with_extensions_ignores_a_planner_attribute_set_to_none(ctx):
     with the other called the ``None``: ``'NoneType' object is not callable``,
     naming neither the extension nor the hook.
 
-    ``_commit_extensions`` still carries a Rust-side ``hasattr``, and the
-    Python caller now hands it a list already narrowed by ``isinstance``. Two
-    checks means one of them decides, and this pins which: a refactor that
-    passes the unfiltered arguments through reintroduces the failure, and
+    ``_commit_extensions`` calls every entry it is handed, so the
+    ``isinstance`` filter in the Python caller is the only check. A refactor
+    that passes the unfiltered arguments through reintroduces the failure, and
     nothing else in this file notices.
     """
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org