Give TableId a catalog component

pkg-py/src/commons/_backends.py

@@ -65,8 +65,7 @@ def list_tables(self) -> list[str]:
         return [row[0] for row in rows]
 
     def quote(self, table_id: TableId) -> str:
-        parts = [table_id.schema, table_id.table] if table_id.schema else [table_id.table]
-        return ".".join(quote_identifier(part) for part in parts)
+        return ".".join(quote_identifier(part) for part in table_id.parts)
 
     def dialect(self) -> str:
         return "duckdb"
@@ -96,9 +95,16 @@ def list_tables(self) -> list[str]:
     def quote(self, table_id: TableId) -> str:
         preparer = self._engine.dialect.identifier_preparer
         quoted = preparer.quote(table_id.table)
-        if table_id.schema:
-            return f"{preparer.quote_schema(table_id.schema)}.{quoted}"
-        return quoted
+        if table_id.schema is None:
+            return quoted
+        # quote_schema() is given one component at a time: handed
+        # "ANALYTICS.PUBLIC" it produces one identifier containing a dot.
+        outer = ".".join(
+            preparer.quote_schema(part)
+            for part in (table_id.catalog, table_id.schema)
+            if part is not None
+        )
+        return f"{outer}.{quoted}"
 
     def dialect(self) -> str:
         return self._engine.dialect.name
@@ -107,6 +113,9 @@ def inspector(self) -> Callable[[TableId], bool] | None:
         inspector = sqlalchemy.inspect(self._engine)
 
         def exists(table_id: TableId) -> bool:
-            return inspector.has_table(table_id.table, schema=table_id.schema)
+            # SQLAlchemy takes every level above the table as one dotted
+            # `schema`, so a catalog is joined onto it rather than dropped.
+            outer = ".".join(table_id.parts[:-1])
+            return inspector.has_table(table_id.table, schema=outer or None)
 
         return exists

pkg-py/src/commons/_data_source.py

@@ -41,15 +41,36 @@ def _fold(name: str) -> str:
 
 @dataclass(frozen=True)
 class TableId:
-    """A table's identity, schema-qualified where the backend has schemas."""
+    """A table's identity, qualified as far as the backend has levels.
+
+    Warehouses name a table `catalog.schema.table`, so the components are
+    kept apart rather than folded into one string. Folding them means quoting
+    `ANALYTICS.PUBLIC` as a single identifier, which names a schema with a dot
+    in it rather than a catalog and a schema.
+    """
 
     table: str
     schema: str | None = None
+    catalog: str | None = None
+
+    def __post_init__(self) -> None:
+        if self.catalog is not None and self.schema is None:
+            raise ValueError(
+                "A TableId with a catalog needs a schema: there is no level "
+                "between them to leave out."
+            )
+
+    @property
+    def parts(self) -> list[str]:
+        """The components, outermost first."""
+        return [
+            part for part in (self.catalog, self.schema, self.table) if part is not None
+        ]
 
     @property
     def label(self) -> str:
         """The name the agent uses for this table."""
-        return f"{self.schema}.{self.table}" if self.schema else self.table
+        return ".".join(self.parts)
 
 
 @dataclass
@@ -326,8 +347,9 @@ def _is_frame(value: Any) -> bool:
 def normalize_table_registry(tables: Any) -> dict[str, TableId]:
     """Turn a `tables` argument into label -> `TableId`.
 
-    Strings containing dots are read as schema-qualified. A literal table name
-    containing a dot is spelled as a `TableId`.
+    Strings containing dots are read as qualified names, at most three
+    parts: catalog.schema.table. A literal table name containing a dot is
+    spelled as a `TableId`.
     """
     if isinstance(tables, (str, TableId)):
         entries: list[Any] = [tables]
@@ -352,7 +374,7 @@ def normalize_table_registry(tables: Any) -> dict[str, TableId]:
 
 def _table_entry_id(entry: Any) -> TableId:
     if isinstance(entry, TableId):
-        if not entry.table or (entry.schema is not None and not entry.schema):
+        if not all(entry.parts):
             raise ValueError("TableId entries must not contain empty name components.")
         return entry
     if not isinstance(entry, str) or not entry:
@@ -366,9 +388,16 @@ def _table_entry_id(entry: Any) -> TableId:
             "Schema-qualified entries in tables must not contain empty name "
             f"components: {entry!r}."
         )
+    if len(parts) > 3:
+        raise ValueError(
+            "A table name has at most three parts, catalog.schema.table, got "
+            f"{entry!r}. Spell a name containing a dot as a TableId."
+        )
     if len(parts) == 1:
         return TableId(table=entry)
-    return TableId(table=parts[-1], schema=".".join(parts[:-1]))
+    if len(parts) == 2:
+        return TableId(table=parts[1], schema=parts[0])
+    return TableId(table=parts[2], schema=parts[1], catalog=parts[0])
 
 
 def _check_tables_exist(backend: Backend, registry: dict[str, TableId]) -> None:

pkg-py/tests/test_data_source_engine.py

@@ -6,6 +6,7 @@
 import sqlalchemy
 
 from commons import data_source, list_tables
+from commons._backends import EngineBackend
 from commons._data_source import TableId, normalize_table_registry
 
 
@@ -41,11 +42,6 @@ def test_an_explicit_table_id_bypasses_dot_splitting() -> None:
     assert normalize_table_registry([literal]) == {"a.b": literal}
 
 
-def test_an_empty_name_component_is_rejected() -> None:
-    with pytest.raises(ValueError, match="empty name components"):
-        normalize_table_registry(["analytics."])
-
-
 def test_duplicate_labels_are_rejected() -> None:
     with pytest.raises(ValueError, match="duplicate labels"):
         normalize_table_registry(["sales", "sales"])
@@ -152,3 +148,67 @@ def broken_inspector(self: object) -> object:
 
     with pytest.raises(RuntimeError, match="connection reset"):
         data_source(engine, tables=["sales"])
+
+
+def test_the_inspector_looks_in_the_catalog_it_was_given(monkeypatch) -> None:
+    """SQLAlchemy takes every level above the table as one dotted `schema`.
+
+    Recording the argument is the only way to see this: no dialect available
+    to the test suite has three levels, so a real probe cannot distinguish
+    dropping the catalog from honouring it.
+    """
+    seen: list[str | None] = []
+
+    class _Recorder:
+        def has_table(self, table: str, schema: str | None = None) -> bool:
+            seen.append(schema)
+            return True
+
+    monkeypatch.setattr(sqlalchemy, "inspect", lambda _engine: _Recorder())
+    backend = EngineBackend(sqlalchemy.create_engine("sqlite://"))
+    exists = backend.inspector()
+    assert exists is not None
+    exists(TableId(table="ORDERS", schema="PUBLIC", catalog="ANALYTICS"))
+    assert seen == ["ANALYTICS.PUBLIC"]
+
+
+def test_a_three_part_name_quotes_each_component_separately() -> None:
+    import duckdb
+
+    from commons._backends import DuckDBBackend
+
+    table_id = normalize_table_registry("ANALYTICS.PUBLIC.ORDERS")[
+        "ANALYTICS.PUBLIC.ORDERS"
+    ]
+    quoted = DuckDBBackend(duckdb.connect()).quote(table_id)
+    assert quoted == '"ANALYTICS"."PUBLIC"."ORDERS"'
+
+
+def test_a_catalog_without_a_schema_is_refused() -> None:
+    # There is no level between them to leave out, so this is a typo rather
+    # than a shape to support.
+    with pytest.raises(ValueError, match="schema"):
+        TableId(table="t", catalog="c")
+
+
+def test_the_label_of_a_three_part_id_round_trips() -> None:
+    table_id = TableId(table="ORDERS", schema="PUBLIC", catalog="ANALYTICS")
+    assert table_id.label == "ANALYTICS.PUBLIC.ORDERS"
+    assert normalize_table_registry(table_id.label)[table_id.label] == table_id
+
+
+def test_the_engine_backend_quotes_each_component_separately() -> None:
+    # The preparer takes one component at a time; handed "ANALYTICS.PUBLIC" it
+    # produces a single identifier with a dot inside it.
+    backend = EngineBackend(sqlalchemy.create_engine("sqlite://"))
+    quoted = backend.quote(
+        TableId(table="ORDERS", schema="PUBLIC", catalog="ANALYTICS")
+    )
+    assert quoted == '"ANALYTICS"."PUBLIC"."ORDERS"'
+
+
+def test_the_engine_backend_still_quotes_a_two_part_name() -> None:
+    backend = EngineBackend(sqlalchemy.create_engine("sqlite://"))
+    assert backend.quote(TableId(table="sales", schema="analytics")) == (
+        "analytics.sales"
+    )

pkg-py/tests/test_table_names.py

@@ -0,0 +1,55 @@
+"""Table-name parsing, driven by the shared fixture.
+
+How a `tables` string splits into catalog, schema, and table is a
+cross-language contract, so the cases live in
+``tests/shared/table-names.json`` and the R suite runs the same ones. Do not
+restate a case here; add it to the fixture.
+"""
+
+from typing import Any
+
+import pytest
+
+from commons._data_source import normalize_table_registry
+
+from ._shared import load_shared_fixture
+
+CASES: list[dict[str, Any]] = load_shared_fixture("table-names")[
+    "parse_table_name"
+]["cases"]
+
+# The fixture pins the refusal as a slug; the wording belongs to each
+# language.
+ERRORS: dict[str, tuple[type[Exception], str]] = {
+    "too_many_parts": (ValueError, "at most three parts"),
+    "empty_component": (ValueError, "empty name components"),
+    "not_a_name": (TypeError, "must be a table name"),
+}
+
+
+def test_shared_fixture_covers_every_shape() -> None:
+    # A truncated fixture would silently collect zero parametrized cases and
+    # the suite would still pass, so pin the coverage the table must have.
+    assert CASES
+    part_counts = {
+        len(case["expected"]) for case in CASES if "expected" in case
+    }
+    assert part_counts == {1, 2, 3}
+    assert {case["error"] for case in CASES if "error" in case} == set(ERRORS)
+
+
+@pytest.mark.parametrize("case", CASES, ids=lambda case: case["name"])
+def test_parse_matches_the_shared_fixture(case: dict[str, Any]) -> None:
+    if "error" in case:
+        error, match = ERRORS[case["error"]]
+        with pytest.raises(error, match=match):
+            normalize_table_registry(case["input"])
+        return
+
+    (table_id,) = normalize_table_registry(case["input"]).values()
+    expected = case["expected"]
+    assert table_id.catalog == expected.get("catalog")
+    assert table_id.schema == expected.get("schema")
+    assert table_id.table == expected["table"]
+    # The label is what the agent sees, so it round-trips the input.
+    assert table_id.label == case["input"]

