Skip to content

Reconcile own indices with pre-existing OHDSI-standard indices - #36

Open
nicoloesch wants to merge 13 commits into
mainfrom
index-reconciliation
Open

Reconcile own indices with pre-existing OHDSI-standard indices#36
nicoloesch wants to merge 13 commits into
mainfrom
index-reconciliation

Conversation

@nicoloesch

@nicoloesch nicoloesch commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Why structural reconciliation instead of a hardcoded name map

Before merging this we checked whether the naming mismatch could be solved more simply, by making omop_index just emit the same literal names as the official OHDSI CDM DDL. It can't, cleanly:

  • The official 5.4 DDL's index names are hand-curated, not systematic.
  • Abbreviations are inconsistent: visit_det for visit_detail, ee for episode_event.
  • The column named in the index is often not the one it indexes: idx_gender indexes gender_concept_id.
  • Numeric suffixing is applied inconsistently across otherwise-similar indexes.
  • The spec has at least one outright typo: idx_concept_vocabluary_id.

A hardcoded lookup table would only ever match a database built from the exact DDL text it was transcribed from, and would fail silently against an older CDM point release, a hand-modified schema, or a future OHDSI edit.

Structural matching by column-set and uniqueness is the only axis that's actually stable against naming that is, by inspection, not principled. We're keeping the reconciliation approach in this PR for that reason.

Summary

  • Adds live, column-set-based reconciliation (_is_plain_index, _find_equivalent_index, _find_shape_conflict in cli_indexes.py) so indexes enable recognizes an index that already exists under a foreign name (e.g. from the official OHDSI CDM DDL script) and never creates a duplicate.
  • Adds capture/restore bookkeeping (a dropped_indexes table in a new reserved schema, omop_alchemy_maintenance on PostgreSQL) so indexes disable can safely drop such a foreign-named index for bulk-load speed, and a later indexes enable restores it under its original name. Includes schema-scoping to prevent collisions across two schemas.
  • Adds a ReservedSchema StrEnum (_cli_utils.py) centralizing STAGING and MAINTENANCE, guarded via reject_reserved_schema() wired into the omop_command decorator so every maintenance CLI command is protected, not just vocab load-vocab-source.
  • Normalizes clustering for vocabulary, domain, concept_class, relationship onto the same pattern already used by person/location/care_site/provider/concept: cluster directly on the primary key's own index instead of declaring a separate, redundant same-column index. Closes #. See background above on why the official DDL's version of that redundant index needs reconciliation, not replication, to support.
  • Updates cli_schema_reconcile.py so a foreign-named equivalent index reports a single "renamed" issue instead of a misleading "missing" + "unexpected" pair, and doesn't count as drift. This includes the CLUSTER comparison and in the doctor/summary rendering layers.
  • Fixes cli_vocab.py's bulk vocabulary loader silently discarding indexes disable's warning results; they now surface on VocabularyLoadReport.index_warnings and render as a dedicated panel.
  • Fixes a crash: a second indexes disable run (without an intervening enable) when a different foreign index appears on the same columns now degrades gracefully instead of raising an uncaught IntegrityError.
  • Fixes load_vocab_source()'s reserved-schema guard only covering STAGING, not MAINTENANCE, when called directly (bypassing the CLI).
  • Extracts a shared _resolve_physical_cluster_name() helper used by both manage_indexes()'s cluster step and the standalone indexes cluster command, fixing a hardcoded unique=False in the latter and removing a duplicated resolution policy that could have drifted out of sync.
  • Replaces every ad hoc status: str field across all ten cli_*.py maintenance modules with one shared typed Status StrEnum, each member carrying a Severity (OK/INFO/WARNING/ERROR, itself carrying a render style).
  • Relocates FullTextResult/FullTextAction from omop_alchemy/backends/base.py to omop_alchemy/maintenance/cli_fulltext.py.
  • Collapses manage_indexes()'s independent Optional outcome locals into a single _IndexOutcome discriminated dataclass, so two of them being set simultaneously by a future edit becomes structurally impossible.

@nicoloesch

Copy link
Copy Markdown
Collaborator Author

Converted to "draft" stage as the structure is entirely dependent on the outcome of AustralianCancerDataNetwork/oa-configurator#18

@gkennos

gkennos commented Aug 3, 2026

Copy link
Copy Markdown
Member

the solution presented here is fine, but might be more simply addressed if we change what omop_index returns to match the existing pattern of indices in https://github.com/OHDSI/CommonDataModel/tree/main/inst/ddl/5.4

exceptions include: missing episode / episode event index that should have been pulled in when those tables were merged into 5.4

@nicoloesch
nicoloesch force-pushed the index-reconciliation branch from 579f547 to a465a16 Compare August 3, 2026 06:19
@nicoloesch
nicoloesch marked this pull request as ready for review August 3, 2026 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants