From f3a3e8f0789c1cddfa6c021d03073e1aef56933c Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Thu, 2 Jul 2026 15:48:02 +1000 Subject: [PATCH 1/2] fix(ibis): table_exists honors its database= argument (DEBT-9) IbisBackend.table_exists accepted a database qualifier but called self.list_tables() with no argument, so the check was always scoped to the default namespace. A namespace-tenancy consumer (schema/catalog per tenant) could not trust it: a false negative re-creates an existing table, a false positive skips creation. Forward the qualifier through list_tables(namespace=database). ibis exposes no native table_exists, so the membership check over list_tables is the correct (and only) mechanism. Tests: - functional: a table living only in a non-default DuckDB schema is found via database= and not in the default namespace, and vice versa. - golden: assert database reaches list_tables (guards the swallowed-error path where list_tables returns [] and a bool-only test gives false confidence). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/mountainash_data/backends/ibis/backend.py | 5 +-- tests/test_unit/backends/ibis/test_backend.py | 33 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/mountainash_data/backends/ibis/backend.py b/src/mountainash_data/backends/ibis/backend.py index cf3aecd..75ebea9 100644 --- a/src/mountainash_data/backends/ibis/backend.py +++ b/src/mountainash_data/backends/ibis/backend.py @@ -483,8 +483,9 @@ def table(self, name: str, *, database: str | None = None) -> t.Any: def table_exists( self, name: str, database: str | None = None ) -> bool: - tables = self.list_tables() - return name in tables + # 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) 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 051a6b8..253ccf9 100644 --- a/tests/test_unit/backends/ibis/test_backend.py +++ b/tests/test_unit/backends/ibis/test_backend.py @@ -320,6 +320,39 @@ 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. + """ + 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") 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 + + +def test_table_exists_forwards_database_to_introspection(mocker): + """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. + """ + with IbisBackend(dialect="sqlite", database=":memory:") as backend: + spy = mocker.patch.object(backend, "list_tables", return_value=["sleep"]) + assert backend.table_exists("sleep", database="tenant_a") is True + spy.assert_called_once_with(namespace="tenant_a") + + def test_fluent_chaining(): """Multiple fluent calls can be chained.""" with IbisBackend(dialect="sqlite", database=":memory:") as backend: From e88acd104eb117f824464b2bb2ff9550c0fc3588 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Thu, 2 Jul 2026 16:00:44 +1000 Subject: [PATCH 2/2] test(ibis): use monkeypatch not mocker for table_exists forwarding test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CI test env does not install pytest-mock, so the `mocker` fixture is unavailable and the golden test errored at setup. Rewrite the forwarding assertion with the built-in `monkeypatch` fixture — same check (database reaches list_tables as namespace=), no extra dependency. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_unit/backends/ibis/test_backend.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/test_unit/backends/ibis/test_backend.py b/tests/test_unit/backends/ibis/test_backend.py index 253ccf9..6f0896b 100644 --- a/tests/test_unit/backends/ibis/test_backend.py +++ b/tests/test_unit/backends/ibis/test_backend.py @@ -340,7 +340,7 @@ def test_table_exists_honors_database_namespace(): assert backend.table_exists("main_only", database="tenant_a") is False -def test_table_exists_forwards_database_to_introspection(mocker): +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 ``[]`` @@ -348,9 +348,15 @@ def test_table_exists_forwards_database_to_introspection(mocker): never forwards ``database``. Assert the forwarding directly. """ with IbisBackend(dialect="sqlite", database=":memory:") as backend: - spy = mocker.patch.object(backend, "list_tables", return_value=["sleep"]) + seen: dict[str, str | None] = {} + + 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 - spy.assert_called_once_with(namespace="tenant_a") + assert seen == {"namespace": "tenant_a"} def test_fluent_chaining():