pkg-r/R/data-source.R

@@ -29,11 +29,11 @@
 #' @param tables Which tables to expose, used when a connection or a board is
 #'   supplied.
 #'
-#'   For a connection, a character vector of table names, schema-qualified
-#'   strings like `"schema.table"`, or `DBI::Id` objects. Defaults to every
-#'   table returned by [DBI::dbListTables()]. Strings containing dots are
-#'   interpreted as schema-qualified names; use `DBI::Id(table = "a.b")` for
-#'   literal table names containing dots. For Snowflake and Databricks
+#'   For a connection, a character vector of table names, qualified strings
+#'   like `"schema.table"` or `"catalog.schema.table"`, or `DBI::Id` objects.
+#'   Defaults to every table returned by [DBI::dbListTables()]. Strings
+#'   containing dots are interpreted as qualified names, at most three parts;
+#'   use `DBI::Id(table = "a.b")` for literal table names containing dots. For Snowflake and Databricks
 #'   connections, a `DBI::Id` ending in `catalog` or `schema` selects every
 #'   table and view in that namespace. Leaving `tables` unset selects the
 #'   current schema. A Databricks `hive_metastore` selection must include a
@@ -901,22 +901,33 @@ table_entry_id <- function(table, call = rlang::caller_env()) {
     )
   }
 
+  # strsplit() drops a trailing empty piece, so "orders." splits to "orders";
+  # check the trailing dot separately rather than accept it as a bare name.
   parts <- strsplit(table, ".", fixed = TRUE)[[1]]
-  if (any(parts == "")) {
+  if (any(parts == "") || endsWith(table, ".")) {
     cli::cli_abort(
       "Schema-qualified entries in {.arg tables} must not contain empty name components.",
       call = call
     )
   }
 
+  if (length(parts) > 3) {
+    cli::cli_abort(
+      c(
+        "A table name has at most three parts, catalog.schema.table: {.val {table}}.",
+        i = "Spell a name containing a dot as a {.cls DBI::Id}."
+      ),
+      call = call
+    )
+  }
   if (length(parts) == 1) {
     return(DBI::Id(table = table))
   }
+  if (length(parts) == 2) {
+    return(DBI::Id(schema = parts[[1]], table = parts[[2]]))
+  }
 
-  DBI::Id(
-    schema = paste(parts[-length(parts)], collapse = "."),
-    table = parts[[length(parts)]]
-  )
+  DBI::Id(catalog = parts[[1]], schema = parts[[2]], table = parts[[3]])
 }
 
 table_id_label <- function(id, call = rlang::caller_env()) {

pkg-r/tests/testthat/fixtures/shared/table-names.json

@@ -0,0 +1,51 @@
+{
+  "parse_table_name": {
+    "description": "How a `tables` string splits into name components. Warehouses name a table catalog.schema.table, so three parts are catalog, schema, and table; folding the outer parts into one schema would quote them as a single identifier containing a dot, which names a schema no warehouse has. More than three parts are refused because there is no fourth level to put them in. Errors are pinned as reason slugs; the wording belongs to each language.",
+    "cases": [
+      {
+        "name": "a bare name is just a table",
+        "input": "orders",
+        "expected": { "table": "orders" }
+      },
+      {
+        "name": "two parts are schema and table",
+        "input": "public.orders",
+        "expected": { "schema": "public", "table": "orders" }
+      },
+      {
+        "name": "three parts are catalog, schema, and table",
+        "input": "analytics.public.orders",
+        "expected": {
+          "catalog": "analytics",
+          "schema": "public",
+          "table": "orders"
+        }
+      },
+      {
+        "name": "four parts have no level to go in",
+        "input": "a.b.c.d",
+        "error": "too_many_parts"
+      },
+      {
+        "name": "an empty leading component",
+        "input": ".orders",
+        "error": "empty_component"
+      },
+      {
+        "name": "an empty middle component",
+        "input": "analytics..orders",
+        "error": "empty_component"
+      },
+      {
+        "name": "an empty trailing component",
+        "input": "orders.",
+        "error": "empty_component"
+      },
+      {
+        "name": "an empty string",
+        "input": "",
+        "error": "not_a_name"
+      }
+    ]
+  }
+}