Overhauled staging tables with schemas and centralised specifiers

[nicoloesch]

Summary

• Fixes Make staging table location schema-aware instead of relying on the _staging_ prefix #15

• Backend now owns staging table naming. staging_name_for_table(tablename) is a @staticmethod @abstractmethod on DatabaseBackend

  • each concrete backend declares its own naming strategy
  • PostgresBackend and SQLiteBackend both implement f"_staging_{tablename}" (Postgres relies on schema isolation; SQLite needs the prefix for collision avoidance).

• staging_name removed as a parameter from all backend abstract methods

  • create_staging_table,
  • drop_staging_table,
  • merge_replace,
  • merge_upsert,
  • merge_insert,
  • load_staging_fast

• Backends derive the name from table_cls.__tablename__ internally via staging_name_for_table.

• qualify_identifier(name, schema) extracted to helpers/sql.py and exported from helpers/__init__.

  • Used by both backends/base.py and loaders/loading_helpers.py, replacing two independent inline f-strings.

• Circular import (backends/postgres -> loaders/loading_helpers -> helpers -> helpers/bulk -> backends/resolve -> backends/postgres) fixed by making resolve_backend imports in helpers/bulk.py lazy (inside function bodies).

• STAGING_SCHEMA: str = "staging" convention constant introduced in backends/base.py and exported from backends/__init__

  • Named as a convention (not a constructor default)
  • The base class __init__ defaults staging_schema to None (schema-agnostic).
  • PostgresBackend.__init__ opts in by defaulting to STAGING_SCHEMA.
  • SQLite always passes None.
  • Future schema-capable backends (e.g. DuckDB) follow the Postgres pattern.

• Dead _staging_tablename: ClassVar[Optional[str]] = None on CSVLoadableTableInterface removed

  • staging_tablename() classmethod removed from model and from CSVTableProtocol.

• FakeBackend in tests updated to match current abstract interface (stale staging_name positional params removed; staging_name_for_table implemented).

[gkennos]

this adds quotes manually but does not escape quotes already present

>>> from orm_loader.helpers import qualify_identifier
>>> print(qualify_identifier("table", 'schema"name'))
"schema"name"."table"

I think we need to make this dialect specific and use the inbuilt sqlalchemy functionality instead

eg in posgres backend...

identifier_preparer = postgresql.dialect().identifier_preparer

Then pass that preparer to qualify_identifier()

SQLAlchemy can quote / escape identifiers per backend

[gkennos]

this makes the use of a staging schema compulsory, right? issue says if they don't pass a schema they should keep current behaviour, therefore I'd expect the default to be none instead?

def __init__(
    self,
    *,
    staging_schema: str | None = None,
) -> None:

this means if you don't configure a staging schema it'll fail unless it already exists

Concept.load_csv(session, path)
# CREATE TABLE "staging"."_staging_concept" ...
# Fails if "staging" schema not there.

• it works for omop-alchemy because that load path explicitly creates the schema first
• PostgreSQL tests work because fixture creates the schema before every test
• failure mode is therefore currently hidden

Looks like it was based on the assumption that all PostgreSQL consumers would follow the same convention as OMOP_Alchemy and provision that schema beforehand?

[gkennos]

I think load_csv needs to accept the resolved backend (that contains staging schema details) OR a staging_schema argument?

backend = PostgresBackend(staging_schema="scratch")
backend.create_staging_table(Concept, session)
# Creates "scratch"."_staging_concept"
backend.merge_insert(Concept, session, "concept")
# Reads from "scratch"."_staging_concept"

and in the omop-alchemy vocab load pathway, you're defaulting to 'staging', so all of the defaults match, but if you did it with a different schema that doesn't match the default, since it's not threaded through to all steps under load_csv, it will fail