Rewrite merge methods on real Core constructs so schema_translate_map applies to loaded data

.github/workflows/ci.yml

@@ -6,18 +6,20 @@ on:
 jobs:
   label-gate:
     uses: AustralianCancerDataNetwork/cava-devops/.github/workflows/label-gate.yml@main
-  build-test:
-    uses: AustralianCancerDataNetwork/cava-devops/.github/workflows/build-test-postgres.yml@main
+  build-test-sqlite:
+    uses: AustralianCancerDataNetwork/cava-devops/.github/workflows/build-test.yml@main
+  build-test-postgres:
+    uses: AustralianCancerDataNetwork/cava-devops/.github/workflows/build-test-postgres-v2.yml@main
     with:
       postgres-db: orm_loader_test
       setup-commands: |
         uv run omop-config configure orm_loader \
-          --set test_orm_db.kind=cdm \
-          --set test_orm_db.connection.dialect=postgresql+psycopg \
-          --set test_orm_db.connection.host=localhost \
-          --set test_orm_db.connection.port=5432 \
-          --set test_orm_db.connection.user=test \
-          --set test_orm_db.connection.password=test \
-          --set test_orm_db.connection.database_name=orm_loader_test \
-          --set test_orm_db.connection.test_only=true \
-          --set test_orm_db.schema_name=public
+          --set test_orm_db_pg.kind=cdm \
+          --set test_orm_db_pg.connection.dialect=postgresql+psycopg \
+          --set test_orm_db_pg.connection.host=localhost \
+          --set test_orm_db_pg.connection.port=5432 \
+          --set test_orm_db_pg.connection.user=test \
+          --set test_orm_db_pg.connection.password=test \
+          --set test_orm_db_pg.connection.database_name=orm_loader_test \
+          --set test_orm_db_pg.connection.test_only=true \
+          --set test_orm_db_pg.cdm_schema=public

docs/tables/mat_view.md

@@ -105,18 +105,22 @@ PatientSummaryMV.refresh_mv(engine, concurrently=True)
 
 This is fail-closed by design. An index created manually outside `__mv_indexes__` does not satisfy the mixin's declaration contract; declare it in the class even if another migration is responsible for creating it. Expressions, partial indexes, and other index forms are outside this simple contract and should not be represented as `MaterializedViewIndex` entries.
 
-By default, `schema=None` leaves the view name unqualified. PostgreSQL resolves that name through the connection's `search_path`, matching the behavior of existing callers. Pass `schema="reporting"` only when the caller intentionally wants an explicit schema-qualified target.
+The view's physical schema comes from the bound connection's own `schema_translate_map`, the same mechanism every other schema-aware table in this stack uses (`oa_configurator.schema_of`). `__mv_role__` declares which role a view resolves through (defaulting to `Role.PRIMARY`); pass `role=` to `create_mv()`/`refresh_mv()`/`drop_mv()` to override it for one call.
 
 ```python
-# Existing/default behavior: search_path resolves the target.
+class VocabSummaryMV(MaterializedViewMixin):
+    __mv_name__ = "vocab_summary"
+    __mv_select__ = ...
+    __mv_role__ = Role.VOCAB  # resolves via the connection's vocab schema
+
+# Uses __mv_role__ (Role.PRIMARY by default) via the connection's own schema_translate_map.
 RecentObservationMV.create_mv(engine)
 
-# Explicit schema: the target is quoted and schema-qualified.
-RecentObservationMV.create_mv(engine, schema="reporting")
-RecentObservationMV.refresh_mv(engine, schema="reporting")
+# Override for one call.
+RecentObservationMV.create_mv(engine, role=Role.VOCAB)
 ```
 
-Explicit schema targets are quoted component by component. This matters for embedded quotes, spaces, and mixed-case identifiers. It also means an unqualified mixed-case name and the same name passed with `schema=` can address different PostgreSQL relations. Keep schema selection at the call site and do not assume that this API provides `schema_translate_map`, role-token, or general multi-schema behavior.
+Every generated identifier is quoted through `oa_configurator.qualified()`, which quotes each component only when the dialect actually requires it (reserved words, mixed case, embedded quotes or spaces) — the same behavior every other Core-built query in this stack has.
 
 ## Failure handling and backend support
 
@@ -166,7 +170,3 @@ The built-in implementation is PostgreSQL-oriented. SQLite rejects materialized-
 ::: orm_loader.mappers.ConcurrentRefreshNotEligibleError
     options:
       heading_level: 3
-
-::: orm_loader.mappers.UnsupportedMaterializationDialectError
-    options:
-      heading_level: 3

pyproject.toml

@@ -82,7 +82,7 @@ testpaths = ["tests"]
 python_files = ["test_*.py"]
 python_classes = ["Test*"]
 python_functions = ["test_*"]
-addopts = "-ra"
+addopts = "-ra -m 'not db_dialect'"
 
 [tool.pyright]
 reportMissingTypeStubs = false

src/orm_loader/backends/__init__.py

@@ -1,13 +1,17 @@
 from .postgres import PostgresBackend
 from .resolve import resolve_backend
 from .sqlite import SQLiteBackend
-from .base import BackendCapabilities, DatabaseBackend, STAGING_SCHEMA, Dialect
+from .base import (
+    BackendCapabilities,
+    DatabaseBackend,
+    Dialect,
+    STAGING_SCHEMA,
+)
 from ..mappers.materialised_view_errors import (
     ConcurrentRefreshNotEligibleError,
     MaterializationError,
     MaterializationFailure,
     MaterializationOperation,
-    UnsupportedMaterializationDialectError,
 )
 
 __all__ = [
@@ -21,6 +25,5 @@
     "PostgresBackend",
     "STAGING_SCHEMA",
     "SQLiteBackend",
-    "UnsupportedMaterializationDialectError",
     "resolve_backend",
 ]

src/orm_loader/backends/base.py

@@ -3,7 +3,6 @@
 from abc import ABC, abstractmethod
 from contextlib import AbstractContextManager, contextmanager, nullcontext
 from dataclasses import dataclass
-from enum import Enum
 from collections.abc import Generator
 from functools import wraps
 from typing import TYPE_CHECKING, Any, Callable, ParamSpec, Type, TypeVar, cast
@@ -13,6 +12,8 @@
 from sqlalchemy.engine import Connection, Engine
 from sqlalchemy.sql.compiler import IdentifierPreparer
 
+from oa_configurator import Dialect, Role
+
 if TYPE_CHECKING:
     from ..loaders.data_classes import LoaderContext
     from ..mappers.materialised_view_contracts import MaterializedViewIndex
@@ -34,13 +35,6 @@ class BackendCapabilities:
     supports_materialized_views: bool = False
 
 
-class Dialect(str, Enum):
-    """Supported SQLAlchemy dialect names."""
-
-    SQLITE = "sqlite"
-    POSTGRESQL = "postgresql"
-
-
 STAGING_SCHEMA: str = "staging"
 
 P = ParamSpec("P")
@@ -192,6 +186,41 @@ def _as_connection(
         self,
         bind: Engine | Connection,
     ) -> Generator[Connection, None, None]:
+        """
+        Normalize a bind into an open connection, guarding its dialect.
+
+        Every backend method that takes a ``bind`` should route it through
+        this context manager rather than opening a connection itself, so the
+        dialect guard below applies uniformly.
+
+        Parameters
+        ----------
+        bind : Engine or Connection
+            An Engine opens a new connection and transaction scoped to this
+            context manager, committing on a clean exit. A Connection is
+            forwarded as-is; its transaction is owned by the caller, and
+            passing the same Connection into several backend calls groups
+            them into one shared transaction.
+
+        Yields
+        ------
+        Connection
+            An open connection whose dialect matches ``self.dialect``.
+
+        Raises
+        ------
+        TypeError
+            If ``bind``'s dialect does not match ``self.dialect``. Guards
+            against a bind resolved through a different backend being
+            passed directly into a method on this one.
+        """
+        if bind.dialect.name != self.dialect.value:
+            raise TypeError(
+                f"{self.name} backend received a {bind.dialect.name!r} connection; "
+                f"expected {self.dialect.value!r}. The bind passed to this method must "
+                "be the same one (or share the same dialect as) the bind resolve_backend() "
+                "was given."
+            )
         if isinstance(bind, Engine):
             with bind.begin() as conn:
                 yield conn
@@ -268,7 +297,6 @@ def merge_replace(
         self,
         table_cls: Type["CSVTableProtocol"],
         session: so.Session,
-        target_name: str,
         pk_cols: list[str],
         *,
         merge_batch_size: int | None = None,
@@ -280,7 +308,6 @@ def merge_upsert(
         self,
         table_cls: Type["CSVTableProtocol"],
         session: so.Session,
-        target_name: str,
         pk_cols: list[str],
         *,
         merge_batch_size: int | None = None,
@@ -292,7 +319,6 @@ def merge_insert(
         self,
         table_cls: Type["CSVTableProtocol"],
         session: so.Session,
-        target_name: str,
         *,
         merge_batch_size: int | None = None,
     ) -> None:
@@ -345,14 +371,16 @@ def create_materialized_view(
         name: str,
         selectable: sa.sql.Select[Any],
         *,
-        schema: str | None = None,
+        role: Role = Role.PRIMARY,
         with_data: bool = True,
         if_not_exists: bool = True,
     ) -> None:
         """Create a materialized view for the supplied selectable.
 
-        ``schema`` defaults to ``None``, leaving the target unqualified for
-        the connection's ``search_path`` to resolve.
+        The view's schema is the bind's own ``schema_translate_map`` entry
+        for *role* (via ``oa_configurator.schema_of``), letting a view
+        built over vocab/results-role tables land in that role's own
+        schema instead of always primary.
         """
         raise NotImplementedError(
             f"Backend '{self.name}' has not implemented create_materialized_view()"
@@ -364,12 +392,17 @@ def refresh_materialized_view(
         bind: "Engine | Connection",
         name: str,
         *,
-        schema: str | None = None,
+        role: Role = Role.PRIMARY,
         concurrently: bool = False,
         declared_indexes: tuple["MaterializedViewIndex", ...] = (),
     ) -> None:
         """Refresh a materialized view.
 
+        The view's schema is the bind's own ``schema_translate_map`` entry
+        for *role* (via ``oa_configurator.schema_of``), letting a view
+        built over vocab/results-role tables land in that role's own
+        schema instead of always primary.
+
         ``declared_indexes`` lets supporting backends validate a concurrent
         refresh request without defining a second catalog-based eligibility
         rule. Other backends may ignore it.
@@ -384,16 +417,18 @@ def drop_materialized_view(
         bind: "Engine | Connection",
         name: str,
         *,
-        schema: str | None = None,
+        role: Role = Role.PRIMARY,
         if_exists: bool = True,
         cascade: bool = False,
     ) -> None:
         """Drop a materialized view.
 
-        This is deliberately non-abstract: the default implementation
-        requires the capability flag and then raises ``NotImplementedError``.
-        Older third-party backend subclasses need no override to receive a
-        clear error when they do not support this operation.
+        The view's schema is the bind's own ``schema_translate_map`` entry
+        for *role*, matching ``create_materialized_view``. This is
+        deliberately non-abstract: the default implementation requires the
+        capability flag and then raises ``NotImplementedError``. Older
+        third-party backend subclasses need no override to receive a clear
+        error when they do not support this operation.
         """
         raise NotImplementedError(
             f"Backend '{self.name}' has not implemented drop_materialized_view()"
@@ -406,13 +441,15 @@ def create_materialized_view_index(
         name: str,
         index: "MaterializedViewIndex",
         *,
-        schema: str | None = None,
+        role: Role = Role.PRIMARY,
         if_not_exists: bool = True,
     ) -> None:
         """Create an index on a materialized view.
 
-        This is deliberately non-abstract for the same compatibility reason
-        as :meth:`drop_materialized_view`.
+        The view's schema is the bind's own ``schema_translate_map`` entry
+        for *role*, matching ``create_materialized_view``. This is
+        deliberately non-abstract for the same compatibility reason as
+        :meth:`drop_materialized_view`.
         """
         raise NotImplementedError(
             f"Backend '{self.name}' has not implemented "