Quotes

CHANGELOG.md

@@ -118,3 +118,8 @@
 - `manage_indexes()` now runs `ANALYZE` immediately after creating any new index, so the planner picks it up right away instead of waiting on autovacuum
 - added `--cluster/--no-cluster` flag to `indexes enable`, to skip the full heap rewrite on large vocabulary tables when disk headroom is limited
 - `indexes enable` now reports a pre-existing expression-based index as skipped instead of raising, since SQLite's reflection can't see it to begin with (affects the new functional indexes under the test backend)
+
+
+## 0.8.1
+- **breaking:** `load-vocab-source` now defaults `quote_mode` to `by_delimiter` (was `auto`). For tab-delimited Athena input this resolves to `literal`: double-quotes are treated as data and preserved verbatim, rather than sniffed and potentially stripped. This is deterministic and avoids `auto` misfiring when quoting first appears beyond the sample window. Sources that genuinely RFC-4180-wrap fields (see 0.6.3) must now pass `--quote-mode csv` (or `auto`) explicitly to strip wrapping quotes and avoid overflowing `VARCHAR(255)` columns such as `concept_name`.
+- bump orm loader version

omop_alchemy/maintenance/cli_vocab.py

@@ -44,6 +44,7 @@
 )
 
 MergeStrategy: TypeAlias = Literal["replace", "upsert", "insert_if_empty"]
+QuoteMode: TypeAlias = Literal["by_delimiter", "auto", "csv", "literal"]
 
 VocabularyModel: TypeAlias = type[CSVTableProtocol]
 VocabularyLoadProgressCallback: TypeAlias = Callable[["VocabularyLoadProgress"], None]
@@ -250,6 +251,7 @@ def load_vocab_source(
     db_schema: str | None = None,
     dry_run: bool = False,
     merge_strategy: MergeStrategy = "replace",
+    quote_mode: QuoteMode = "by_delimiter",
     chunksize: int | None = 100_000,
     bulk_mode: bool = True,
     merge_batch_size: int | None = None,
@@ -365,7 +367,7 @@ def _set_search_path(dbapi_conn, _record):
                     row_count=None,
                     csv_path=str(csv_path),
                     required=required,
-                    detail="Athena CSV would be loaded via staged ORM CSV loader using tab-delimited input and auto-detected quote mode",
+                    detail=f"Athena CSV would be loaded via staged ORM CSV loader (quote_mode={quote_mode!r})",
                 ))
                 continue
 
@@ -396,7 +398,7 @@ def _set_search_path(dbapi_conn, _record):
                             model=model,
                             csv_path=csv_path,
                             merge_strategy=merge_strategy,
-                            quote_mode="auto",
+                            quote_mode=quote_mode,
                             index_strategy="keep" if _use_bulk_mode else "auto",
                             chunksize=chunksize,
                             merge_batch_size=merge_batch_size,
@@ -423,7 +425,7 @@ def _set_search_path(dbapi_conn, _record):
                 row_count=row_count,
                 csv_path=str(csv_path),
                 required=required,
-                detail="Athena CSV loaded via staged ORM CSV loader using tab-delimited input and auto-detected quote mode",
+                detail=f"Athena CSV loaded via staged ORM CSV loader (quote_mode={quote_mode!r})",
             ))
 
             _emit(
@@ -503,6 +505,17 @@ def load_vocab_source_command(
             "`insert_if_empty` is the fast path for a fresh empty target."
         ),
     ),
+    quote_mode: QuoteMode = typer.Option(
+        "by_delimiter",
+        help=(
+            "How the vocabulary CSV loader interprets double-quotes. `by_delimiter` (default) "
+            "derives the mode from the delimiter with no content scan: tab-delimited input "
+            "is treated as literal (double-quotes are data), comma-delimited input uses "
+            "RFC-4180 quoting. This is deterministic and safe for a fresh full vocabulary "
+            "load. `auto` sniffs a sample of rows and can misfire when quoting first appears "
+            "beyond the sample window. `csv` / `literal` force one mode for every file."
+        ),
+    ),
     staging_chunk_size: int | None = typer.Option(
         100_000,
         help=(
@@ -578,6 +591,7 @@ def _update_progress(event: VocabularyLoadProgress) -> None:
             db_schema=conn.db_schema,
             dry_run=dry_run,
             merge_strategy=merge_strategy,
+            quote_mode=quote_mode,
             chunksize=None if staging_chunk_size == 0 else staging_chunk_size,
             bulk_mode=bulk_mode,
             merge_batch_size=merge_batch_size,

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "omop-alchemy"
-version = "0.8.0"
+version = "0.8.1"
 description = "SQLAlchemy-based models, validation, and utilities for the OHDSI OMOP Common Data Model"
 readme = "README.md"
 requires-python = ">=3.12"
@@ -36,7 +36,7 @@ dependencies = [
   "oa-configurator==0.1.2",
   "typer>=0.12",
   "rich>=13.0",
-  "orm-loader==0.5.1",
+  "orm-loader==0.5.2",
 ]
 
 [project.optional-dependencies]

tests/test_load_vocab_postgres.py

@@ -75,41 +75,79 @@ def test_end_to_end_vocab_load_on_postgres(pg_session, pg_engine, tmp_path):
 
 
 @pytest.mark.requires_resource(OmopAlchemyConfig.TEST_DB)
-def test_quote_mode_auto_regression_on_postgres(pg_session, pg_engine, tmp_path):
+def test_default_quote_mode_preserves_literal_quotes_on_postgres(pg_session, pg_engine, tmp_path):
     """
-    quote_mode='auto' strips RFC-4180 double-quotes via PostgreSQL COPY.
+    The default quote_mode='by_delimiter' treats double-quotes in tab-delimited
+    Athena input as literal data, not RFC-4180 field wrappers, so a
+    concept_name containing double-quotes is stored verbatim via COPY.
+
+    Athena vocabulary exports are tab-delimited and do not RFC-4180-quote
+    fields, so a double-quote in the source is part of the value and must
+    survive the load intact. (Callers with genuinely quote-wrapped sources can
+    opt into stripping with quote_mode='csv' — see the companion test below.)
+    """
+    source_path = tmp_path / "athena_source"
+    source_path.mkdir()
+
+    # Begins and ends with a literal double-quote: under RFC-4180 (csv) these
+    # boundary quotes would be stripped as wrappers; under the by_delimiter
+    # default (tab -> literal) they are data and must be preserved verbatim.
+    # Comfortably within VARCHAR(255).
+    quoted_name = '"BRCA1 positive"'
+
+    for table_name, data in _ATHENA_FIXTURE_DATA.items():
+        if table_name != "concept":
+            _write_fixture_csv(source_path, table_name, data)
+
+    concept_cols = list(_ATHENA_FIXTURE_DATA["concept"].keys())
+    concept_row = [1, quoted_name, "Gender", "Gender", "Gender", "S", "TEST", "19700101", "20991231", None]
+    _write_fixture_csv(source_path, "concept", {col: (val,) for col, val in zip(concept_cols, concept_row)})
 
-    Under the old quote_mode='literal' a concept_name of exactly 255 chars
-    wrapped in double-quotes would be stored as 257 chars and violate the
-    VARCHAR(255) constraint. This test would fail under literal mode.
+    # Default quote_mode resolves by_delimiter -> literal for tab-delimited input.
+    load_vocab_source(pg_engine, source_path=source_path)
+
+    concept_name = pg_session.execute(
+        sa.text("SELECT concept_name FROM concept WHERE concept_id = 1")
+    ).scalar()
+    assert concept_name == quoted_name, (
+        f"by_delimiter default must preserve literal double-quotes verbatim; got {concept_name!r}"
+    )
+
+
+@pytest.mark.requires_resource(OmopAlchemyConfig.TEST_DB)
+def test_explicit_csv_quote_mode_strips_quotes_on_postgres(pg_session, pg_engine, tmp_path):
+    """
+    Passing quote_mode='csv' explicitly still applies RFC-4180 stripping via
+    PostgreSQL COPY, for sources that genuinely wrap fields in double-quotes.
+
+    A concept_name of exactly 255 chars wrapped in quotes is 257 chars in the
+    file; csv mode strips the wrappers back to 255 so it fits VARCHAR(255).
+    Under the by_delimiter default this same input would overflow, which is the
+    reason the opt-in exists (see CHANGELOG 0.6.3 / 0.8.1).
     """
     source_path = tmp_path / "athena_source"
     source_path.mkdir()
 
-    long_name = "A" * 255  # exactly at VARCHAR(255) limit when unquoted
+    long_name = "A" * 255  # exactly at the VARCHAR(255) limit once unwrapped
 
-    # All tables except concept get the standard fixture data.
     for table_name, data in _ATHENA_FIXTURE_DATA.items():
         if table_name != "concept":
             _write_fixture_csv(source_path, table_name, data)
 
-    # Concept gets a single row whose name is wrapped in RFC-4180 double-quotes
-    # so the raw file value is 257 chars. quote_mode='auto' must strip them.
     concept_cols = list(_ATHENA_FIXTURE_DATA["concept"].keys())
     concept_row = [1, f'"{long_name}"', "Gender", "Gender", "Gender", "S", "TEST", "19700101", "20991231", None]
     _write_fixture_csv(source_path, "concept", {col: (val,) for col, val in zip(concept_cols, concept_row)})
 
-    # Should not raise: literal mode would produce a 257-char value and fail.
-    load_vocab_source(pg_engine, source_path=source_path)
+    load_vocab_source(pg_engine, source_path=source_path, quote_mode="csv")
 
     concept_name = pg_session.execute(
         sa.text("SELECT concept_name FROM concept WHERE concept_id = 1")
     ).scalar()
     assert concept_name is not None
     assert len(concept_name) == 255, (
-        f"Expected 255-char name; got {len(concept_name)}: {concept_name!r}"
+        f"Expected 255-char name after csv stripping; got {len(concept_name)}: {concept_name!r}"
     )
-    assert not concept_name.startswith('"'), "Surrounding quotes were not stripped"
+    assert not concept_name.startswith('"'), "csv mode should strip surrounding quotes"
 
 
 

tests/test_load_vocab_source.py

@@ -11,6 +11,7 @@
     OPTIONAL_VOCAB_MODELS,
     REQUIRED_VOCAB_MODELS,
     MergeStrategy,
+    QuoteMode,
     _load_vocab_model_csv,
     load_vocab_source,
 )
@@ -93,7 +94,7 @@ def fake_load_vocab_model_csv(
     assert all(result_by_name[model.__tablename__].status == "loaded" for model in REQUIRED_VOCAB_MODELS)
     assert all(result_by_name[model.__tablename__].status == "skipped" for model in OPTIONAL_VOCAB_MODELS)
     assert all(merge_strategy == "replace" for _, merge_strategy, _, _ in loaded_tables)
-    assert all(quote_mode == "auto" for _, _, quote_mode, _ in loaded_tables)
+    assert all(quote_mode == "by_delimiter" for _, _, quote_mode, _ in loaded_tables)
     assert {table_name for table_name, _, _, _ in loaded_tables} == {
         model.__tablename__
         for model in REQUIRED_VOCAB_MODELS
@@ -176,6 +177,7 @@ def fake_load_vocab_source(
         db_schema: str | None = None,
         dry_run: bool = False,
         merge_strategy: MergeStrategy = "replace",
+        quote_mode: QuoteMode = "by_delimiter",
         chunksize: int | None = None,
         bulk_mode: bool = True,
         merge_batch_size: int = 1_000_000,
@@ -458,32 +460,23 @@ def fail_load_vocab_source(*args, **kwargs):
     assert "value too long for type character varying(255)" in result.stdout
 
 
-def test_load_vocab_source_uses_auto_not_literal_quote_mode(monkeypatch, tmp_path):
-    """Regression: Athena load must use auto quote mode so that quoted concept_name
-    values are not padded with surrounding double-quote characters, which would
-    cause 'value too long for type character varying(255)' on CONCEPT.csv."""
-    engine = sa.create_engine(f"sqlite:///{tmp_path / 'quote_mode_regression.db'}", future=True)
-
-    # Build a tab-delimited CSV where concept_name is exactly 255 chars when
-    # unquoted, but would be 257 chars if the surrounding CSV quotes were kept
-    # as literal characters (the literal-mode bug).
-    source_path = tmp_path / "athena_source"
-    source_path.mkdir()
-
-    long_name = "A" * 255
-    for model in REQUIRED_VOCAB_MODELS:
-        table_name = model.__tablename__.upper()
-        csv_path = source_path / f"{table_name}.csv"
-        if table_name == "CONCEPT":
-            csv_path.write_text(
-                "concept_id\tconcept_name\tdomain_id\tvocabulary_id\t"
-                "concept_class_id\tstandard_concept\tconcept_code\t"
-                "valid_start_date\tvalid_end_date\tinvalid_reason\n"
-                f'4715176\t"{long_name}"\t...\t...\t...\t\t...\t20000101\t20991231\t\n',
-                encoding="utf-8",
-            )
-        else:
-            csv_path.write_text("stub\n", encoding="utf-8")
+def test_load_vocab_source_defaults_to_by_delimiter_quote_mode(monkeypatch, tmp_path):
+    """The vocab load must default to `by_delimiter`, never `auto`/`csv`.
+
+    Real Athena vocabulary CSVs are tab-delimited with double-quotes as *literal*
+    data characters, not RFC-4180 field wrappers — e.g. CONCEPT_SYNONYM rows such
+    as '"Morning after" intrauterine contraceptive device fitted'. Interpreting
+    them as csv/RFC-4180 (which `auto` may pick, since its sample can miss the
+    discriminating rows) silently strips those quotes, which:
+      * corrupts concept/synonym names, and
+      * collapses two genuinely-distinct synonyms of the same concept into an
+        identical (concept_id, concept_synonym_name, language_concept_id) tuple,
+        violating Concept_Synonym's composite primary key on load.
+    `by_delimiter` resolves tab-delimited input to literal (quotes preserved) and
+    comma-delimited input to csv, deterministically and per file.
+    """
+    engine = sa.create_engine(f"sqlite:///{tmp_path / 'quote_mode_default.db'}", future=True)
+    source_path = _build_required_athena_source(tmp_path)
 
     received_quote_modes: list[str] = []
 
@@ -508,7 +501,8 @@ def fake_load_vocab_model_csv(
 
     load_vocab_source(engine, source_path=source_path)
 
-    assert all(mode == "auto" for mode in received_quote_modes), (
-        f"Expected all tables to use quote_mode='auto', got: {received_quote_modes}"
+    assert all(mode == "by_delimiter" for mode in received_quote_modes), (
+        f"Expected all tables to use quote_mode='by_delimiter', got: {received_quote_modes}"
     )
-    assert "literal" not in received_quote_modes
+    assert "auto" not in received_quote_modes
+    assert "csv" not in received_quote_modes