Mv public only

[gkennos]

Summary

Pulls current omop-alchemy mv implementation (with richer details like indexation) upstream in preparation for retiring downstream definitions

Checklist

• Applied exactly one label (breaking, feature, fix, dependencies, or chore)
• Tests pass locally (uv run pytest -q)
• Lint passes (uv run ruff check .)

[Copilot]

🟡 Changes recommended

There are confirmed runtime/test robustness issues around psycopg optionality and Postgres dialect guarding that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR upstreams a richer, public-only materialized-view (MV) implementation into orm_loader, adding index declarations, explicit drop support, and improved lifecycle orchestration/error reporting (especially for PostgreSQL), along with comprehensive tests and updated documentation.

Changes:

• Extended MV lifecycle APIs to support schema qualification, WITH NO DATA, IF NOT EXISTS, declared MV indexes, and drop_mv().
• Added DDL contracts (MaterializedViewIndex, CreateMaterializedViewIndex, DropMaterializedView) plus structured MV error types for operational handling.
• Expanded backend + mapper test coverage and refreshed documentation to formalize the ownership boundary and usage patterns.

File summaries

File	Description
tests/mappers/test_materialised_view_mixin.py	Adds tests for mixin lifecycle forwarding, index creation, and dependency refresh ordering.
tests/mappers/test_materialised_view_contracts.py	Adds tests validating MV index contracts and DDL compilation.
tests/backends/test_sqlite_backend.py	Adds assertions that MV drop/index APIs remain unsupported on SQLite.
tests/backends/test_postgres_backend.py	Adds extensive MV lifecycle tests for PostgreSQL including concurrent refresh behavior and error translation.
tests/backends/test_base_backend.py	Updates backend protocol coverage and verifies default MV drop/index behavior in base backend.
src/orm_loader/mappers/materialised_view_mixin.py	Extends MV mixin API (schema/with_data/if_not_exists/indexes/drop) and forwards declared indexes to backends.
src/orm_loader/mappers/materialised_view_contracts.py	Introduces MV index + DDL contract types and compilers.
src/orm_loader/backends/sqlite.py	Updates method signatures for MV APIs while preserving “unsupported” behavior via capability checks.
src/orm_loader/backends/postgres.py	Implements MV index creation, MV drop, schema qualification, concurrent refresh eligibility checks + error translation.
src/orm_loader/backends/materialized_view_errors.py	Adds structured MV error model for create/refresh/drop/index operations.
src/orm_loader/backends/base.py	Extends backend abstract API; adds default implementations for MV drop/index for compatibility.
src/orm_loader/backends/init.py	Re-exports MV error types from the backends package.
docs/tables/mat_view.md	Substantially expands MV documentation to define the public downstream contract and lifecycle semantics.

Review details

Suppressed comments (1)

tests/backends/test_postgres_backend.py:353

• Same issue here: avoid a hard dependency on psycopg in the test suite by using pytest.importorskip("psycopg") and then referencing psycopg.errors.*.

def test_postgres_backend_refresh_concurrently_unrelated_operational_error_propagates_unchanged():
    import psycopg.errors

• Files reviewed: 13/13 changed files
• Comments generated: 5
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

PostgresBackend.refresh_materialized_view currently skips the dialect guard when concurrently=False, which undermines the intended safety/consistency of the new defense-in-depth dialect checks.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/orm_loader/mappers/materialised_view_contracts.py:7

• The module docstring states callers "must" provide a fully-qualified quoted target string, but these DDL elements also work with unquoted identifiers (and quoting is only required when names need escaping). This wording is likely to mislead downstream users about what's actually required.

• Files reviewed: 14/14 changed files
• Comments generated: 1
• Review effort level: Lite

[nicoloesch]

Some changes and clarifications required, a bit more than I first thought. A few things came up that go beyond minor cleanup, like inconsistent error handling across the view/refresh/drop methods and a case where an unquoted identifier could produce invalid SQL. Worth sorting those before merge. The rest is smaller stuff, mostly clarifications and small modifications. Overall the change follows the style already established in the codebase, which will be partially reworked by me but may prompt another iteration to buff out some rough edges!