From 7a73a0154f09bda91dcbebbacf34da52b9b5a0ad Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Fri, 3 Jul 2026 01:28:57 +1000 Subject: [PATCH 01/11] feat(namespace): add Namespace value object + public exports (DEBT-10) --- src/mountainash_data/__init__.py | 3 ++ src/mountainash_data/core/__init__.py | 5 +++ src/mountainash_data/core/namespace.py | 54 +++++++++++++++++++++++ tests/test_unit/core/test_namespace.py | 60 ++++++++++++++++++++++++++ 4 files changed, 122 insertions(+) create mode 100644 src/mountainash_data/core/namespace.py create mode 100644 tests/test_unit/core/test_namespace.py diff --git a/src/mountainash_data/__init__.py b/src/mountainash_data/__init__.py index a690c51..31d189c 100644 --- a/src/mountainash_data/__init__.py +++ b/src/mountainash_data/__init__.py @@ -15,6 +15,7 @@ NamespaceInfo, TableInfo, ) +from mountainash_data.core.namespace import Namespace, NamespaceLike from mountainash_data.backends.ibis.backend import IbisBackend try: @@ -29,6 +30,8 @@ "ColumnInfo", "NamespaceInfo", "TableInfo", + "Namespace", + "NamespaceLike", "IbisBackend", "IcebergBackend", ] diff --git a/src/mountainash_data/core/__init__.py b/src/mountainash_data/core/__init__.py index e69de29..b075e6e 100644 --- a/src/mountainash_data/core/__init__.py +++ b/src/mountainash_data/core/__init__.py @@ -0,0 +1,5 @@ +"""mountainash_data.core — protocol, inspection model, and the Namespace value object.""" + +from mountainash_data.core.namespace import Namespace, NamespaceLike + +__all__ = ["Namespace", "NamespaceLike"] diff --git a/src/mountainash_data/core/namespace.py b/src/mountainash_data/core/namespace.py new file mode 100644 index 0000000..487bfb6 --- /dev/null +++ b/src/mountainash_data/core/namespace.py @@ -0,0 +1,54 @@ +"""Backend-agnostic table location — the Namespace value object. + +A location has two dimensions kept in NAMED fields so nothing is inferred +from tuple position: `path` (the schema/namespace levels between catalog and +table) and `catalog` (the top-level catalog, always explicit). Rendering to a +backend's native form lives with each backend, never here. +""" + +from __future__ import annotations + +from dataclasses import dataclass +import typing as t + +NamespaceLike = t.Union["Namespace", str, tuple, None] + + +@dataclass(frozen=True) +class Namespace: + """A backend-agnostic table location. + + `path` is the schema/namespace path (the levels between catalog and table). + `catalog` is the top-level catalog, always explicit — never encoded by tuple + position. `None`/empty mean "the connection's current/default". + """ + + path: tuple[str, ...] = () + catalog: t.Optional[str] = None + + def __post_init__(self) -> None: + if any(not isinstance(p, str) or p == "" for p in self.path): + raise ValueError( + f"Namespace.path segments must be non-empty strings: {self.path!r}" + ) + + @property + def is_default(self) -> bool: + return not self.path and self.catalog is None + + @property + def dotted(self) -> str: + """Human-readable `catalog.level1.level2` (for messages/logging only).""" + return ".".join(p for p in ((self.catalog,) + self.path) if p) + + @classmethod + def coerce(cls, value: NamespaceLike) -> "Namespace": + if value is None: + return cls() + if isinstance(value, Namespace): + return value + if isinstance(value, str): + return cls(path=(value,)) + if isinstance(value, tuple): + return cls(path=value) + raise TypeError(f"Cannot coerce {value!r} to Namespace") diff --git a/tests/test_unit/core/test_namespace.py b/tests/test_unit/core/test_namespace.py new file mode 100644 index 0000000..0fa83aa --- /dev/null +++ b/tests/test_unit/core/test_namespace.py @@ -0,0 +1,60 @@ +"""Tests for the Namespace value object (core.namespace).""" + +import pytest + +from mountainash_data.core.namespace import Namespace + + +class TestCoercion: + def test_none_is_default(self): + assert Namespace.coerce(None) == Namespace() + + def test_str_becomes_single_level_path(self): + assert Namespace.coerce("sales") == Namespace(path=("sales",)) + + def test_tuple_is_pure_path_never_catalog(self): + # A bare tuple is NEVER read positionally as (catalog, database). + assert Namespace.coerce(("a", "b")) == Namespace(path=("a", "b")) + assert Namespace.coerce(("a", "b")).catalog is None + + def test_namespace_passthrough(self): + ns = Namespace(catalog="wh", path=("sales",)) + assert Namespace.coerce(ns) is ns + + def test_unsupported_type_raises(self): + with pytest.raises(TypeError): + Namespace.coerce(123) + + +class TestValidation: + def test_empty_segment_rejected(self): + with pytest.raises(ValueError): + Namespace(path=("",)) + + def test_non_string_segment_rejected(self): + with pytest.raises(ValueError): + Namespace(path=(1,)) # type: ignore[arg-type] + + +class TestProperties: + def test_is_default_true_only_when_empty(self): + assert Namespace().is_default is True + assert Namespace(path=("x",)).is_default is False + assert Namespace(catalog="c").is_default is False + + def test_dotted_joins_catalog_then_path(self): + assert Namespace(catalog="wh", path=("a", "b")).dotted == "wh.a.b" + assert Namespace(path=("a", "b")).dotted == "a.b" + assert Namespace().dotted == "" + + def test_frozen(self): + ns = Namespace(path=("x",)) + with pytest.raises(Exception): + ns.path = ("y",) # type: ignore[misc] + + +def test_exported_from_public_surface(): + import mountainash_data + from mountainash_data.core import Namespace as CoreNamespace + + assert mountainash_data.Namespace is CoreNamespace From defa9ccbecd7711500173b8c29940aeb9b811dec Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Fri, 3 Jul 2026 01:34:48 +1000 Subject: [PATCH 02/11] feat(inspection): TableInfo/NamespaceInfo carry location: Namespace (DEBT-10) --- src/mountainash_data/core/inspection.py | 16 +++++++---- tests/test_unit/core/test_inspection.py | 37 +++++++++++++++---------- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/mountainash_data/core/inspection.py b/src/mountainash_data/core/inspection.py index f186bb5..b35e171 100644 --- a/src/mountainash_data/core/inspection.py +++ b/src/mountainash_data/core/inspection.py @@ -10,6 +10,8 @@ from dataclasses import dataclass, field import typing as t +from mountainash_data.core.namespace import Namespace + @dataclass(frozen=True) class ColumnInfo: @@ -28,8 +30,7 @@ class TableInfo: name: str columns: t.Sequence[ColumnInfo] - namespace: t.Optional[str] = None - catalog: t.Optional[str] = None + location: Namespace = field(default_factory=Namespace) description: t.Optional[str] = None metadata: t.Mapping[str, t.Any] = field(default_factory=dict) @@ -39,19 +40,22 @@ def column_names(self) -> list[str]: @property def qualified_name(self) -> str: - parts = [p for p in (self.catalog, self.namespace, self.name) if p] - return ".".join(parts) + return ".".join(p for p in (self.location.dotted, self.name) if p) @dataclass(frozen=True) class NamespaceInfo: """Physical metadata for a namespace (schema/database/dataset).""" - name: str + location: Namespace tables: t.Sequence[str] - catalog: t.Optional[str] = None metadata: t.Mapping[str, t.Any] = field(default_factory=dict) + @property + def name(self) -> str: + """The last path segment (the immediate namespace name), or "" for default.""" + return self.location.path[-1] if self.location.path else "" + @dataclass(frozen=True) class CatalogInfo: diff --git a/tests/test_unit/core/test_inspection.py b/tests/test_unit/core/test_inspection.py index bb23cbb..225d574 100644 --- a/tests/test_unit/core/test_inspection.py +++ b/tests/test_unit/core/test_inspection.py @@ -6,6 +6,7 @@ NamespaceInfo, TableInfo, ) +from mountainash_data.core.namespace import Namespace class TestColumnInfo: @@ -17,9 +18,7 @@ def test_minimal_column(self): def test_column_with_metadata(self): col = ColumnInfo( - name="created_at", - type_name="timestamp", - nullable=True, + name="created_at", type_name="timestamp", nullable=True, description="row creation time", ) assert col.description == "row creation time" @@ -33,39 +32,49 @@ def test_table_with_columns(self): ] table = TableInfo(name="users", columns=cols) assert table.name == "users" - assert len(table.columns) == 2 assert table.column_names == ["id", "name"] + assert table.location == Namespace() - def test_table_qualified_name(self): + def test_qualified_name_with_catalog(self): table = TableInfo( - name="users", - columns=[], - namespace="public", - catalog="main", + name="users", columns=[], + location=Namespace(catalog="main", path=("public",)), ) assert table.qualified_name == "main.public.users" - def test_table_qualified_name_no_catalog(self): - table = TableInfo(name="users", columns=[], namespace="public") + def test_qualified_name_no_catalog(self): + table = TableInfo(name="users", columns=[], location=Namespace(path=("public",))) assert table.qualified_name == "public.users" - def test_table_qualified_name_bare(self): + def test_qualified_name_deep_path_roundtrips(self): + table = TableInfo(name="t", columns=[], location=Namespace(path=("a", "b", "c"))) + assert table.qualified_name == "a.b.c.t" + + def test_qualified_name_bare(self): table = TableInfo(name="users", columns=[]) assert table.qualified_name == "users" class TestNamespaceInfo: def test_namespace_with_tables(self): - ns = NamespaceInfo(name="public", tables=["users", "orders"]) + ns = NamespaceInfo(location=Namespace(path=("public",)), tables=["users", "orders"]) assert ns.name == "public" assert ns.tables == ["users", "orders"] + def test_name_is_last_segment(self): + ns = NamespaceInfo(location=Namespace(path=("a", "b")), tables=[]) + assert ns.name == "b" + + def test_name_empty_for_default(self): + ns = NamespaceInfo(location=Namespace(), tables=[]) + assert ns.name == "" + class TestCatalogInfo: def test_catalog_with_namespaces(self): cat = CatalogInfo( name="main", - namespaces=[NamespaceInfo(name="public", tables=["users"])], + namespaces=[NamespaceInfo(location=Namespace(path=("public",)), tables=["users"])], ) assert cat.name == "main" assert len(cat.namespaces) == 1 From dd46bda0df1e77e31d5569abf62b79a9b0559b5e Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Fri, 3 Jul 2026 01:39:46 +1000 Subject: [PATCH 03/11] feat(protocol): NamespaceLike params + list_catalogs/list_namespaces(catalog=) (DEBT-10) --- src/mountainash_data/core/protocol.py | 10 +++--- tests/test_unit/core/test_protocol.py | 48 ++++++++++----------------- 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/src/mountainash_data/core/protocol.py b/src/mountainash_data/core/protocol.py index 26c60fb..7b2659d 100644 --- a/src/mountainash_data/core/protocol.py +++ b/src/mountainash_data/core/protocol.py @@ -13,6 +13,7 @@ NamespaceInfo, TableInfo, ) +from mountainash_data.core.namespace import NamespaceLike @t.runtime_checkable @@ -30,12 +31,13 @@ def close(self) -> t.Self: ... def __enter__(self) -> t.Self: ... def __exit__(self, *args: t.Any) -> None: ... - def list_tables(self, namespace: str | None = None) -> list[str]: ... - def list_namespaces(self) -> list[str]: ... + def list_tables(self, namespace: NamespaceLike = None) -> list[str]: ... + def list_namespaces(self, catalog: str | None = None) -> list[str]: ... + def list_catalogs(self) -> list[str]: ... def inspect_table( - self, name: str, namespace: str | None = None + self, name: str, namespace: NamespaceLike = None ) -> TableInfo: ... def inspect_namespace(self, name: str) -> NamespaceInfo: ... - def inspect_catalog(self) -> CatalogInfo: ... + def inspect_catalog(self, catalog: str | None = None) -> CatalogInfo: ... diff --git a/tests/test_unit/core/test_protocol.py b/tests/test_unit/core/test_protocol.py index 5e6826f..13f1f54 100644 --- a/tests/test_unit/core/test_protocol.py +++ b/tests/test_unit/core/test_protocol.py @@ -1,50 +1,39 @@ -"""Tests for core.protocol — structural Protocol definitions. - -These tests verify that the Protocols are well-formed and that a minimal -fake implementation type-checks at runtime via isinstance() with -runtime_checkable Protocols. -""" +"""Tests for core.protocol — structural Protocol definitions.""" from __future__ import annotations -import typing as t - -from mountainash_data.core.inspection import ( - CatalogInfo, - NamespaceInfo, - TableInfo, -) +from mountainash_data.core.inspection import CatalogInfo, NamespaceInfo, TableInfo +from mountainash_data.core.namespace import Namespace, NamespaceLike from mountainash_data.core.protocol import Backend class _FakeConnection: - """Minimal in-memory Connection implementation for protocol verification.""" - def __init__(self): self.closed = False - def list_namespaces(self) -> list[str]: + def list_namespaces(self, catalog: str | None = None) -> list[str]: return ["public"] - def list_tables(self, namespace: str | None = None) -> list[str]: + def list_catalogs(self) -> list[str]: + return ["main"] + + def list_tables(self, namespace: NamespaceLike = None) -> list[str]: return ["users"] - def inspect_table(self, name: str, namespace: str | None = None) -> TableInfo: - return TableInfo(name=name, columns=[], namespace=namespace) + def inspect_table(self, name: str, namespace: NamespaceLike = None) -> TableInfo: + return TableInfo(name=name, columns=[], location=Namespace.coerce(namespace)) def inspect_namespace(self, name: str) -> NamespaceInfo: - return NamespaceInfo(name=name, tables=["users"]) + return NamespaceInfo(location=Namespace(path=(name,)), tables=["users"]) - def inspect_catalog(self) -> CatalogInfo: - return CatalogInfo(name="fake", namespaces=[]) + def inspect_catalog(self, catalog: str | None = None) -> CatalogInfo: + return CatalogInfo(name=catalog or "fake", namespaces=[]) def close(self) -> None: self.closed = True class _FakeBackend: - """Minimal Backend implementation.""" - name = "fake" def connect(self) -> _FakeConnection: @@ -56,21 +45,20 @@ def test_fake_backend_satisfies_protocol(): assert backend.name == "fake" -def test_fake_connection_satisfies_protocol(): +def test_discovery_methods_present(): conn = _FakeConnection() + assert conn.list_catalogs() == ["main"] assert conn.list_namespaces() == ["public"] -def test_connection_inspect_returns_table_info(): +def test_inspect_table_carries_location(): conn = _FakeConnection() - info = conn.inspect_table("users", namespace="public") + info = conn.inspect_table("users", namespace=("a", "b")) assert isinstance(info, TableInfo) - assert info.name == "users" - assert info.namespace == "public" + assert info.location == Namespace(path=("a", "b")) def test_connection_close_idempotent_marker(): conn = _FakeConnection() - assert conn.closed is False conn.close() assert conn.closed is True From f3fae0393e44691736c7724f5fba6c756fa10471 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Fri, 3 Jul 2026 01:45:38 +1000 Subject: [PATCH 04/11] feat(ibis): _render_ibis_database + manual-SQL catalog gate render helpers (DEBT-10) --- src/mountainash_data/backends/ibis/backend.py | 44 +++++++++++++++++++ tests/test_unit/core/test_namespace.py | 42 ++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/src/mountainash_data/backends/ibis/backend.py b/src/mountainash_data/backends/ibis/backend.py index 75ebea9..3f17965 100644 --- a/src/mountainash_data/backends/ibis/backend.py +++ b/src/mountainash_data/backends/ibis/backend.py @@ -28,9 +28,53 @@ provider_for_dialect, provider_for_scheme, ) +from mountainash_data.core.namespace import Namespace, NamespaceLike # noqa: F401 (NamespaceLike: Task 5 signatures) from mountainash_auth_client import PasswordAuthProfile +def _render_ibis_database(ns: Namespace) -> tuple[str, str] | str | None: + """Render a coerced Namespace to ibis's native `database=` value (native ops). + + ibis models exactly `catalog -> database -> table`, so a namespace path + deeper than one level is unrepresentable and raises at this boundary. + """ + if len(ns.path) > 1: + raise ValueError( + f"ibis backends support a single namespace level; got path={ns.path!r}. " + f"Use Namespace(catalog=..., path=(one_level,)) to target a catalog." + ) + level = ns.path[0] if ns.path else None + if ns.catalog is not None: + if level is None: + raise ValueError( + "A catalog-qualified ibis namespace requires one path level." + ) + return (ns.catalog, level) + return level + + +def _render_ibis_namespace_single(ns: Namespace, *, op: str) -> str | None: + """Render for the manual-SQL families (upsert/add_columns/index). + + These build engine-native SQL and feed scalar index-introspection literals, + which cannot address a foreign catalog (postgres has no cross-database SQL; + the index builders take one scalar namespace literal). Reject a + catalog-qualified namespace here with a remedial ValueError (spec §8) rather + than emit broken three-part SQL downstream. + """ + if ns.catalog is not None: + raise ValueError( + f"{op} does not support catalog-qualified namespaces " + f"(catalog={ns.catalog!r}): it builds engine-native SQL that cannot " + f"address a foreign catalog. Use a native-delegating op, or omit the catalog." + ) + if len(ns.path) > 1: + raise ValueError( + f"ibis backends support a single namespace level; got path={ns.path!r}." + ) + return ns.path[0] if ns.path else None + + class IbisConnection: """A live ibis connection satisfying core.protocol.Connection. diff --git a/tests/test_unit/core/test_namespace.py b/tests/test_unit/core/test_namespace.py index 0fa83aa..15c113a 100644 --- a/tests/test_unit/core/test_namespace.py +++ b/tests/test_unit/core/test_namespace.py @@ -58,3 +58,45 @@ def test_exported_from_public_surface(): from mountainash_data.core import Namespace as CoreNamespace assert mountainash_data.Namespace is CoreNamespace + + +from mountainash_data.backends.ibis.backend import ( + _render_ibis_database, + _render_ibis_namespace_single, +) + + +class TestRenderIbisDatabase: + def test_default_renders_none(self): + assert _render_ibis_database(Namespace()) is None + + def test_single_level_renders_str(self): + assert _render_ibis_database(Namespace(path=("sales",))) == "sales" + + def test_catalog_qualified_renders_tuple(self): + ns = Namespace(catalog="wh", path=("sales",)) + assert _render_ibis_database(ns) == ("wh", "sales") + + def test_depth_greater_than_one_raises(self): + with pytest.raises(ValueError, match="single namespace level"): + _render_ibis_database(Namespace(path=("a", "b"))) + + def test_catalog_without_level_raises(self): + with pytest.raises(ValueError, match="requires one path level"): + _render_ibis_database(Namespace(catalog="wh")) + + +class TestRenderIbisNamespaceSingle: + def test_single_level_ok(self): + assert _render_ibis_namespace_single(Namespace(path=("sales",)), op="upsert") == "sales" + + def test_default_ok(self): + assert _render_ibis_namespace_single(Namespace(), op="upsert") is None + + def test_catalog_qualified_rejected(self): + with pytest.raises(ValueError, match="does not support catalog-qualified"): + _render_ibis_namespace_single(Namespace(catalog="wh", path=("sales",)), op="upsert") + + def test_depth_over_one_rejected(self): + with pytest.raises(ValueError, match="single namespace level"): + _render_ibis_namespace_single(Namespace(path=("a", "b")), op="create_index") From bd9a3634a5d173e65e07969ca180629851a1dadb Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Fri, 3 Jul 2026 01:53:35 +1000 Subject: [PATCH 05/11] feat(ibis): inspection carries Namespace + list_catalogs/list_namespaces(catalog=) (DEBT-10) --- src/mountainash_data/backends/ibis/backend.py | 69 ++++++++++++------- src/mountainash_data/backends/ibis/inspect.py | 23 ++----- tests/test_unit/backends/ibis/test_backend.py | 37 ++++++++++ tests/test_unit/backends/ibis/test_inspect.py | 13 +++- 4 files changed, 99 insertions(+), 43 deletions(-) diff --git a/src/mountainash_data/backends/ibis/backend.py b/src/mountainash_data/backends/ibis/backend.py index 3f17965..3451e26 100644 --- a/src/mountainash_data/backends/ibis/backend.py +++ b/src/mountainash_data/backends/ibis/backend.py @@ -28,7 +28,7 @@ provider_for_dialect, provider_for_scheme, ) -from mountainash_data.core.namespace import Namespace, NamespaceLike # noqa: F401 (NamespaceLike: Task 5 signatures) +from mountainash_data.core.namespace import Namespace, NamespaceLike from mountainash_auth_client import PasswordAuthProfile @@ -87,11 +87,13 @@ def __init__(self, ibis_conn: t.Any, dialect_spec: DialectSpec) -> None: self._dialect_spec = dialect_spec self._closed = False - def list_namespaces(self) -> list[str]: + def list_namespaces(self, catalog: str | None = None) -> list[str]: """Return the names of all namespaces (schemas/databases) visible to this connection.""" try: # ibis backends vary — some expose list_databases, some list_schemas if hasattr(self._ibis_conn, "list_databases"): + if catalog is not None: + return self._ibis_conn.list_databases(catalog=catalog) return self._ibis_conn.list_databases() if hasattr(self._ibis_conn, "list_schemas"): return self._ibis_conn.list_schemas() @@ -100,25 +102,45 @@ def list_namespaces(self) -> list[str]: print(f"Error listing namespaces: {e}") return [] - def list_tables(self, namespace: str | None = None) -> list[str]: + def list_catalogs(self) -> list[str]: + """Return catalogs visible to this connection. Degrades, never raises. + + Not every ibis backend exposes catalogs (only the CanListCatalog mixin + does). Fall back to the connection's current catalog, then — for + backends with no catalog concept at all (e.g. sqlite) — to the + dialect's own ibis backend name as a single-entry pseudo-catalog, so + callers always get at least one entry back for a live connection. + """ + try: + if hasattr(self._ibis_conn, "list_catalogs"): + return list(self._ibis_conn.list_catalogs()) + except Exception as e: + print(f"Error listing catalogs: {e}") + current = getattr(self._ibis_conn, "current_catalog", None) + if current is not None: + return [current] + return [self._dialect_spec.ibis_backend_name] + + def list_tables(self, namespace: NamespaceLike = None) -> list[str]: """Return the names of tables in the given namespace.""" + rendered = _render_ibis_database(Namespace.coerce(namespace)) try: - if namespace is not None: - return self._ibis_conn.list_tables(database=namespace) + if rendered is not None: + return self._ibis_conn.list_tables(database=rendered) return self._ibis_conn.list_tables() except Exception as e: print(f"Error listing tables: {e}") return [] - def inspect_table( - self, name: str, namespace: str | None = None - ) -> TableInfo: + def inspect_table(self, name: str, namespace: NamespaceLike = None) -> TableInfo: """Return shared-model metadata for one table.""" from mountainash_data.backends.ibis.inspect import table_to_info + ns = Namespace.coerce(namespace) + rendered = _render_ibis_database(ns) try: - ibis_table = self._ibis_conn.table(name, database=namespace) - return table_to_info(ibis_table, name=name, namespace=namespace) + ibis_table = self._ibis_conn.table(name, database=rendered) + return table_to_info(ibis_table, name=name, location=ns) except Exception as e: raise ValueError(f"Could not inspect table {name!r}: {e}") from e @@ -126,19 +148,19 @@ def inspect_namespace(self, name: str) -> NamespaceInfo: """Return shared-model metadata for one namespace.""" try: tables = self.list_tables(namespace=name) - return NamespaceInfo(name=name, tables=tables) + return NamespaceInfo(location=Namespace(path=(name,)), tables=tables) except Exception as e: raise ValueError(f"Could not inspect namespace {name!r}: {e}") from e - def inspect_catalog(self) -> CatalogInfo: + def inspect_catalog(self, catalog: str | None = None) -> CatalogInfo: """Return shared-model metadata for the connection's catalog.""" - namespaces = self.list_namespaces() + namespaces = self.list_namespaces(catalog=catalog) ns_infos = [ - NamespaceInfo(name=ns, tables=self.list_tables(namespace=ns)) + NamespaceInfo(location=Namespace(path=(ns,)), tables=self.list_tables(namespace=ns)) for ns in namespaces ] return CatalogInfo( - name=self._dialect_spec.ibis_backend_name, + name=catalog or self._dialect_spec.ibis_backend_name, namespaces=ns_infos, ) @@ -411,22 +433,23 @@ def get_connection(self) -> IbisConnection: # --- Inspection (terminal — delegates to IbisConnection) --- - def list_tables(self, namespace: str | None = None) -> list[str]: + def list_tables(self, namespace: NamespaceLike = None) -> list[str]: return self._require_connected().list_tables(namespace=namespace) - def list_namespaces(self) -> list[str]: - return self._require_connected().list_namespaces() + def list_namespaces(self, catalog: str | None = None) -> list[str]: + return self._require_connected().list_namespaces(catalog=catalog) + + def list_catalogs(self) -> list[str]: + return self._require_connected().list_catalogs() - def inspect_table( - self, name: str, namespace: str | None = None - ) -> TableInfo: + def inspect_table(self, name: str, namespace: NamespaceLike = None) -> TableInfo: return self._require_connected().inspect_table(name, namespace=namespace) def inspect_namespace(self, name: str) -> NamespaceInfo: return self._require_connected().inspect_namespace(name) - def inspect_catalog(self) -> CatalogInfo: - return self._require_connected().inspect_catalog() + def inspect_catalog(self, catalog: str | None = None) -> CatalogInfo: + return self._require_connected().inspect_catalog(catalog=catalog) # --- Thin wrapper operations (fluent — return self) --- diff --git a/src/mountainash_data/backends/ibis/inspect.py b/src/mountainash_data/backends/ibis/inspect.py index 3cc64dd..a27d817 100644 --- a/src/mountainash_data/backends/ibis/inspect.py +++ b/src/mountainash_data/backends/ibis/inspect.py @@ -2,33 +2,20 @@ from __future__ import annotations - -from mountainash_data.core.inspection import ( - ColumnInfo, - TableInfo, -) +from mountainash_data.core.inspection import ColumnInfo, TableInfo +from mountainash_data.core.namespace import Namespace def table_to_info( ibis_table, *, name: str, - namespace: str | None = None, - catalog: str | None = None, + location: Namespace = Namespace(), ) -> TableInfo: """Convert an ibis Table object into a TableInfo.""" schema = ibis_table.schema() columns = [ - ColumnInfo( - name=col_name, - type_name=str(col_type), - nullable=col_type.nullable, - ) + ColumnInfo(name=col_name, type_name=str(col_type), nullable=col_type.nullable) for col_name, col_type in zip(schema.names, schema.types) ] - return TableInfo( - name=name, - columns=columns, - namespace=namespace, - catalog=catalog, - ) + return TableInfo(name=name, columns=columns, location=location) diff --git a/tests/test_unit/backends/ibis/test_backend.py b/tests/test_unit/backends/ibis/test_backend.py index 6f0896b..bdc7936 100644 --- a/tests/test_unit/backends/ibis/test_backend.py +++ b/tests/test_unit/backends/ibis/test_backend.py @@ -6,6 +6,7 @@ from mountainash_data.backends.ibis.backend import IbisBackend from mountainash_data.backends.ibis.dialects._registry import DIALECTS from mountainash_data.core.protocol import Backend +from mountainash_data.core.namespace import Namespace def test_ibis_backend_satisfies_protocol(): @@ -443,3 +444,39 @@ def test_upsert_unsupported_dialect_raises(): backend._conn = IbisConnection(None, DIALECTS["clickhouse"]) with pytest.raises(NotImplementedError, match="does not support upsert"): backend.upsert("t", {}, conflict_columns=["id"]) + + +# --------------------------------------------------------------------------- +# Namespace-carrying inspection + discovery (DEBT-10 Task 5) +# --------------------------------------------------------------------------- + +def test_list_catalogs_degrades_to_current(): + """A catalog-less/simple backend still answers list_catalogs().""" + with IbisBackend(dialect="sqlite", database=":memory:") as backend: + cats = backend.list_catalogs() + assert isinstance(cats, list) + assert len(cats) >= 1 + + +def test_inspect_table_location_default_namespace(): + with IbisBackend(dialect="sqlite", database=":memory:") as backend: + backend.create_table("t", {"id": [1]}) + info = backend.inspect_table("t") + assert info.name == "t" + assert info.location == Namespace() + + +def test_inspect_table_location_reflects_namespace(): + with IbisBackend(dialect="duckdb", database=":memory:") as backend: + raw = backend.ibis_connection() + raw.raw_sql("CREATE SCHEMA tenant_a") + raw.raw_sql("CREATE TABLE tenant_a.widgets (id INTEGER)") + info = backend.inspect_table("widgets", namespace="tenant_a") + assert info.location == Namespace(path=("tenant_a",)) + assert info.qualified_name == "tenant_a.widgets" + + +def test_list_namespaces_accepts_catalog_kwarg(): + with IbisBackend(dialect="duckdb", database=":memory:") as backend: + # catalog=None is the default; the kwarg must be accepted without error. + assert isinstance(backend.list_namespaces(catalog=None), list) diff --git a/tests/test_unit/backends/ibis/test_inspect.py b/tests/test_unit/backends/ibis/test_inspect.py index 0bf44e9..248f01e 100644 --- a/tests/test_unit/backends/ibis/test_inspect.py +++ b/tests/test_unit/backends/ibis/test_inspect.py @@ -4,6 +4,7 @@ from mountainash_data.backends.ibis.inspect import table_to_info from mountainash_data.core.inspection import TableInfo +from mountainash_data.core.namespace import Namespace def test_table_to_info_from_ibis_table(): @@ -14,9 +15,17 @@ def test_table_to_info_from_ibis_table(): schema=ibis.schema({"id": "int64", "name": "string"}), ) table = conn.table("users") - info = table_to_info(table, name="users", namespace="main") + info = table_to_info(table, name="users", location=Namespace(path=("main",))) assert isinstance(info, TableInfo) assert info.name == "users" - assert info.namespace == "main" + assert info.location == Namespace(path=("main",)) assert info.column_names == ["id", "name"] assert info.columns[0].type_name == "int64" + + +def test_table_to_info_builds_location(): + con = ibis.duckdb.connect() + con.create_table("t", schema=ibis.schema({"id": "int64"})) + info = table_to_info(con.table("t"), name="t", location=Namespace(path=("main",))) + assert info.location == Namespace(path=("main",)) + assert info.column_names == ["id"] From dcc2710462dd202935b4300e9db40c973a3ea184 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Fri, 3 Jul 2026 02:01:39 +1000 Subject: [PATCH 06/11] feat(ibis): rename database= -> namespace= on native DDL/ops + coerce/render (DEBT-10) --- src/mountainash_data/backends/ibis/backend.py | 46 +++++++++++-------- tests/test_unit/backends/ibis/test_backend.py | 45 ++++++++++-------- 2 files changed, 51 insertions(+), 40 deletions(-) diff --git a/src/mountainash_data/backends/ibis/backend.py b/src/mountainash_data/backends/ibis/backend.py index 3451e26..721bae5 100644 --- a/src/mountainash_data/backends/ibis/backend.py +++ b/src/mountainash_data/backends/ibis/backend.py @@ -459,13 +459,14 @@ def create_table( obj: t.Any, *, schema: t.Any | None = None, - database: str | None = None, + namespace: NamespaceLike = None, temp: bool = False, overwrite: bool = False, ) -> IbisBackend: conn = self._require_connected() + rendered = _render_ibis_database(Namespace.coerce(namespace)) conn._ibis_conn.create_table( - name, obj=obj, schema=schema, database=database, + name, obj=obj, schema=schema, database=rendered, temp=temp, overwrite=overwrite, ) return self @@ -474,11 +475,12 @@ def drop_table( self, name: str, *, - database: str | None = None, + namespace: NamespaceLike = None, force: bool = False, ) -> IbisBackend: conn = self._require_connected() - conn._ibis_conn.drop_table(name, database=database, force=force) + rendered = _render_ibis_database(Namespace.coerce(namespace)) + conn._ibis_conn.drop_table(name, database=rendered, force=force) return self def create_view( @@ -486,22 +488,24 @@ def create_view( name: str, obj: t.Any, *, - database: str | None = None, + namespace: NamespaceLike = None, overwrite: bool = False, ) -> IbisBackend: conn = self._require_connected() - conn._ibis_conn.create_view(name, obj=obj, database=database, overwrite=overwrite) + rendered = _render_ibis_database(Namespace.coerce(namespace)) + conn._ibis_conn.create_view(name, obj=obj, database=rendered, overwrite=overwrite) return self def drop_view( self, name: str, *, - database: str | None = None, + namespace: NamespaceLike = None, force: bool = False, ) -> IbisBackend: conn = self._require_connected() - conn._ibis_conn.drop_view(name, database=database, force=force) + rendered = _render_ibis_database(Namespace.coerce(namespace)) + conn._ibis_conn.drop_view(name, database=rendered, force=force) return self def insert( @@ -509,26 +513,28 @@ def insert( name: str, obj: t.Any, *, - database: str | None = None, + namespace: NamespaceLike = None, overwrite: bool = False, ) -> IbisBackend: conn = self._require_connected() - conn._ibis_conn.insert(name, obj=obj, database=database, overwrite=overwrite) + rendered = _render_ibis_database(Namespace.coerce(namespace)) + conn._ibis_conn.insert(name, obj=obj, database=rendered, overwrite=overwrite) return self def truncate( self, name: str, *, - database: str | None = None, + namespace: NamespaceLike = None, schema: str | None = None, ) -> IbisBackend: conn = self._require_connected() + rendered = _render_ibis_database(Namespace.coerce(namespace)) # ibis SQLBackend.truncate_table() accepts only table_name + database; # schema is not a standard kwarg at the SQLBackend level. kwargs: dict[str, t.Any] = {} - if database is not None: - kwargs["database"] = database + if rendered is not None: + kwargs["database"] = rendered conn._ibis_conn.truncate_table(name, **kwargs) return self @@ -543,16 +549,16 @@ def rename_table(self, old_name: str, new_name: str) -> IbisBackend: # --- Terminal operations (return data) --- - def table(self, name: str, *, database: str | None = None) -> t.Any: + def table(self, name: str, *, namespace: NamespaceLike = None) -> t.Any: conn = self._require_connected() - return conn._ibis_conn.table(name, database=database) + rendered = _render_ibis_database(Namespace.coerce(namespace)) + return conn._ibis_conn.table(name, database=rendered) - def table_exists( - self, name: str, database: str | None = None - ) -> bool: + def table_exists(self, name: str, namespace: NamespaceLike = None) -> bool: # ibis exposes no native table_exists; scope the membership check to the - # requested namespace by forwarding database= through list_tables (DEBT-9). - return name in self.list_tables(namespace=database) + # requested namespace by forwarding through list_tables (DEBT-9/10). + # Pass the RAW namespace — list_tables coerces once (no double render). + return name in self.list_tables(namespace=namespace) def run_sql( self, diff --git a/tests/test_unit/backends/ibis/test_backend.py b/tests/test_unit/backends/ibis/test_backend.py index bdc7936..9b9ddf2 100644 --- a/tests/test_unit/backends/ibis/test_backend.py +++ b/tests/test_unit/backends/ibis/test_backend.py @@ -321,45 +321,50 @@ def test_table_exists_returns_bool(): assert backend.table_exists("t") is True -def test_table_exists_honors_database_namespace(): - """table_exists(database=...) must scope the check to that namespace (DEBT-9). - - A table living only in a non-default schema must be found via ``database=`` - and must NOT be found in the default namespace, and vice versa. - """ +def test_table_exists_honors_namespace(): + """table_exists(namespace=...) scopes the check to that namespace (DEBT-9/10).""" with IbisBackend(dialect="duckdb", database=":memory:") as backend: raw = backend.ibis_connection() raw.raw_sql("CREATE SCHEMA tenant_a") raw.raw_sql("CREATE TABLE tenant_a.sleep (id INTEGER)") raw.raw_sql("CREATE TABLE main_only (id INTEGER)") - - # Table exists only in tenant_a. - assert backend.table_exists("sleep", database="tenant_a") is True + assert backend.table_exists("sleep", namespace="tenant_a") is True assert backend.table_exists("sleep") is False - # Table exists only in the default namespace. assert backend.table_exists("main_only") is True - assert backend.table_exists("main_only", database="tenant_a") is False + assert backend.table_exists("main_only", namespace="tenant_a") is False -def test_table_exists_forwards_database_to_introspection(monkeypatch): - """The ``database`` arg must reach the introspection call, not be dropped. - - Guards the swallowed-error path: ``IbisConnection.list_tables`` returns ``[]`` - on failure, so a test asserting only a bool return can pass on a version that - never forwards ``database``. Assert the forwarding directly. - """ +def test_table_exists_forwards_namespace_to_introspection(monkeypatch): with IbisBackend(dialect="sqlite", database=":memory:") as backend: - seen: dict[str, str | None] = {} + seen: dict[str, object] = {} def fake_list_tables(namespace=None): seen["namespace"] = namespace return ["sleep"] monkeypatch.setattr(backend, "list_tables", fake_list_tables) - assert backend.table_exists("sleep", database="tenant_a") is True + assert backend.table_exists("sleep", namespace="tenant_a") is True assert seen == {"namespace": "tenant_a"} +def test_database_keyword_rejected_on_table_exists(): + """Clean break: the old database= keyword no longer exists.""" + import pytest + + with IbisBackend(dialect="sqlite", database=":memory:") as backend: + with pytest.raises(TypeError): + backend.table_exists("t", database="x") + + +def test_create_and_drop_table_in_namespace(): + with IbisBackend(dialect="duckdb", database=":memory:") as backend: + backend.ibis_connection().raw_sql("CREATE SCHEMA tenant_b") + backend.create_table("gadgets", {"id": [1]}, namespace="tenant_b") + assert backend.table_exists("gadgets", namespace="tenant_b") is True + backend.drop_table("gadgets", namespace="tenant_b") + assert backend.table_exists("gadgets", namespace="tenant_b") is False + + def test_fluent_chaining(): """Multiple fluent calls can be chained.""" with IbisBackend(dialect="sqlite", database=":memory:") as backend: From 6ebaa17b42cf367a6128c852c7e9c77cdc44429e Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Fri, 3 Jul 2026 02:11:51 +1000 Subject: [PATCH 07/11] feat(ibis): rename database= -> namespace= on upsert/add_columns + catalog-qualified targets (DEBT-10) --- src/mountainash_data/backends/ibis/backend.py | 14 +++-- .../backends/ibis/operations.py | 63 ++++++++++--------- .../test_upsert_mysql_preflight.py | 2 +- tests/test_integration/test_write_ops_live.py | 4 +- .../backends/ibis/test_add_columns.py | 33 ++++++++-- .../backends/ibis/test_upsert_render.py | 27 ++++++-- 6 files changed, 92 insertions(+), 51 deletions(-) diff --git a/src/mountainash_data/backends/ibis/backend.py b/src/mountainash_data/backends/ibis/backend.py index 721bae5..9875a7c 100644 --- a/src/mountainash_data/backends/ibis/backend.py +++ b/src/mountainash_data/backends/ibis/backend.py @@ -604,10 +604,11 @@ def upsert( update_columns: list[str] | str | None = None, conflict_action: str = "UPDATE", update_condition: t.Any = None, # ConditionPredicate | None - database: str | None = None, + namespace: NamespaceLike = None, schema: str | None = None, ) -> IbisBackend: conn = self._require_connected() + rendered = _render_ibis_namespace_single(Namespace.coerce(namespace), op="upsert") hook = self._spec.upsert_hook if hook is not None: hook( @@ -616,7 +617,7 @@ def upsert( update_columns=update_columns, conflict_action=conflict_action, update_condition=update_condition, - database=database, + namespace=rendered, schema=schema, ) else: @@ -626,7 +627,7 @@ def upsert( update_columns=update_columns, conflict_action=conflict_action, update_condition=update_condition, - database=database, + namespace=rendered, schema=schema, ) return self @@ -636,19 +637,20 @@ def add_columns( name: str, source: t.Any, *, - database: str | None = None, + namespace: NamespaceLike = None, ) -> IbisBackend: """Additively evolve `name`: add columns present in `source` but missing from the table. `source` is a frame (types inferred) or a ``{column: dtype}`` mapping. Additive, idempotent, dialect-agnostic. """ conn = self._require_connected() + rendered = _render_ibis_namespace_single(Namespace.coerce(namespace), op="add_columns") hook = self._spec.add_columns_hook if hook is not None: - hook(conn._ibis_conn, name, source, database=database) + hook(conn._ibis_conn, name, source, namespace=rendered) else: _generic_add_columns( - conn._ibis_conn, name, source, database=database + conn._ibis_conn, name, source, namespace=rendered ) return self diff --git a/src/mountainash_data/backends/ibis/operations.py b/src/mountainash_data/backends/ibis/operations.py index 700aef1..6a6f26b 100644 --- a/src/mountainash_data/backends/ibis/operations.py +++ b/src/mountainash_data/backends/ibis/operations.py @@ -49,13 +49,13 @@ def _generate_index_name( def _format_qualified_table( table_name: str, *, - database: str | None = None, + namespace: str | None = None, schema: str | None = None ) -> str: """Format fully qualified table name.""" parts = [] - if database: - parts.append(database) + if namespace: + parts.append(namespace) if schema: parts.append(schema) parts.append(table_name) @@ -188,7 +188,7 @@ def _generic_add_columns( table_name: str, source: t.Any, *, - database: str | None = None, + namespace: str | None = None, ) -> None: """Add columns present in `source` but missing from `table_name`. @@ -201,19 +201,20 @@ def _generic_add_columns( TABLE … ADD COLUMN`` is issued per new column (SQLite permits only one per statement). - `table_name` and `database` must each be a simple (non-dotted) identifier; + `table_name` and `namespace` must each be a simple (non-dotted) identifier; each is quoted as a single part. Dotted/multi-part qualified names are out - of scope. + of scope. `namespace` is a single namespace level (str | None); catalog- + qualified targets are rejected upstream by `_render_ibis_namespace_single`. """ _validate_simple_identifier(table_name, kind="table_name") - if database is not None: - _validate_simple_identifier(database, kind="database") + if namespace is not None: + _validate_simple_identifier(namespace, kind="namespace") candidate = _normalize_to_schema(source) - existing = set(ibis_conn.table(table_name, database=database).schema().names) + existing = set(ibis_conn.table(table_name, database=namespace).schema().names) type_mapper = ibis_conn.compiler.type_mapper dialect = ibis_conn.compiler.dialect - table_parts = [database, table_name] if database else [table_name] + table_parts = [namespace, table_name] if namespace else [table_name] qualified = ".".join(quote_identifier(part, dialect) for part in table_parts) for col_name, dtype in candidate.items(): @@ -513,14 +514,14 @@ def _render_on_conflict( update: list[str], conflict_action: str, update_condition: t.Any, - database: str | None, + namespace: str | None, schema: str | None, ) -> str: """Thin wrapper: derive dialect/source_sql/condition_sql from the live connection and delegate to ``build_on_conflict_sql``.""" dialect = dialect_of(ibis_conn) source_sql, cols = compiled_source(ibis_conn, obj, target_schema) - parts = [p for p in (database, schema, name) if p] + parts = [p for p in (namespace, schema, name) if p] target = qualified_name(parts, dialect) # update_condition only shapes the DO UPDATE arm; ON CONFLICT … DO NOTHING @@ -607,14 +608,14 @@ def _render_merge( update: list[str], conflict_action: str, update_condition: t.Any, - database: str | None, + namespace: str | None, schema: str | None, ) -> str: """Thin wrapper: derive dialect/source_sql/condition_sql from the live connection and delegate to ``build_merge_sql``.""" dialect = dialect_of(ibis_conn) source_sql, cols = compiled_source(ibis_conn, obj, target_schema) - parts = [p for p in (database, schema, name) if p] + parts = [p for p in (namespace, schema, name) if p] target = qualified_name(parts, dialect) condition_sql: str | None = None @@ -640,7 +641,7 @@ def _mysql_validate_conflict_key( ibis_conn: t.Any, name: str, conflict: list[str], - database: str | None, + namespace: str | None, ) -> None: """Prove the safe MySQL/MariaDB ON DUPLICATE KEY case or raise (spec §6.2). @@ -656,13 +657,13 @@ def _mysql_validate_conflict_key( NOTE: ``ibis_conn.current_database`` is a PROPERTY in ibis >=12 (no parens). """ - # Primary gate: name/database must be simple identifiers (charset-allowlisted + # Primary gate: name/namespace must be simple identifiers (charset-allowlisted # by _validate_simple_identifier). _generic_upsert validates them upstream; # re-validate here so a direct caller is equally safe. _validate_simple_identifier(name, kind="name") - if database is not None: - _validate_simple_identifier(database, kind="database") - db = database or ibis_conn.current_database + if namespace is not None: + _validate_simple_identifier(namespace, kind="namespace") + db = namespace or ibis_conn.current_database # Defense in depth: these values go into SQL *string literals*, so render # them as escaped literals via sqlglot rather than bare f-string interpolation # (belt-and-suspenders behind the allowlist above). @@ -782,7 +783,7 @@ def _render_on_duplicate_key( update: list[str], conflict_action: str, update_condition: t.Any, - database: str | None, + namespace: str | None, schema: str | None, ) -> str: """Thin wrapper: run the MySQL prove-safe preflight, then render the SQL. @@ -790,10 +791,10 @@ def _render_on_duplicate_key( Delegates SQL construction to ``build_on_duplicate_key_sql`` so the pure builder is testable without a live MySQL connection. """ - _mysql_validate_conflict_key(ibis_conn, name, conflict, database) + _mysql_validate_conflict_key(ibis_conn, name, conflict, namespace) dialect = dialect_of(ibis_conn) source_sql, cols = compiled_source(ibis_conn, obj, target_schema) - parts = [p for p in (database, schema, name) if p] + parts = [p for p in (namespace, schema, name) if p] target = qualified_name(parts, dialect) return build_on_duplicate_key_sql( @@ -817,7 +818,7 @@ def _generic_upsert( update_columns: t.Any, conflict_action: str, update_condition: t.Any, - database: str | None, + namespace: str | None, schema: str | None, ) -> None: """Dialect-agnostic upsert dispatcher. @@ -825,7 +826,7 @@ def _generic_upsert( Validation precedence (spec §10): 1. style (unknown → NotImplementedError) 2. target existence - 3. identifier validation (name, database) + 3. identifier validation (name, namespace) 4. conflict_action validity 5. update_condition — validated UNCONDITIONALLY even under NOTHING (malformed predicate must error regardless of action path) @@ -843,14 +844,14 @@ def _generic_upsert( ) # §10.2 — target existence - _tables = ibis_conn.list_tables(database=database) if database is not None else ibis_conn.list_tables() + _tables = ibis_conn.list_tables(database=namespace) if namespace is not None else ibis_conn.list_tables() if name not in _tables: raise ValueError(f"target table {name!r} does not exist") # §10.3 — identifier validation (every part that reaches qualified_name) _validate_simple_identifier(name, kind="name") - if database is not None: - _validate_simple_identifier(database, kind="database") + if namespace is not None: + _validate_simple_identifier(namespace, kind="namespace") if schema is not None: _validate_simple_identifier(schema, kind="schema") @@ -860,7 +861,7 @@ def _generic_upsert( f"conflict_action must be UPDATE or NOTHING, got {conflict_action!r}" ) - target_schema = ibis_conn.table(name, database=database).schema() + target_schema = ibis_conn.table(name, database=namespace).schema() conflict = _normalize_columns(conflict_columns) # conflict column existence @@ -897,19 +898,19 @@ def _generic_upsert( stmt = _render_on_conflict( ibis_conn, name, obj, target_schema=target_schema, conflict=conflict, update=update, conflict_action=conflict_action, - update_condition=update_condition, database=database, schema=schema, + update_condition=update_condition, namespace=namespace, schema=schema, ) elif style is UpsertStyle.MERGE: stmt = _render_merge( ibis_conn, name, obj, target_schema=target_schema, conflict=conflict, update=update, conflict_action=conflict_action, - update_condition=update_condition, database=database, schema=schema, + update_condition=update_condition, namespace=namespace, schema=schema, ) elif style is UpsertStyle.ON_DUPLICATE_KEY: stmt = _render_on_duplicate_key( ibis_conn, name, obj, target_schema=target_schema, conflict=conflict, update=update, conflict_action=conflict_action, - update_condition=update_condition, database=database, schema=schema, + update_condition=update_condition, namespace=namespace, schema=schema, ) else: raise NotImplementedError(f"unknown upsert_style: {style!r}") diff --git a/tests/test_integration/test_upsert_mysql_preflight.py b/tests/test_integration/test_upsert_mysql_preflight.py index a25a39c..cb65aaf 100644 --- a/tests/test_integration/test_upsert_mysql_preflight.py +++ b/tests/test_integration/test_upsert_mysql_preflight.py @@ -21,7 +21,7 @@ def _odk(con, name, df, conflict): _generic_upsert( con, name, df, style=UpsertStyle.ON_DUPLICATE_KEY, conflict_columns=conflict, update_columns=None, conflict_action="UPDATE", - update_condition=None, database=None, schema=None, + update_condition=None, namespace=None, schema=None, ) diff --git a/tests/test_integration/test_write_ops_live.py b/tests/test_integration/test_write_ops_live.py index 9d8caca..fade3ea 100644 --- a/tests/test_integration/test_write_ops_live.py +++ b/tests/test_integration/test_write_ops_live.py @@ -35,7 +35,7 @@ def test_merge_insert_and_update_postgres(postgres_backend): _generic_upsert( con, "mrg", pl.DataFrame({"id": [2, 3], "v": ["B", "c"]}), style=UpsertStyle.MERGE, conflict_columns=["id"], update_columns=None, - conflict_action="UPDATE", update_condition=None, database=None, schema=None, + conflict_action="UPDATE", update_condition=None, namespace=None, schema=None, ) rows = dict( con.table("mrg").order_by("id").execute()[["id", "v"]].itertuples(index=False) @@ -54,7 +54,7 @@ def test_merge_nothing_postgres(postgres_backend): _generic_upsert( con, "mrg_nothing", pl.DataFrame({"id": [1, 2], "v": ["X", "b"]}), style=UpsertStyle.MERGE, conflict_columns=["id"], update_columns=None, - conflict_action="NOTHING", update_condition=None, database=None, schema=None, + conflict_action="NOTHING", update_condition=None, namespace=None, schema=None, ) rows = dict( con.table("mrg_nothing").order_by("id").execute()[["id", "v"]].itertuples(index=False) diff --git a/tests/test_unit/backends/ibis/test_add_columns.py b/tests/test_unit/backends/ibis/test_add_columns.py index d4e62a0..23cb5d0 100644 --- a/tests/test_unit/backends/ibis/test_add_columns.py +++ b/tests/test_unit/backends/ibis/test_add_columns.py @@ -105,14 +105,14 @@ def test_rejects_dotted_table_name(self): with pytest.raises(ValueError, match="simple"): _generic_add_columns(con, "schema.t", {"x": "float64"}) - def test_rejects_dotted_database(self): + def test_rejects_dotted_namespace(self): con = ibis.duckdb.connect() con.create_table("t", pl.DataFrame({"id": [1]})) with pytest.raises(ValueError, match="simple"): - _generic_add_columns(con, "t", {"x": "float64"}, database="a.b") + _generic_add_columns(con, "t", {"x": "float64"}, namespace="a.b") - def test_database_qualified_add_on_duckdb(self): - """Happy-path: two-part qualified quoting (database.table) via ATTACH. + def test_namespace_qualified_add_on_duckdb(self): + """Happy-path: two-part qualified quoting (namespace.table) via ATTACH. ibis 10.4.0's duckdb backend does not support create_table(database=...) for attached databases — ``database=`` resolves to the DuckDB *schema* @@ -125,7 +125,7 @@ def test_database_qualified_add_on_duckdb(self): con.raw_sql("ATTACH ':memory:' AS mem2") con.raw_sql("CREATE TABLE mem2.t (id INTEGER)") con.raw_sql("INSERT INTO mem2.t VALUES (1)") - _generic_add_columns(con, "t", {"score": "float64"}, database="mem2") + _generic_add_columns(con, "t", {"score": "float64"}, namespace="mem2") assert "score" in con.table("t", database="mem2").schema().names @@ -166,7 +166,7 @@ def test_create_evolve_type_parity_sqlite(self): def test_hook_override_wins_over_generic(self): calls = [] - def fake_hook(ibis_conn, name, source, *, database=None): + def fake_hook(ibis_conn, name, source, *, namespace=None): calls.append((name, source)) with IbisBackend(dialect="duckdb", database=":memory:") as be: @@ -177,3 +177,24 @@ def fake_hook(ibis_conn, name, source, *, database=None): # generic path did NOT run -> column absent cols = {c.name for c in be.inspect_table("t").columns} assert "x" not in cols + + +def test_add_columns_accepts_namespace_kwarg(): + from mountainash_data import IbisBackend + + with IbisBackend(dialect="duckdb", database=":memory:") as backend: + backend.ibis_connection().raw_sql("CREATE SCHEMA tn") + backend.create_table("evolving", {"id": [1]}, namespace="tn") + backend.add_columns("evolving", {"id": "int64", "extra": "string"}, namespace="tn") + info = backend.inspect_table("evolving", namespace="tn") + assert "extra" in info.column_names + + +def test_add_columns_rejects_database_kwarg(): + import pytest + from mountainash_data import IbisBackend + + with IbisBackend(dialect="duckdb", database=":memory:") as backend: + backend.create_table("t", {"id": [1]}) + with pytest.raises(TypeError): + backend.add_columns("t", {"id": "int64"}, database="x") diff --git a/tests/test_unit/backends/ibis/test_upsert_render.py b/tests/test_unit/backends/ibis/test_upsert_render.py index 69f776d..73cc51c 100644 --- a/tests/test_unit/backends/ibis/test_upsert_render.py +++ b/tests/test_unit/backends/ibis/test_upsert_render.py @@ -37,7 +37,7 @@ def test_insert_and_update_duckdb(self): con, "t", pl.DataFrame({"id": [2, 3], "v": ["B", "c"]}), style=UpsertStyle.ON_CONFLICT, conflict_columns=["id"], update_columns=None, conflict_action="UPDATE", - update_condition=None, database=None, schema=None, + update_condition=None, namespace=None, schema=None, ) rows = dict(con.table("t").order_by("id").execute()[["id", "v"]].itertuples(index=False)) assert rows == {1: "a", 2: "B", 3: "c"} @@ -49,7 +49,7 @@ def test_do_nothing_duckdb(self): con, "t", pl.DataFrame({"id": [2], "v": ["X"]}), style=UpsertStyle.ON_CONFLICT, conflict_columns="id", update_columns=None, conflict_action="NOTHING", - update_condition=None, database=None, schema=None, + update_condition=None, namespace=None, schema=None, ) assert con.table("t").filter(ibis._.id == 2).execute()["v"].iloc[0] == "b" @@ -62,7 +62,7 @@ def test_composite_key_sqlite(self): con, "t", pl.DataFrame({"a": [1], "b": [1], "v": ["y"]}), style=UpsertStyle.ON_CONFLICT, conflict_columns=["a", "b"], update_columns=None, conflict_action="UPDATE", - update_condition=None, database=None, schema=None, + update_condition=None, namespace=None, schema=None, ) assert con.table("t").execute()["v"].iloc[0] == "y" @@ -75,7 +75,7 @@ def test_conditional_update_only_when_newer_duckdb(self): style=UpsertStyle.ON_CONFLICT, conflict_columns=["id"], update_columns=None, conflict_action="UPDATE", update_condition=lambda inc, exi: inc.ver > exi.ver, - database=None, schema=None, + namespace=None, schema=None, ) # incoming ver(3) is NOT newer than existing(5) -> unchanged assert con.table("t").execute()["v"].iloc[0] == "old" @@ -88,7 +88,7 @@ def test_unknown_style_raises_notimplemented(self): con, "t", pl.DataFrame({"id": [9], "v": ["z"]}), style=None, conflict_columns=["id"], update_columns=None, conflict_action="UPDATE", update_condition=None, - database=None, schema=None, + namespace=None, schema=None, ) @@ -223,3 +223,20 @@ def test_rejects_unsafe_identifiers(self, bad: str) -> None: ) def test_accepts_safe_identifiers(self, good: str) -> None: _validate_simple_identifier(good, kind="name") # no raise + + +def test_upsert_rejects_catalog_qualified_namespace(): + """upsert builds engine-native SQL; a catalog-qualified namespace must raise + a clean ValueError, never reach the SQL builders.""" + import pytest + from mountainash_data import IbisBackend + from mountainash_data.core.namespace import Namespace + + with IbisBackend(dialect="duckdb", database=":memory:") as backend: + backend.create_table("accounts", {"id": [1], "bal": [10]}) + with pytest.raises(ValueError, match="does not support catalog-qualified"): + backend.upsert( + "accounts", {"id": [1], "bal": [20]}, + conflict_columns=["id"], + namespace=Namespace(catalog="wh", path=("sales",)), + ) From 0e9435ce714c465935fef04b1d5c2b85221eda48 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Fri, 3 Jul 2026 02:23:09 +1000 Subject: [PATCH 08/11] feat(ibis): rename database= -> namespace= on index ops end-to-end (DEBT-10) --- src/mountainash_data/backends/ibis/_index.py | 34 +++---- src/mountainash_data/backends/ibis/backend.py | 32 ++++--- .../backends/ibis/operations.py | 88 +++++++++++-------- .../test_unit/backends/ibis/test_index_ops.py | 32 +++++++ 4 files changed, 118 insertions(+), 68 deletions(-) diff --git a/src/mountainash_data/backends/ibis/_index.py b/src/mountainash_data/backends/ibis/_index.py index 5e9d0c4..71e16fa 100644 --- a/src/mountainash_data/backends/ibis/_index.py +++ b/src/mountainash_data/backends/ibis/_index.py @@ -110,7 +110,7 @@ def _generic_index_exists( index_name: str, *, table_name: t.Optional[str] = None, - database: t.Optional[str] = None, + namespace: t.Optional[str] = None, exists_sql_fn: t.Any, ) -> bool: """Run the dialect's introspection SQL and return whether the index exists.""" @@ -119,9 +119,9 @@ def _generic_index_exists( _validate_simple_identifier(index_name, kind="index_name") if table_name is not None: _validate_simple_identifier(table_name, kind="table_name") - if database is not None: - _validate_simple_identifier(database, kind="database") - result = ibis_conn.sql(exists_sql_fn(index_name, table_name, database)) + if namespace is not None: + _validate_simple_identifier(namespace, kind="namespace") + result = ibis_conn.sql(exists_sql_fn(index_name, table_name, namespace)) if result is None: return False import mountainash as ma @@ -135,9 +135,9 @@ def _generic_index_exists( return first_col[0] > 0 -def _target_ref(ibis_conn: t.Any, table_name: str, database: t.Optional[str]) -> str: +def _target_ref(ibis_conn: t.Any, table_name: str, namespace: t.Optional[str]) -> str: dialect = dialect_of(ibis_conn) - parts = [database, table_name] if database else [table_name] + parts = [namespace, table_name] if namespace else [table_name] return qualified_name(parts, dialect) @@ -150,7 +150,7 @@ def _generic_create_index( unique: bool = False, index_type: t.Optional[str] = None, where: t.Any = None, - database: t.Optional[str] = None, + namespace: t.Optional[str] = None, if_not_exists: bool = True, caps: IndexCapability, exists_sql_fn: t.Any, @@ -162,8 +162,8 @@ def _generic_create_index( error is surfaced, never swallowed. """ _validate_simple_identifier(table_name, kind="table_name") - if database is not None: - _validate_simple_identifier(database, kind="database") + if namespace is not None: + _validate_simple_identifier(namespace, kind="namespace") cols = _normalize_columns(columns) for c in cols: _validate_simple_identifier(c, kind="column") @@ -186,19 +186,19 @@ def _generic_create_index( if caps.native_if_not_exists: guard = "IF NOT EXISTS " elif _generic_index_exists( - ibis_conn, index_name, table_name=table_name, database=database, + ibis_conn, index_name, table_name=table_name, namespace=namespace, exists_sql_fn=exists_sql_fn, ): return # emulated: already present where_sql = None if where is not None: - schema = ibis_conn.table(table_name, database=database).schema() + schema = ibis_conn.table(table_name, database=namespace).schema() where_sql = compile_index_predicate(ibis_conn, schema, table_name, where) sql = build_create_index_sql( dialect=dialect_of(ibis_conn), - target=_target_ref(ibis_conn, table_name, database), + target=_target_ref(ibis_conn, table_name, namespace), index_name=index_name, cols=cols, unique=unique, index_type=index_type, guard=guard, where_sql=where_sql, ) @@ -210,7 +210,7 @@ def _generic_drop_index( index_name: str, *, table_name: t.Optional[str] = None, - database: t.Optional[str] = None, + namespace: t.Optional[str] = None, if_exists: bool = True, caps: IndexCapability, exists_sql_fn: t.Any, @@ -228,20 +228,20 @@ def _generic_drop_index( ) if table_name is not None: _validate_simple_identifier(table_name, kind="table_name") - if database is not None: - _validate_simple_identifier(database, kind="database") + if namespace is not None: + _validate_simple_identifier(namespace, kind="namespace") guard = "" if if_exists: if caps.native_if_exists: guard = "IF EXISTS " elif not _generic_index_exists( - ibis_conn, index_name, table_name=table_name, database=database, + ibis_conn, index_name, table_name=table_name, namespace=namespace, exists_sql_fn=exists_sql_fn, ): return # emulated: already absent - target = _target_ref(ibis_conn, table_name, database) if table_name else None + target = _target_ref(ibis_conn, table_name, namespace) if table_name else None sql = build_drop_index_sql( dialect=dialect_of(ibis_conn), drop_scope=caps.drop_scope, index_name=index_name, target=target, guard=guard, diff --git a/src/mountainash_data/backends/ibis/backend.py b/src/mountainash_data/backends/ibis/backend.py index 9875a7c..2f3df97 100644 --- a/src/mountainash_data/backends/ibis/backend.py +++ b/src/mountainash_data/backends/ibis/backend.py @@ -663,22 +663,23 @@ def create_index( unique: bool = False, index_type: str | None = None, where: t.Any = None, # IndexPredicate | None - database: str | None = None, + namespace: NamespaceLike = None, if_not_exists: bool = True, ) -> IbisBackend: conn = self._require_connected() + rendered = _render_ibis_namespace_single(Namespace.coerce(namespace), op="create_index") hook = self._spec.create_index_hook if hook is not None: hook( conn._ibis_conn, table_name, columns, index_name=index_name, unique=unique, index_type=index_type, - where=where, database=database, if_not_exists=if_not_exists, + where=where, namespace=rendered, if_not_exists=if_not_exists, ) elif self._spec.index_caps is not None: _generic_create_index( conn._ibis_conn, table_name, columns, index_name=index_name, unique=unique, index_type=index_type, - where=where, database=database, if_not_exists=if_not_exists, + where=where, namespace=rendered, if_not_exists=if_not_exists, caps=self._spec.index_caps, exists_sql_fn=self._spec.get_index_exists_sql, ) @@ -695,11 +696,15 @@ def create_unique_index( *, index_name: str | None = None, where: t.Any = None, # IndexPredicate | None - database: str | None = None, + namespace: NamespaceLike = None, ) -> IbisBackend: + # Gate here so a catalog-qualified namespace raises naming THIS method, + # not the create_index it delegates to (the delegated call re-renders, + # but by then the namespace has passed and won't raise again). + _render_ibis_namespace_single(Namespace.coerce(namespace), op="create_unique_index") return self.create_index( table_name, columns, - index_name=index_name, unique=True, where=where, database=database, + index_name=index_name, unique=True, where=where, namespace=namespace, ) def drop_index( @@ -707,20 +712,21 @@ def drop_index( index_name: str, *, table_name: str | None = None, - database: str | None = None, + namespace: NamespaceLike = None, if_exists: bool = True, ) -> IbisBackend: conn = self._require_connected() + rendered = _render_ibis_namespace_single(Namespace.coerce(namespace), op="drop_index") hook = self._spec.drop_index_hook if hook is not None: hook( conn._ibis_conn, index_name, - table_name=table_name, database=database, if_exists=if_exists, + table_name=table_name, namespace=rendered, if_exists=if_exists, ) elif self._spec.index_caps is not None: _generic_drop_index( conn._ibis_conn, index_name, - table_name=table_name, database=database, if_exists=if_exists, + table_name=table_name, namespace=rendered, if_exists=if_exists, caps=self._spec.index_caps, exists_sql_fn=self._spec.get_index_exists_sql, ) @@ -735,16 +741,17 @@ def index_exists( index_name: str, *, table_name: str | None = None, - database: str | None = None, + namespace: NamespaceLike = None, ) -> bool: if self._spec.get_index_exists_sql is None: raise NotImplementedError( f"Dialect {self.dialect!r} does not support index_exists" ) conn = self._require_connected() + rendered = _render_ibis_namespace_single(Namespace.coerce(namespace), op="index_exists") return _generic_index_exists( conn._ibis_conn, index_name, - table_name=table_name, database=database, + table_name=table_name, namespace=rendered, exists_sql_fn=self._spec.get_index_exists_sql, ) @@ -752,14 +759,15 @@ def list_indexes( self, table_name: str, *, - database: str | None = None, + namespace: NamespaceLike = None, ) -> list[dict]: if self._spec.get_list_indexes_sql is None: raise NotImplementedError( f"Dialect {self.dialect!r} does not support list_indexes" ) conn = self._require_connected() - list_sql = self._spec.get_list_indexes_sql(table_name, database) + rendered = _render_ibis_namespace_single(Namespace.coerce(namespace), op="list_indexes") + list_sql = self._spec.get_list_indexes_sql(table_name, rendered) result = conn._ibis_conn.sql(list_sql) if result is None: return [] diff --git a/src/mountainash_data/backends/ibis/operations.py b/src/mountainash_data/backends/ibis/operations.py index 6a6f26b..20763cd 100644 --- a/src/mountainash_data/backends/ibis/operations.py +++ b/src/mountainash_data/backends/ibis/operations.py @@ -237,14 +237,18 @@ def _generic_add_columns( def duckdb_get_index_exists_sql( index_name: str, table_name: str | None, - database: str | None + namespace: str | None ) -> str: - """DuckDB uses duckdb_indexes() system function.""" + """DuckDB uses duckdb_indexes() system function. `namespace` is a single- + level qualifier, which DuckDB's ibis backend (and this package's own + `database=` convention) treats as the SCHEMA, not the catalog — so it is + matched against duckdb_indexes()'s `schema_name` column, not + `database_name` (which holds the catalog, e.g. "memory").""" where_clauses = [f"index_name = {_sql_literal(index_name)}"] if table_name: where_clauses.append(f"table_name = {_sql_literal(table_name)}") - if database: - where_clauses.append(f"database_name = {_sql_literal(database)}") + if namespace: + where_clauses.append(f"schema_name = {_sql_literal(namespace)}") where_sql = " AND ".join(where_clauses) return f"SELECT COUNT(*) as count FROM duckdb_indexes() WHERE {where_sql}" @@ -252,12 +256,14 @@ def duckdb_get_index_exists_sql( def duckdb_get_list_indexes_sql( table_name: str, - database: str | None + namespace: str | None ) -> str: - """DuckDB uses duckdb_indexes() system function.""" + """DuckDB uses duckdb_indexes() system function. See `namespace` note on + `duckdb_get_index_exists_sql` — matched against `schema_name`, not + `database_name`.""" where_clauses = [f"table_name = '{table_name}'"] - if database: - where_clauses.append(f"database_name = '{database}'") + if namespace: + where_clauses.append(f"schema_name = '{namespace}'") where_sql = " AND ".join(where_clauses) return f""" @@ -275,9 +281,9 @@ def duckdb_get_list_indexes_sql( def sqlite_get_index_exists_sql( index_name: str, table_name: str | None, - database: str | None + namespace: str | None ) -> str: - """SQLite uses the sqlite_master system table. `database` is unused (no + """SQLite uses the sqlite_master system table. `namespace` is unused (no cross-database queries).""" where_clauses = ["type = 'index'", f"name = {_sql_literal(index_name)}"] if table_name: @@ -288,10 +294,10 @@ def sqlite_get_index_exists_sql( def sqlite_get_list_indexes_sql( table_name: str, - database: str | None + namespace: str | None ) -> str: """SQLite uses sqlite_master system table. - Note: database parameter is not used as SQLite doesn't support cross-database queries. + Note: namespace parameter is not used as SQLite doesn't support cross-database queries. """ return f""" SELECT @@ -309,14 +315,16 @@ def sqlite_get_list_indexes_sql( def motherduck_get_index_exists_sql( index_name: str, table_name: str | None, - database: str | None + namespace: str | None ) -> str: - """MotherDuck uses DuckDB's duckdb_indexes() system function.""" + """MotherDuck uses DuckDB's duckdb_indexes() system function. See + `namespace` note on `duckdb_get_index_exists_sql` — matched against + `schema_name`, not `database_name`.""" where_clauses = [f"index_name = {_sql_literal(index_name)}"] if table_name: where_clauses.append(f"table_name = {_sql_literal(table_name)}") - if database: - where_clauses.append(f"database_name = {_sql_literal(database)}") + if namespace: + where_clauses.append(f"schema_name = {_sql_literal(namespace)}") where_sql = " AND ".join(where_clauses) return f"SELECT COUNT(*) as count FROM duckdb_indexes() WHERE {where_sql}" @@ -324,12 +332,14 @@ def motherduck_get_index_exists_sql( def motherduck_get_list_indexes_sql( table_name: str, - database: str | None + namespace: str | None ) -> str: - """MotherDuck uses DuckDB's duckdb_indexes() system function.""" + """MotherDuck uses DuckDB's duckdb_indexes() system function. See + `namespace` note on `duckdb_get_index_exists_sql` — matched against + `schema_name`, not `database_name`.""" where_clauses = [f"table_name = '{table_name}'"] - if database: - where_clauses.append(f"database_name = '{database}'") + if namespace: + where_clauses.append(f"schema_name = '{namespace}'") where_sql = " AND ".join(where_clauses) return f""" @@ -345,29 +355,29 @@ def motherduck_get_list_indexes_sql( # --- PostgreSQL --- def postgres_get_index_exists_sql( - index_name: str, table_name: str | None, database: str | None + index_name: str, table_name: str | None, namespace: str | None ) -> str: - """PostgreSQL pg_indexes catalog view. `database` maps to schemaname.""" + """PostgreSQL pg_indexes catalog view. `namespace` maps to schemaname.""" where = [f"indexname = {_sql_literal(index_name)}"] if table_name: where.append(f"tablename = {_sql_literal(table_name)}") - if database: - where.append(f"schemaname = {_sql_literal(database)}") + if namespace: + where.append(f"schemaname = {_sql_literal(namespace)}") return f"SELECT COUNT(*) AS count FROM pg_indexes WHERE {' AND '.join(where)}" # --- MySQL / MariaDB --- def mysql_get_index_exists_sql( - index_name: str, table_name: str | None, database: str | None + index_name: str, table_name: str | None, namespace: str | None ) -> str: """information_schema.STATISTICS (table-scoped). Defaults schema to the - current database when `database` is omitted.""" + current database when `namespace` is omitted.""" where = [f"INDEX_NAME = {_sql_literal(index_name)}"] if table_name: where.append(f"TABLE_NAME = {_sql_literal(table_name)}") schema_pred = ( - f"TABLE_SCHEMA = {_sql_literal(database)}" if database else "TABLE_SCHEMA = DATABASE()" + f"TABLE_SCHEMA = {_sql_literal(namespace)}" if namespace else "TABLE_SCHEMA = DATABASE()" ) where.append(schema_pred) return ( @@ -379,20 +389,20 @@ def mysql_get_index_exists_sql( # --- SQL Server --- def mssql_get_index_exists_sql( - index_name: str, table_name: str | None, database: str | None + index_name: str, table_name: str | None, namespace: str | None ) -> str: """sys.indexes joined to the table via OBJECT_ID (table-scoped). - NOTE on the `database` parameter: across this package `database` denotes the + NOTE on the `namespace` parameter: across this package `namespace` denotes the immediate NAMESPACE qualifier, which SQL Server interprets as the *schema* in - a two-part name. The generic CREATE renders ``"".""`` (a - schema.object reference to SQL Server), so OBJECT_ID('.
') + a two-part name. The generic CREATE renders ``""."
"`` (a + schema.object reference to SQL Server), so OBJECT_ID('.
') targets the same object — consistent, not conflated. Cross-database (three-part) index DDL is out of scope for the generic path. """ obj = table_name if table_name else "" - if database and table_name: - obj = f"{database}.{table_name}" + if namespace and table_name: + obj = f"{namespace}.{table_name}" return ( "SELECT COUNT(*) AS count FROM sys.indexes " f"WHERE name = {_sql_literal(index_name)} " @@ -403,7 +413,7 @@ def mssql_get_index_exists_sql( # --- Oracle --- def oracle_get_index_exists_sql( - index_name: str, table_name: str | None, database: str | None + index_name: str, table_name: str | None, namespace: str | None ) -> str: """user_indexes (schema-global). The generic builder ALWAYS quotes identifiers (quote_identifier), so Oracle stores them case-sensitively as @@ -418,17 +428,17 @@ def oracle_get_index_exists_sql( # --- SingleStore --- def singlestore_get_index_exists_sql( - index_name: str, table_name: str | None, database: str | None + index_name: str, table_name: str | None, namespace: str | None ) -> str: """information_schema.STATISTICS (MySQL-compatible, table-scoped). Like MySQL, ALWAYS constrain TABLE_SCHEMA — defaulting to DATABASE() when - `database` is omitted — so an index/table name shared across schemas cannot + `namespace` is omitted — so an index/table name shared across schemas cannot produce a cross-schema false positive.""" where = [f"INDEX_NAME = {_sql_literal(index_name)}"] if table_name: where.append(f"TABLE_NAME = {_sql_literal(table_name)}") schema_pred = ( - f"TABLE_SCHEMA = {_sql_literal(database)}" if database else "TABLE_SCHEMA = DATABASE()" + f"TABLE_SCHEMA = {_sql_literal(namespace)}" if namespace else "TABLE_SCHEMA = DATABASE()" ) where.append(schema_pred) return ( @@ -441,10 +451,10 @@ def singlestore_get_index_exists_sql( def motherduck_list_tables( ibis_backend: t.Any, like: str | None = None, - database: str | None = None, + namespace: str | None = None, ) -> list[str]: """MotherDuck-specific list_tables using DuckDB backend's database parameter.""" - return ibis_backend.list_tables(like=like, database=database) if ibis_backend is not None else [] + return ibis_backend.list_tables(like=like, database=namespace) if ibis_backend is not None else [] # =========================================================================== diff --git a/tests/test_unit/backends/ibis/test_index_ops.py b/tests/test_unit/backends/ibis/test_index_ops.py index 55b62ea..2cff3a2 100644 --- a/tests/test_unit/backends/ibis/test_index_ops.py +++ b/tests/test_unit/backends/ibis/test_index_ops.py @@ -147,3 +147,35 @@ def test_unsupported_dialect_raises_notimplemented(self): be.create_index("t", ["id"]) finally: be.close() + + +def test_create_index_accepts_namespace_kwarg(): + from mountainash_data import IbisBackend + + with IbisBackend(dialect="duckdb", database=":memory:") as backend: + backend.ibis_connection().raw_sql("CREATE SCHEMA idx_ns") + backend.create_table("t", {"id": [1], "name": ["a"]}, namespace="idx_ns") + backend.create_index("t", ["name"], namespace="idx_ns", index_name="idx_t_name") + assert backend.index_exists("idx_t_name", table_name="t", namespace="idx_ns") is True + + +def test_index_ops_reject_database_kwarg(): + import pytest + from mountainash_data import IbisBackend + + with IbisBackend(dialect="sqlite", database=":memory:") as backend: + backend.create_table("t", {"id": [1]}) + with pytest.raises(TypeError): + backend.create_index("t", ["id"], database="x") + + +def test_create_index_rejects_catalog_qualified_namespace(): + """Index DDL builds engine-native SQL; catalog-qualified must raise (not reach SQL).""" + import pytest + from mountainash_data import IbisBackend + from mountainash_data.core.namespace import Namespace + + with IbisBackend(dialect="duckdb", database=":memory:") as backend: + backend.create_table("t", {"id": [1]}) + with pytest.raises(ValueError, match="does not support catalog-qualified"): + backend.create_index("t", ["id"], namespace=Namespace(catalog="wh", path=("s",))) From abc61486bf592653ad01c3f2d2257fcef14ff7ea Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Fri, 3 Jul 2026 02:32:23 +1000 Subject: [PATCH 09/11] feat(iceberg): interface alignment to widened protocol; deep-path fidelity deferred to DEBT-11 --- .../backends/iceberg/backend.py | 17 ++-- .../backends/iceberg/catalogs/rest.py | 2 +- .../backends/iceberg/connection.py | 88 +++++++++++-------- .../backends/iceberg/inspect.py | 26 ++---- .../backends/iceberg/test_namespace.py | 23 +++++ 5 files changed, 92 insertions(+), 64 deletions(-) create mode 100644 tests/test_unit/backends/iceberg/test_namespace.py diff --git a/src/mountainash_data/backends/iceberg/backend.py b/src/mountainash_data/backends/iceberg/backend.py index f353437..ea19e44 100644 --- a/src/mountainash_data/backends/iceberg/backend.py +++ b/src/mountainash_data/backends/iceberg/backend.py @@ -60,19 +60,20 @@ def _require_connected(self) -> IcebergConnectionBase: ) return self._conn - def list_tables(self, namespace: str | None = None) -> list[str]: + def list_tables(self, namespace: t.Any = None) -> list[str]: return self._require_connected().list_tables(namespace=namespace) - def list_namespaces(self) -> list[str]: - return self._require_connected().list_namespaces() + def list_namespaces(self, catalog: str | None = None) -> list[str]: + return self._require_connected().list_namespaces(catalog=catalog) - def inspect_table( - self, name: str, namespace: str | None = None - ) -> t.Any: + def list_catalogs(self) -> list[str]: + return self._require_connected().list_catalogs() + + def inspect_table(self, name: str, namespace: t.Any = None) -> t.Any: return self._require_connected().inspect_table(name, namespace=namespace) def inspect_namespace(self, name: str) -> t.Any: return self._require_connected().inspect_namespace(name) - def inspect_catalog(self) -> t.Any: - return self._require_connected().inspect_catalog() + def inspect_catalog(self, catalog: str | None = None) -> t.Any: + return self._require_connected().inspect_catalog(catalog=catalog) diff --git a/src/mountainash_data/backends/iceberg/catalogs/rest.py b/src/mountainash_data/backends/iceberg/catalogs/rest.py index ff7fd1f..ceadb73 100644 --- a/src/mountainash_data/backends/iceberg/catalogs/rest.py +++ b/src/mountainash_data/backends/iceberg/catalogs/rest.py @@ -71,7 +71,7 @@ def settings_class(self) -> t.Type[BaseSettings]: def _list_tables( self, - namespace: str | None = None, + namespace: str | t.Tuple[str, ...] | None = None, ) -> t.List[str]: """Return table names within ``namespace`` from the REST catalog.""" return ( diff --git a/src/mountainash_data/backends/iceberg/connection.py b/src/mountainash_data/backends/iceberg/connection.py index b49b040..e1720f0 100644 --- a/src/mountainash_data/backends/iceberg/connection.py +++ b/src/mountainash_data/backends/iceberg/connection.py @@ -34,6 +34,7 @@ NamespaceInfo, TableInfo, ) +from mountainash_data.core.namespace import Namespace, NamespaceLike from mountainash_data.core.factories.connection_factory import build_driver_kwargs from mountainash_dataframes import DataFrameUtils, SupportedDataFrames @@ -256,29 +257,42 @@ def load_table( # Inspection (satisfies core.protocol.Connection) # ------------------------------------------------------------------ - def list_namespaces( - self, - parent: t.Optional[str | t.Tuple[str, ...]] = None, - ) -> t.Optional[list]: + def _catalog_name(self) -> str: + return getattr(self.catalog_backend, "name", "iceberg") + + def _check_catalog(self, catalog: str | None) -> None: + """iceberg's catalog IS the connection; a foreign catalog is unaddressable.""" + if catalog is not None and catalog != self._catalog_name(): + raise ValueError( + f"iceberg connection is bound to catalog {self._catalog_name()!r}; " + f"cannot address catalog {catalog!r}." + ) + + def list_catalogs(self) -> list[str]: + """One catalog per iceberg connection.""" + self.connect() + return [self._catalog_name()] + + def list_namespaces(self, catalog: str | None = None) -> list[str]: """Return all namespaces visible to this catalog connection. Args: - parent: Optional parent namespace to list sub-namespaces of. + catalog: Must match this connection's catalog name, or be None. Returns: - List of namespace tuples/names, or None if not connected. + List of namespace names (dotted for multi-level). """ self.connect() - return ( - self.catalog_backend.list_namespaces(parent) - if self.catalog_backend is not None - else None - ) - - def list_tables( - self, - namespace: str | t.Tuple[str, ...] | None = None, - ) -> list[str]: + self._check_catalog(catalog) + if self.catalog_backend is None: + return [] + raw = self.catalog_backend.list_namespaces() + # DEFERRED (DEBT-11): multi-level namespaces are joined to a dotted string; + # this is NOT round-trippable as a bare string. Structured round-trip is + # tracked in DEBT-11 (list[Namespace] vs dotted list[str] fork). + return [".".join(ns) if isinstance(ns, (tuple, list)) else str(ns) for ns in raw] + + def list_tables(self, namespace: NamespaceLike = None) -> list[str]: """Return the names of tables in ``namespace``. Args: @@ -289,11 +303,14 @@ def list_tables( List of table name strings. """ self.connect() - return self._list_tables(namespace=namespace) + ns = Namespace.coerce(namespace) + self._check_catalog(ns.catalog) + # pyiceberg accepts a namespace tuple directly (behavior-preserving). + return self._list_tables(namespace=(ns.path or None)) def _list_tables( self, - namespace: str | None = None, + namespace: str | t.Tuple[str, ...] | None = None, ) -> list[str]: """Hook for subclasses to implement namespace-scoped table listing.""" raise NotImplementedError @@ -301,7 +318,7 @@ def _list_tables( def inspect_table( self, name: str, - namespace: t.Optional[str] = None, + namespace: NamespaceLike = None, ) -> TableInfo: """Return shared-model metadata for one table. @@ -314,18 +331,13 @@ def inspect_table( """ from mountainash_data.backends.iceberg.inspect import table_to_info - identifier = (namespace, name) if namespace else name + ns = Namespace.coerce(namespace) + self._check_catalog(ns.catalog) + identifier = (*ns.path, name) if ns.path else name iceberg_table = self.table(identifier) if iceberg_table is None: raise ValueError(f"Table not found: {identifier!r}") - - catalog_name = getattr(self.catalog_backend, "name", None) - return table_to_info( - iceberg_table, - name=name, - namespace=namespace, - catalog=catalog_name, - ) + return table_to_info(iceberg_table, name=name, location=ns) def inspect_namespace(self, name: str) -> NamespaceInfo: """Return shared-model metadata for one namespace. @@ -338,11 +350,12 @@ def inspect_namespace(self, name: str) -> NamespaceInfo: """ from mountainash_data.backends.iceberg.inspect import namespace_to_info - table_names = self._list_tables(namespace=name) - catalog_name = getattr(self.catalog_backend, "name", None) - return namespace_to_info(name, table_names, catalog=catalog_name) + ns = Namespace.coerce(name) + self._check_catalog(ns.catalog) + table_names = self._list_tables(namespace=(ns.path or None)) + return namespace_to_info(ns.path, table_names) - def inspect_catalog(self) -> CatalogInfo: + def inspect_catalog(self, catalog: str | None = None) -> CatalogInfo: """Return shared-model metadata for the connection's catalog. Returns: @@ -354,19 +367,20 @@ def inspect_catalog(self) -> CatalogInfo: ) self.connect() + self._check_catalog(catalog) + catalog_name = self._catalog_name() raw_namespaces = self.catalog_backend.list_namespaces() - catalog_name = getattr(self.catalog_backend, "name", "iceberg") namespace_infos = [] for ns in raw_namespaces: + # DEFERRED (DEBT-11): deep namespaces are flattened to their first + # segment here, mirroring current behavior. Faithful round-trip is DEBT-11. ns_name = ns[0] if isinstance(ns, (tuple, list)) else str(ns) try: - table_names = self._list_tables(namespace=ns_name) + table_names = self._list_tables(namespace=(ns_name,)) except NotImplementedError: table_names = [] - namespace_infos.append( - namespace_to_info(ns_name, table_names, catalog=catalog_name) - ) + namespace_infos.append(namespace_to_info((ns_name,), table_names)) return catalog_to_info(catalog_name, namespace_infos) diff --git a/src/mountainash_data/backends/iceberg/inspect.py b/src/mountainash_data/backends/iceberg/inspect.py index 34e8963..b6bf45e 100644 --- a/src/mountainash_data/backends/iceberg/inspect.py +++ b/src/mountainash_data/backends/iceberg/inspect.py @@ -14,22 +14,21 @@ NamespaceInfo, TableInfo, ) +from mountainash_data.core.namespace import Namespace def table_to_info( iceberg_table, *, name: str, - namespace: t.Optional[str] = None, - catalog: t.Optional[str] = None, + location: Namespace = Namespace(), ) -> TableInfo: """Convert a pyiceberg Table object into a TableInfo. Args: iceberg_table: A ``pyiceberg.table.Table`` instance. name: The simple table name (without namespace prefix). - namespace: The namespace (schema) the table belongs to, if known. - catalog: The catalog name, if known. + location: The table's namespace/catalog location. Returns: A ``TableInfo`` populated from the table's current schema. @@ -42,34 +41,25 @@ def table_to_info( ) for field in iceberg_table.schema().fields ] - return TableInfo( - name=name, - columns=columns, - namespace=namespace, - catalog=catalog, - ) + return TableInfo(name=name, columns=columns, location=location) def namespace_to_info( - namespace_name: str, + namespace_path: t.Sequence[str], table_names: t.Sequence[str], - *, - catalog: t.Optional[str] = None, ) -> NamespaceInfo: - """Build a NamespaceInfo from a list of table names. + """Build a NamespaceInfo from a namespace path and its table names. Args: - namespace_name: The namespace identifier. + namespace_path: The namespace path segments. table_names: Names of tables within this namespace. - catalog: The catalog this namespace belongs to, if known. Returns: A populated ``NamespaceInfo``. """ return NamespaceInfo( - name=namespace_name, + location=Namespace(path=tuple(namespace_path)), tables=list(table_names), - catalog=catalog, ) diff --git a/tests/test_unit/backends/iceberg/test_namespace.py b/tests/test_unit/backends/iceberg/test_namespace.py new file mode 100644 index 0000000..7b24692 --- /dev/null +++ b/tests/test_unit/backends/iceberg/test_namespace.py @@ -0,0 +1,23 @@ +"""Iceberg namespace interface alignment (DEBT-10; deep-path fidelity = DEBT-11). + +NOTE: There is no live/in-memory iceberg catalog fixture in this repo (see +tests/fixtures/database_fixtures.py and test_backend.py), so this module is +limited to a pure structural check that needs no live catalog. Behavioral / +round-trip coverage against a real catalog is out of scope here and tracked +under DEBT-11. +""" + +import pytest + +pytest.importorskip("pyiceberg") + +from mountainash_data.core.namespace import Namespace, NamespaceLike # noqa: F401 + + +def test_iceberg_connection_satisfies_widened_protocol(): + """The iceberg connection base exposes the widened discovery surface.""" + from mountainash_data.backends.iceberg.connection import IcebergConnectionBase + + for meth in ("list_tables", "list_namespaces", "list_catalogs", + "inspect_table", "inspect_namespace", "inspect_catalog"): + assert hasattr(IcebergConnectionBase, meth) From cfb490fa33c1d724592e5d7835e143d1549e84e8 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Fri, 3 Jul 2026 02:40:23 +1000 Subject: [PATCH 10/11] test(namespace): two-level live + render-only spies + rename regression; migrate call sites (DEBT-10) --- .../backends/ibis/test_namespace_hierarchy.py | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 tests/test_unit/backends/ibis/test_namespace_hierarchy.py diff --git a/tests/test_unit/backends/ibis/test_namespace_hierarchy.py b/tests/test_unit/backends/ibis/test_namespace_hierarchy.py new file mode 100644 index 0000000..2328a00 --- /dev/null +++ b/tests/test_unit/backends/ibis/test_namespace_hierarchy.py @@ -0,0 +1,102 @@ +"""Namespace hierarchy support matrix (DEBT-10, spec §10). + +Live: DuckDB single-level (CREATE SCHEMA) + two-level (ATTACH). +Render-only: postgres/snowflake/bigquery — spy the raw connection to assert the +coerced Namespace renders to the correct ibis database= shape without a live DB. +DIALECTS[dialect] is import-time-safe for these three dialects — the registry's +connection_builder functions do all driver imports lazily inside the callable, +not at module import time — so no pytest.importorskip guard is needed here; +_RecordingConn stands in for the ibis connection object entirely. +Regression: the database= keyword is gone from the public surface. + +Iceberg deep-namespace round-trip is deferred to DEBT-11 (see spec §10 note). +""" + +from __future__ import annotations + +import pytest + +from mountainash_data import IbisBackend +from mountainash_data.backends.ibis.backend import IbisConnection +from mountainash_data.backends.ibis.dialects._registry import DIALECTS +from mountainash_data.core.namespace import Namespace + + +# --- Live: DuckDB two-level via ATTACH ------------------------------------ + +def test_duckdb_two_level_attach_roundtrip(tmp_path): + """A table in an ATTACHed catalog is addressable via Namespace(catalog=...).""" + other = tmp_path / "other.duckdb" + with IbisBackend(dialect="duckdb", database=":memory:") as backend: + raw = backend.ibis_connection() + raw.raw_sql(f"ATTACH '{other}' AS other_cat") + raw.raw_sql("CREATE SCHEMA other_cat.sales") + raw.raw_sql("CREATE TABLE other_cat.sales.orders (id INTEGER)") + + ns = Namespace(catalog="other_cat", path=("sales",)) + assert "orders" in backend.list_tables(namespace=ns) + assert backend.table_exists("orders", namespace=ns) is True + info = backend.inspect_table("orders", namespace=ns) + assert info.location == ns + assert info.qualified_name == "other_cat.sales.orders" + + +def test_duckdb_depth_over_one_raises(): + with IbisBackend(dialect="duckdb", database=":memory:") as backend: + with pytest.raises(ValueError, match="single namespace level"): + backend.list_tables(namespace=("a", "b")) + + +# --- Render-only spies: postgres/snowflake/bigquery ----------------------- + +class _RecordingConn: + """Records the database= value reaching ibis's native calls.""" + + def __init__(self): + self.calls: list[tuple[str, object]] = [] + + def list_tables(self, database=None): + self.calls.append(("list_tables", database)) + return [] + + def table(self, name, database=None): + self.calls.append(("table", database)) + raise RuntimeError("stop after recording") # inspect not needed here + + +@pytest.mark.parametrize("dialect", ["postgres", "snowflake", "bigquery"]) +def test_catalog_qualified_renders_tuple_to_ibis(dialect): + rec = _RecordingConn() + conn = IbisConnection(rec, DIALECTS[dialect]) + conn.list_tables(namespace=Namespace(catalog="wh", path=("sales",))) + assert rec.calls == [("list_tables", ("wh", "sales"))] + + +@pytest.mark.parametrize("dialect", ["postgres", "snowflake", "bigquery"]) +def test_single_level_renders_str_to_ibis(dialect): + rec = _RecordingConn() + conn = IbisConnection(rec, DIALECTS[dialect]) + conn.list_tables(namespace="sales") + assert rec.calls == [("list_tables", "sales")] + + +# --- Rename regression (behavioral; replaces a brittle grep) -------------- + +@pytest.mark.parametrize( + "call", + [ + lambda be: be.list_tables(database="x"), + lambda be: be.inspect_table("t", database="x"), + lambda be: be.create_table("t", {"id": [1]}, database="x"), + lambda be: be.drop_table("t", database="x"), + lambda be: be.table_exists("t", database="x"), + lambda be: be.upsert("t", {"id": [1]}, conflict_columns=["id"], database="x"), + lambda be: be.add_columns("t", {"id": "int64"}, database="x"), + lambda be: be.create_index("t", ["id"], database="x"), + lambda be: be.index_exists("i", database="x"), + ], +) +def test_database_keyword_removed_from_public_surface(call): + with IbisBackend(dialect="sqlite", database=":memory:") as backend: + with pytest.raises(TypeError): + call(backend) From b24cc62306b373aa82484dba8c995457bb9c9bc2 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Fri, 3 Jul 2026 02:53:05 +1000 Subject: [PATCH 11/11] fix(ibis): inspect_catalog scopes namespace table reads to the requested catalog (DEBT-10) Co-Authored-By: Claude Opus 4.8 (1M context) --- src/mountainash_data/backends/ibis/backend.py | 10 ++++--- .../backends/ibis/test_namespace_hierarchy.py | 26 +++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/mountainash_data/backends/ibis/backend.py b/src/mountainash_data/backends/ibis/backend.py index 2f3df97..67a2433 100644 --- a/src/mountainash_data/backends/ibis/backend.py +++ b/src/mountainash_data/backends/ibis/backend.py @@ -155,10 +155,12 @@ def inspect_namespace(self, name: str) -> NamespaceInfo: def inspect_catalog(self, catalog: str | None = None) -> CatalogInfo: """Return shared-model metadata for the connection's catalog.""" namespaces = self.list_namespaces(catalog=catalog) - ns_infos = [ - NamespaceInfo(location=Namespace(path=(ns,)), tables=self.list_tables(namespace=ns)) - for ns in namespaces - ] + ns_infos = [] + for ns in namespaces: + location = Namespace(catalog=catalog, path=(ns,)) + ns_infos.append( + NamespaceInfo(location=location, tables=self.list_tables(namespace=location)) + ) return CatalogInfo( name=catalog or self._dialect_spec.ibis_backend_name, namespaces=ns_infos, diff --git a/tests/test_unit/backends/ibis/test_namespace_hierarchy.py b/tests/test_unit/backends/ibis/test_namespace_hierarchy.py index 2328a00..ce48445 100644 --- a/tests/test_unit/backends/ibis/test_namespace_hierarchy.py +++ b/tests/test_unit/backends/ibis/test_namespace_hierarchy.py @@ -47,6 +47,32 @@ def test_duckdb_depth_over_one_raises(): backend.list_tables(namespace=("a", "b")) +def test_inspect_catalog_scopes_tables_to_attached_catalog(tmp_path): + """inspect_catalog(catalog=X) must read each namespace's tables from X. + + Two catalogs each have a `sales` schema, but only the ATTACHed + `other_cat.sales` has an `orders` table. If inspect_catalog resolved + namespaces against the CURRENT catalog (the bug) instead of the + requested one, the `sales` NamespaceInfo would come back with an empty + (or wrong-catalog) table list and location.catalog would be lost. + """ + other = tmp_path / "other.duckdb" + with IbisBackend(dialect="duckdb", database=":memory:") as backend: + raw = backend.ibis_connection() + raw.raw_sql(f"ATTACH '{other}' AS other_cat") + raw.raw_sql("CREATE SCHEMA other_cat.sales") + raw.raw_sql("CREATE TABLE other_cat.sales.orders (id INTEGER)") + # Same-named schema in the default (in-memory) catalog, deliberately + # left without an "orders" table, so a mix-up would be visible. + raw.raw_sql("CREATE SCHEMA sales") + + info = backend.inspect_catalog(catalog="other_cat") + + sales = next(ns for ns in info.namespaces if ns.name == "sales") + assert "orders" in sales.tables + assert sales.location.catalog == "other_cat" + + # --- Render-only spies: postgres/snowflake/bigquery ----------------------- class _RecordingConn: