Rewrite merge methods on real Core constructs so schema_translate_map applies to loaded data - #39
nicoloesch wants to merge 24 commits into
Conversation
…t SQLAlchemy core objects, update test suite and unify shared backend tests, update schema registration
gkennos
left a comment
There was a problem hiding this comment.
raise dependency base for oa-configurator
also depending how you resolve the comment that remains here, you may need to update oa-configurator --> sql.py as well (validate_schema_tag)
| @@ -120,11 +122,11 @@ def manage_indices( | |||
| table_name = cls.__tablename__ | |||
|
|
|||
| indices = list(cls.__table__.indexes) if resolved_index_strategy == "drop_rebuild" else [] | |||
There was a problem hiding this comment.
manage_indices now defers schema resolution until there are indexes to rebuild, which fixes the crash under index_strategy="keep", so that's good, but postgres still defaults to drop_rebuild so a legit literal schema still calls validate_schema_tag, which raises for anything that isn't a registered Role
Please add a third case that preserves any unrecognised string as a literal physical schema, or explicitly narrow/version the public contract to document that literal schemas are no longer supported.
Summary
Depends on AustralianCancerDataNetwork/oa-configurator#37.
Requires
pyproject.tomlversion update foroa-configuratorBreaking change
OrmLoaderConfig.test_orm_dbis gone, replaced bytest_orm_db_pg/test_orm_db_sqlite. Anyone withtest_orm_dbset in theiromop.configwill need to update it. There is no back-compat shim. Needed because dialect-specific integration tests require a guaranteed-correct-dialect test database each, which one shared field couldn't express.Root cause
Four related ways
schema_translate_mapwas silently bypassed across orm-loader:merge_replace/merge_insert/merge_upsertbuilt raw, unqualifiedtext()SQLsa.inspect()schemastring instead of resolving through the connection, andFix
Loaded/merged data
merge_replace/merge_insert/merge_upsertrewritten against realTableobjects anddialect-specific Core constructs (
sa.delete,insert().from_select(),postgresql.insert().on_conflict_do_nothing(), with a SQLite-specific counterpart), for both backends.isolation_level="AUTOCOMMIT"call site.connection()to prevent data lossMaterialized views
create_mv/refresh_mv/drop_mv/index creation are fully role-awareschema_translate_map(__mv_schema_tag__, defaulting toRole.PRIMARY)_as_connection()context manager, replacing a narrower per-method check that only ever guarded direct/manualPostgresBackend()construction.manage_indices) now resolves itsInspectorschema-aware, viaphysical_schema_of(session, schema_tag=validate_schema_tag(cls.__table__)).Split primary/vocab connections
Config, CI, test infrastructure
STAGING_SCHEMA's reserved-schema registration moved intoconfig.py(the actualomop.configentry point), so it's guaranteed to run whenever this package's config is resolvableOrmLoaderConfig.test_orm_dbsplit intotest_orm_db_pg/test_orm_db_sqlitefor dialect-specific test configurationsengine_with_replica_rolenow usesautocommit_connection()'sEnginebranch directlyDialectsystem in favour of the unifiedoa-configuratoroneoa-configurator's new test infrastructure (isolated_test_database,DIALECT_PARAMS,db_dialectmarker).-
test_schema_translate_map.py: the actual regression test for the originally-reportedbug,
test_split_connection.py,test_shared_backend.py: merge-method contract tests unified across both dialects,test_reserved_schema.py: provesSTAGING_SCHEMAregistration is actuallyenforced cross-package
Schema drift protection, primitive consolidation
helpers/bootstrap.py::create_db/bootstrap()is now wrapped inguard_schema_provenance_forvia oa-configurator'sopen_connectionto prevent schema drifthelpers/sql.py::qualify_identifierdeleted entirelyqualified()backends/base.py::_as_connectionnow delegates its Engine/Connection branch to oa-configurator'sopen_connection.physical_schema_ofstack-widevalidate_schema_tagusage inmaterialised_view_mixin.pyandschema_if_supportedusage inbackends/sqlite.py.Checklist
breaking,feature,fix,dependencies, orchore)uv run pytest -q): 246 passed, 1 xfaileduv run pytest -q -m postgresql): 37 passeduv run ruff check .)