From af65a814f5e1da23f0914ca85de46f8a00844753 Mon Sep 17 00:00:00 2001 From: Thomas Gschwind Date: Thu, 1 Oct 2026 10:05:02 +0200 Subject: [PATCH 1/2] fix: load default extensions using iterdir() instead of Traversable.glob() ExtensionRegistry.__init__ loaded the bundled extension YAMLs with `importlib.resources.files("substrait_extensions.extensions").glob("functions*.yaml")`. `importlib.resources.files` returns a `Traversable`, which may not be a filesystem `Path`. When substrait_extensions.extensions resolves as a namespace package (no `__init__.py`), CPython's resource reader returns a `MultiplexedPath` that implements `iterdir`/`open`/`joinpath` but is missing `glob` raising: AttributeError: 'MultiplexedPath' object has no attribute 'glob' This occured under the pure-Python CPython-on-WASI interpreter, where `files(...)` returns a `MultiplexedPath` for the namespace package even for a single directory. Iterate with `iterdir()` and filter by name instead. `iterdir()` is part of the `Traversable` protocol and works across `Path`, `MultiplexedPath`, and zip-based readers alike. Add a regression test that forces `files()` to return a `MultiplexedPath` and asserts default extensions still load. --- src/substrait/extension_registry/registry.py | 16 ++++++-- .../test_multiplexed_path.py | 41 +++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 tests/extension_registry/test_multiplexed_path.py diff --git a/src/substrait/extension_registry/registry.py b/src/substrait/extension_registry/registry.py index 7eae5a0..b0a2195 100644 --- a/src/substrait/extension_registry/registry.py +++ b/src/substrait/extension_registry/registry.py @@ -34,10 +34,18 @@ def __init__(self, load_default_extensions=True) -> None: # extension relation's output schema can be derived during inference. self._extension_relations: dict = {} if load_default_extensions: - for fpath in importlib_files("substrait_extensions.extensions").glob( # type: ignore - "functions*.yaml" - ): - self.register_extension_yaml(fpath) + # NB: iterate + filter instead of ``.glob("functions*.yaml")``. + # ``importlib.resources.files`` returns a ``Traversable``, which is + # not guaranteed to be a filesystem path: when + # ``substrait_extensions.extensions`` resolves as a namespace + # package the reader returns a ``MultiplexedPath``, and that type + # implements ``iterdir``/``open``/``joinpath`` but not ``glob`` + # (calling ``.glob`` raises ``AttributeError``). ``iterdir`` is part + # of the ``Traversable`` protocol and works across ``Path``, + # ``MultiplexedPath``, and zip-based readers alike. + for fpath in importlib_files("substrait_extensions.extensions").iterdir(): + if fpath.name.startswith("functions") and fpath.name.endswith(".yaml"): + self.register_extension_yaml(fpath) def register_extension_yaml( self, diff --git a/tests/extension_registry/test_multiplexed_path.py b/tests/extension_registry/test_multiplexed_path.py new file mode 100644 index 0000000..71804dd --- /dev/null +++ b/tests/extension_registry/test_multiplexed_path.py @@ -0,0 +1,41 @@ +"""Regression test for loading default extensions via a ``MultiplexedPath``. + +``importlib.resources.files`` returns a ``Traversable``, not necessarily a +filesystem ``Path``. When ``substrait_extensions.extensions`` resolves as a +namespace package, CPython's resource reader returns a ``MultiplexedPath`` -- +which implements ``iterdir``/``open``/``joinpath`` but *not* ``glob``. The +registry used to call ``files(...).glob("functions*.yaml")``, so in that case it +raised ``AttributeError: 'MultiplexedPath' object has no attribute 'glob'`` +(observed under the pure-Python WASI guest interpreter). It now iterates with +``iterdir()`` and filters by name, which works for any ``Traversable``. +""" + +from importlib.resources import files +from importlib.resources.readers import MultiplexedPath + +import substrait.extension_registry.registry as registry_module +from substrait.builders.type import i8 +from substrait.extension_registry import ExtensionRegistry + + +def test_load_default_extensions_via_multiplexed_path(monkeypatch): + # Wrap the real extensions directory in a MultiplexedPath so ``files()`` + # yields exactly what it would for a namespace package. Guard the premise: + # MultiplexedPath must not expose ``glob`` (otherwise this test proves + # nothing). + ext_dir = files("substrait_extensions.extensions") + multiplexed = MultiplexedPath(ext_dir) + assert not hasattr(multiplexed, "glob") + + monkeypatch.setattr(registry_module, "importlib_files", lambda _pkg: multiplexed) + + # Previously raised AttributeError here; must now load cleanly. + reg = ExtensionRegistry(load_default_extensions=True) + + # The default extension set was parsed and registered. + assert reg._function_mapping + assert reg.lookup_function( + urn="extension:io.substrait:functions_arithmetic", + function_name="add", + signature=[i8(nullable=False), i8(nullable=False)], + ) From 27f92d3b4d8039b7a29758c7abe4ac423626dac0 Mon Sep 17 00:00:00 2001 From: Thomas Gschwind Date: Thu, 1 Oct 2026 10:56:25 +0200 Subject: [PATCH 2/2] fix(extension_registry): CI review follow-ups (py3.10 import, zip paths) Address CodeRabbit findings on the default-extension loading fix: - Import MultiplexedPath from importlib.readers on Python 3.10, where importlib.resources.readers does not exist. The test module failed to import with 3.10 with ModuleNotFoundError. Guard the import with a try/except so it resolves on 3.10-3.13. - Materialize each resource with importlib.resources.as_file before passing it to register_extension_yaml. iterdir() now succeeds on a zipfile.Path, which newly reaches register_extension_yaml's Path(fname). as_file is a no-op for on-disk resources and extracts zip-backed ones to a temp file, so loading works regardless of how substrait_extensions is imported. Add a zipfile.Path regression test alongside the MultiplexedPath one. --- src/substrait/extension_registry/registry.py | 10 ++- .../test_multiplexed_path.py | 64 ++++++++++++++----- 2 files changed, 57 insertions(+), 17 deletions(-) diff --git a/src/substrait/extension_registry/registry.py b/src/substrait/extension_registry/registry.py index b0a2195..2ba813f 100644 --- a/src/substrait/extension_registry/registry.py +++ b/src/substrait/extension_registry/registry.py @@ -2,6 +2,7 @@ import re from collections import defaultdict +from importlib.resources import as_file from importlib.resources import files as importlib_files from pathlib import Path from typing import Optional, Union @@ -43,9 +44,16 @@ def __init__(self, load_default_extensions=True) -> None: # (calling ``.glob`` raises ``AttributeError``). ``iterdir`` is part # of the ``Traversable`` protocol and works across ``Path``, # ``MultiplexedPath``, and zip-based readers alike. + # + # ``as_file`` turns each entry into a real filesystem path for the + # duration of the ``with`` block (a no-op for on-disk resources, an + # extraction to a temp file for zip-backed ones). Without it, + # ``register_extension_yaml`` -> ``Path(fname)`` would raise + # ``TypeError`` on a ``zipfile.Path``. for fpath in importlib_files("substrait_extensions.extensions").iterdir(): if fpath.name.startswith("functions") and fpath.name.endswith(".yaml"): - self.register_extension_yaml(fpath) + with as_file(fpath) as real_path: + self.register_extension_yaml(real_path) def register_extension_yaml( self, diff --git a/tests/extension_registry/test_multiplexed_path.py b/tests/extension_registry/test_multiplexed_path.py index 71804dd..4038ba7 100644 --- a/tests/extension_registry/test_multiplexed_path.py +++ b/tests/extension_registry/test_multiplexed_path.py @@ -1,22 +1,41 @@ -"""Regression test for loading default extensions via a ``MultiplexedPath``. +"""Regression tests for loading default extensions off a non-``Path`` resource. ``importlib.resources.files`` returns a ``Traversable``, not necessarily a -filesystem ``Path``. When ``substrait_extensions.extensions`` resolves as a -namespace package, CPython's resource reader returns a ``MultiplexedPath`` -- -which implements ``iterdir``/``open``/``joinpath`` but *not* ``glob``. The -registry used to call ``files(...).glob("functions*.yaml")``, so in that case it -raised ``AttributeError: 'MultiplexedPath' object has no attribute 'glob'`` -(observed under the pure-Python WASI guest interpreter). It now iterates with -``iterdir()`` and filters by name, which works for any ``Traversable``. +filesystem ``Path``. The registry must load the bundled extension YAMLs +regardless of which concrete ``Traversable`` the resource reader hands back: + +* ``MultiplexedPath`` -- returned for a *namespace* package (no ``__init__.py``). + It implements ``iterdir``/``open``/``joinpath`` but *not* ``glob``, so the old + ``files(...).glob("functions*.yaml")`` raised + ``AttributeError: 'MultiplexedPath' object has no attribute 'glob'`` (observed + under the pure-Python WASI guest interpreter). +* ``zipfile.Path`` -- returned for a zip-imported package. It is not an + ``os.PathLike``, so ``register_extension_yaml``'s ``Path(fname)`` raised + ``TypeError``; ``as_file`` now materialises a real path first. """ +import zipfile from importlib.resources import files -from importlib.resources.readers import MultiplexedPath import substrait.extension_registry.registry as registry_module from substrait.builders.type import i8 from substrait.extension_registry import ExtensionRegistry +try: # Python 3.11+ + from importlib.resources.readers import MultiplexedPath +except ModuleNotFoundError: # Python 3.10 + from importlib.readers import MultiplexedPath + + +def _assert_defaults_loaded(reg: ExtensionRegistry) -> None: + """The default extension set was parsed and registered.""" + assert reg._function_mapping + assert reg.lookup_function( + urn="extension:io.substrait:functions_arithmetic", + function_name="add", + signature=[i8(nullable=False), i8(nullable=False)], + ) + def test_load_default_extensions_via_multiplexed_path(monkeypatch): # Wrap the real extensions directory in a MultiplexedPath so ``files()`` @@ -31,11 +50,24 @@ def test_load_default_extensions_via_multiplexed_path(monkeypatch): # Previously raised AttributeError here; must now load cleanly. reg = ExtensionRegistry(load_default_extensions=True) + _assert_defaults_loaded(reg) - # The default extension set was parsed and registered. - assert reg._function_mapping - assert reg.lookup_function( - urn="extension:io.substrait:functions_arithmetic", - function_name="add", - signature=[i8(nullable=False), i8(nullable=False)], - ) + +def test_load_default_extensions_via_zipfile_path(monkeypatch, tmp_path): + # Mirror a zip-imported install: copy the real YAMLs into a zip and resolve + # ``files()`` to a zipfile.Path over the archive. The entries are not + # os.PathLike, so register_extension_yaml's ``Path(fname)`` would raise + # TypeError without the ``as_file`` materialisation. + ext_dir = files("substrait_extensions.extensions") + archive = tmp_path / "substrait_extensions.zip" + with zipfile.ZipFile(archive, "w") as zf: + for child in ext_dir.iterdir(): + if child.name.endswith(".yaml"): + zf.writestr(f"extensions/{child.name}", child.read_bytes()) + + zip_dir = zipfile.Path(archive, "extensions/") + monkeypatch.setattr(registry_module, "importlib_files", lambda _pkg: zip_dir) + + # Previously raised TypeError in register_extension_yaml; must now load. + reg = ExtensionRegistry(load_default_extensions=True) + _assert_defaults_loaded(reg)