Skip to content

One file-to-database pipeline with a single conversion policy - #46

Merged
gkennos merged 21 commits into
mainfrom
44-null-handling-pipeline
Nov 5, 2026
Merged

gkennos merged 21 commits into
mainfrom
44-null-handling-pipeline

Conversation

@nicoloesch

@nicoloesch nicoloesch commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Every file load now takes the same path on every backend: file, reader, Arrow batches, conversion policy, backend sink.
  • What counts as missing, how each type is read and what happens to a bad value is decided in one place, the ConversionPolicy, and not in several loaders.
  • Text is kept verbatim, typed columns raise on unreadable values, and every row read is accounted for.
  • Pandas leaves the load path.

Closes

What changed

  • Pipeline
    • stage_file(ctx, reader, staging) runs reader, BatchProcessor and the backend's bulk sink
    • Files are streamed in bounded batches, so memory does not grow with row count
  • Conversion (orm_loader.conversion)
    • ConversionPolicy is passed to load_file(policy=...) and holds NullPolicy, ErrorPolicy and one handler per kind of column
    • NullPolicy sets what counts as missing: empty values, optional text markers, and whether quoting matters
    • ErrorPolicy sets raise (default) or reject_row, with an optional max_rejects. A count stops the load as soon as it is passed, and a fraction is checked against all rows read once the file has been read
    • Column handlers each own one kind of column: text, integer, float, numeric, boolean, date, datetime, plus passthrough
    • A new type is one ConvertingHandler subclass (clean, detect missing, parse, check) added through the policy
    • A handler is chosen from policy.columns, then the column type's class hierarchy. A type with no handler raises before any row is read, and passthrough is an explicit choice
    • Dates use declared formats and are validated against the calendar, and ambiguous forms are never guessed
    • Integers are checked against the width of the column type, and an offset is required in a time zone column and rejected in a naive one
    • Numeric columns are read by Arrow into a decimal of the declared precision and scale, from plain and exponent forms such as 1.5E3, and a value that would round or overflow is an error. Text is checked before it reaches Arrow's parser, which silently wraps on very long digit strings (apache/arrow#49817) and returns 0.00 for exponents such as 5e-50 in the versions tested (pyarrow 23.0.1 to 26.0.1)
    • Input that already has a type, such as Parquet, is cast and checked like text, so lossy casts are rejected. In a Numeric column an integer is widened before the cast, so an int64 column loads, and a float is read as its shortest decimal text, so a Parquet float column loads the same values as a CSV of the same numbers
    • A blank value, and a column the file lacks, take the column's default in the processor, read by the column's handler like file data. SQL expression defaults are evaluated by the database once, before the file is read
  • Readers (orm_loader.loaders.readers)
    • FileReader is the contract, with DelimitedTextReader and ParquetReader, and open_reader picks one from the file suffix
    • load_file(reader=...) accepts a reader, and quote_mode and chunksize configure the default one. Passing either together with a reader raises ValueError
    • Quote handling, delimiter and encoding detection live with the delimited reader, and quote_mode defaults to by_delimiter
    • Under reject_row, lines with the wrong number of fields are written to the rejects file as well, with their row number in the file
  • Processor and accounting
    • BatchProcessor applies the handlers, checks required columns and writes rejected rows with their raw text and reason
    • LoadReport accounts for every row read as loaded, rejected or deduplicated, and a load that cannot account for every row raises
  • Sinks
    • PostgreSQL: one COPY written from Arrow
    • DuckDB: insert from a registered Arrow view, committed every five batches to bound memory
    • SQLite: native executemany
    • A sink failure raises, with no fallback, and stage_file rolls the session back after its first write (DuckDB commits every few batches, so earlier batches stay committed)
    • One staging table definition for all backends: the target's columns and nullability, no defaults or constraints, and a backend-specific row key. Each backend only defines its row key column, and create and drop both use that definition
  • One backend per load
    • load_file resolves the backend once and holds it on LoaderContext.backend
    • The staging table is created once and passed to the sink and the merges, so it is no longer reflected again
  • Merge strategies
    • replace deletes the matching keys and inserts, upsert skips keys that already exist and the first of a repeated key wins, and insert is a plain insert whose key collision raises the database's IntegrityError
    • Each runs as named. Upsert into an empty table only logs a hint that insert with dedupe=True is faster
    • Every primary key column must be in the file
    • A failed index rebuild after a committed merge raises IngestError
  • Deduplication now runs in the database after staging and reports a DedupeResult
  • Referential integrity
    • Postgres and SQLite relax FK checks for the whole load, from before staging is written until the merge is committed, so orphan references were accepted unnoticed
    • SQLite ignores the pragma inside a transaction, so load_file reads the state back and raises when the session already has uncommitted writes
    • Table.validate_foreign_keys(session) checks every FK with an anti-join, in any load order, and raises ForeignKeyViolationError
    • On DuckDB it checks the keys DuckDB cannot enforce
  • Tests and docs
    • New layers for readers, processor, sink contract and a dialect-by-format pipeline matrix
    • The Loaders docs are reorganised into Overview, Readers, Conversion, Staging and Sinks, and Context and Report

Breaking changes

Compared with main.

  • Renames
    • CSVLoadableTableInterface is now FileLoadableTableInterface
    • CSVTableProtocol is now FileTableProtocol
    • load_csv is now load_file
    • _select_loader is removed, and load_file(reader=...) replaces it
    • csv_columns is removed, and readers map file headers to model_columns()
  • Removed from the loaders
    • LoaderInterface, PandasLoader, ParquetLoader and the loader= argument of load_file
    • normalise_null
    • Column cast rules and their statistics: CastRule, register_column_cast_rule, TableCastingStats and ColumnCastingStats
    • infer_delim, infer_encoding and quick_load_pg from orm_loader.loaders. The detection functions are in orm_loader.loaders.readers.detection
    • orm_staging_load and orm_file_load
    • The auto value of quote_mode
    • LoaderContext.staging_table, LoaderContext.staging_schema_tag, LoaderContext.quote_mode and LoaderContext.chunksize. The staging table is passed to stage_file and the sinks, the context holds the resolved backend, and the reader options belong to load_file and the reader
  • Removed from the tables
    • get_staging_table, load_staging, create_staging_table and drop_staging_table: use the backend's create_staging_table and drop_staging_table
    • ORMTableBase.pk_values and pk_tuple
    • The dict helpers clean_kwargs and from_dict
  • Removed from the backends and helpers
    • helpers/bulk.py and helpers/sqlite.py: use resolve_backend(session).relaxed_foreign_keys(session), and load_file already handles FKs for file loads
    • bulk_load_context, merge_context, enable_fk_check and load_staging_fast
    • BackendCapabilities, capabilities, supports_dialect and requires_capability. supports_replace_with_kept_indexes is a class attribute of the backend
    • The SQLite install_engine_hooks, configure_dbapi_connection, explain_fk_error and restore_journal_mode, and the Postgres engine_with_replica_role
    • The materialized view methods of DatabaseBackend: they exist only on backends that inherit MaterializedViewBackend, which is PostgreSQL, and MaterializedViewMixin raises NotImplementedError on the others
  • Changed signatures
    • merge_replace, merge_upsert and merge_insert take a staging parameter, and create_staging_table returns the table it created
    • manage_indices and merge_from_staging take the backend instead of resolving it
  • Behaviour
    • The merge strategy insert replaces insert_if_empty. It does not check that the target is empty, and a key collision raises the database's IntegrityError
    • replace and upsert no longer turn into a plain insert when the target is empty. Upsert into an empty target only logs a hint, and a repeated key keeps the first row
    • Every primary key column must be in the file. A column that resolves to autoincrement is no longer optional, and a load never lets the database assign a key
    • A failed index rebuild after a committed merge raises IngestError instead of only being logged
    • quote_mode defaults to by_delimiter, which reads quotes in tab-delimited files as data
    • On SQLite, load_file raises when the session has uncommitted writes, because the foreign key pragma cannot change inside a transaction
    • Text is stripped and otherwise kept verbatim, with no numeric canonicalisation
    • Words such as NULL, None, N/A and NaN stay as text in text columns, and only an empty field is NULL. To treat words as missing again, list them in NullPolicy(text_markers=...)
    • Typed columns raise on unreadable values
    • Missing words in typed columns are null, none, na, n/a, nan, #n/a, -nan and <na>, case-insensitive
    • Integers outside the width of the column type, Infinity, and year 0 are errors
    • A column type with no handler raises, and an Enum column accepts only its labels
    • A blank value in a column that has a default takes the default instead of NULL
    • A Numeric column must declare a precision of at most 38 digits. A wider or unconstrained Numeric raises a PolicyError before any row is read, and columns={(table, column): PassthroughHandler()} leaves such a column to the database as main did
    • A Numeric value that does not fit the declared scale or precision is an error. main passed the text to the database, which rounded it (1.555 into scale 2 became 1.56) or aborted the whole load
  • Dependencies
    • Pandas moves from a dependency to the dev group

Deferred

  • Converting Numeric columns of more than 38 digits, or with no declared precision, in the loader
    • The loader converts only what an Arrow decimal represents exactly, which is at most 38 digits. That is also the widest decimal DuckDB stores, and SQLite stores Numeric as a float
    • Such a column raises a PolicyError and does not get a separate text path with its own validation. It loads today with a PassthroughHandler, which leaves parsing and rounding to the database
    • A dedicated path can be added later if a use case appears. No other repository in the stack declares such a column

Benchmark

  • What was measured
    • Only the file to staging step. The merge into the target table is the same in both versions and is excluded
    • Before is the loader on main (3998ff9), after is this branch
    • Three OMOP vocabulary files from the vocab_test_duckdb set, tab-delimited: CONCEPT (2.3M rows), CONCEPT_SYNONYM (2.5M rows) and DRUG_STRENGTH (210k rows)
    • PostgreSQL, DuckDB and SQLite, with the quote mode shown in the table. The old loader's default is auto, the new loader's default is by_delimiter, which is also what omop-alchemy uses
    • On PostgreSQL the old loader tries COPY first and falls back to pandas when it fails. The path that ran is recorded, and the pandas path is also run on its own
    • One run per cell in its own process with a 6 GB memory cap above baseline, so expect about 10 percent noise
    • After each load the counts in staging (rows, marker-named values, leading-zero codes, quoted text) are compared with the same counts taken from the file
    • The benchmark script is kept local and is not part of this PR
Backend Quote mode Path before Before (s) After (s) Speedup Peak memory before / after (MB) Matches the file before / after
CONCEPT (2,286,910 rows)
SQLite by_delimiter pandas 44.8 8.1 5.5x 4,030 / 591 no / yes
DuckDB by_delimiter pandas 40.5 3.4 12.0x 4,457 / 1,052 no / yes
PostgreSQL by_delimiter COPY 2.0 2.2 0.9x 492 / 608 yes / yes
PostgreSQL by_delimiter pandas 50.5 2.2 22.5x 4,238 / 608 no / yes
PostgreSQL old auto (default), new by_delimiter (default) COPY failed, then pandas 51.1 2.2 23.2x 4,291 / 572 no / yes
CONCEPT_SYNONYM (2,549,771 rows)
SQLite by_delimiter pandas 14.3 3.1 4.6x 2,696 / 542 no / yes
DuckDB by_delimiter pandas 14.9 1.6 9.2x 3,313 / 931 no / yes
PostgreSQL by_delimiter COPY 1.1 1.1 1.0x 492 / 577 yes / yes
PostgreSQL by_delimiter pandas 27.9 1.1 24.9x 3,121 / 577 no / yes
PostgreSQL old auto (default), new by_delimiter (default) COPY failed, then pandas 29.6 1.2 24.7x 3,118 / 528 no / yes
DRUG_STRENGTH (209,849 rows)
SQLite by_delimiter pandas 3.6 0.8 4.6x 711 / 422 yes / yes
DuckDB by_delimiter pandas 3.3 0.2 14.3x 789 / 422 yes / yes
PostgreSQL by_delimiter COPY 0.1 0.3 0.3x 492 / 422 yes / yes
PostgreSQL by_delimiter pandas 4.5 0.3 13.7x 796 / 422 yes / yes
PostgreSQL old auto (default), new by_delimiter (default) COPY 0.1 0.3 0.4x 609 / 509 yes / yes
  • Reading the results
    • On SQLite and DuckDB the new loader is 4.5 to 14 times faster and uses a quarter to a half of the memory
    • On PostgreSQL with by_delimiter the old COPY path and the new loader take about the same time, and the old path is faster on the smallest file, where fixed costs dominate. The new loader parses, validates and re-serialises the data instead of copying the file
    • Faster did not mean correct. The old COPY path and its pandas fallback gave different data for the same file
      • Missing values
        • COPY keeps NULL, None, N/A and NaN as text and keeps a quoted "" as an empty string
        • The pandas path turns all of them into NULL
      • Dates and numbers
        • COPY lets the Postgres session parse dates, so 02/01/2020 is 1 Feb or 2 Jan depending on DateStyle, while the pandas path always reads 2 Jan
        • Postgres stores NaN in numeric columns and the pandas path makes it NULL
        • The pandas path rewrote text that looks like a number, so 007 became 7 and 1.50 became 1.5
        • The pandas path turned unparseable dates and booleans such as on and off into NULL without an error
      • Required columns
        • An empty value in a required column fails COPY, and the pandas fallback then drops that row with no error
    • The fallback is not rare. The old loader could not read these valid Athena files correctly in its default mode, and COPY failed on CONCEPT and CONCEPT_SYNONYM, two of the three files here
      • The old default auto samples the first 2000 rows and chooses CSV quoting for these tab-delimited files, because the first field that starts with a quote ("Sick" headaches) is on line 103,811. In CSV mode it opens a quoted field that swallows the following fields
      • The merged value is too long for the column, COPY fails, and the loader logs a warning and switches to pandas
      • The pandas run is 23 to 27 times slower than the new loader, takes about 4 GB of memory and brings the pandas problems above
    • The old pandas path (SQLite, DuckDB, and PostgreSQL after a fallback) dropped 46 CONCEPT rows and 1 CONCEPT_SYNONYM row named None, N/A or Null, and rewrote 2,319 concept_code values
    • With by_delimiter the new loader loads every row unchanged
    • The new loader has no auto mode. Its default by_delimiter reads quotes in tab-delimited files as data, and csv is an explicit choice for files that quote fields
    • At a 4 GB memory cap the old loader ran out of memory on CONCEPT on all three backends

@nicoloesch nicoloesch added the breaking Incompatible API change. MAJOR: x+1.y.z label Oct 29, 2026
…ing passed as an argument, defaults filled by the processor
…atabase-assigned keys, rename insert_if_empty to insert, move reader options off LoaderContext
…pragma took effect, remove unused FK and connection hooks
…s; evaluate expression defaults once; move materialized views to their own backend interface
Comment thread src/orm_loader/loaders/processor.py Resolved
Comment thread src/orm_loader/loaders/processor.py Resolved
Comment thread src/orm_loader/loaders/processor.py Resolved
Comment thread src/orm_loader/tables/loadable_table.py Resolved
Comment thread src/orm_loader/tables/orm_table.py Resolved
Comment thread src/orm_loader/tables/typing.py Resolved
Comment thread src/orm_loader/tables/orm_table.py Resolved
Comment thread src/orm_loader/loaders/loader.py Resolved

@gkennos gkennos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes needed before merge:

  1. Text null words: keep the new default (it fixes #44), but document the change under Breaking changes and ship the 2.x words as a preset that can be selected or adapted. Details inline on conversion/policy.py.
  2. Re-export infer_delim from orm_loader.loaders; downstream code still imports it from the old path.
  3. Inline, blocking: per-row SQL defaults (loaders/processor.py) and primary-key presence (loaders/loader.py).

Comment thread src/orm_loader/conversion/policy.py Resolved
gkennos and others added 6 commits November 3, 2026 11:49
…and column-name handling, report rejects per row

- SQL-expression defaults are applied per row by COALESCE in the merge;
  their staging columns are nullable and evaluate_default is removed
- Numeric without precision, or above 76 digits, is read as text; 39-76
  digits use decimal256; invalid scale is rejected
- Legacy null markers cover the pandas/pyarrow defaults and case variants
- Reject counts and reasons are reported per row; the row limit is
  checked after processing
- Primary keys and model columns are keyed by database column name;
  computed columns are skipped; primary keys stay required without
  normalisation
- infer_delim is exported; staging and conversion docs updated
…he role into the pool

relaxed_foreign_keys sets session_replication_role with SET LOCAL and
reapplies it whenever a transaction begins on the selected bind, so a
commit that switches pooled connections keeps keys relaxed and no
connection is returned to the pool in replica. The role found in an
open transaction is restored on exit. A scoped_session relaxes only
the current thread's session. Docs say to flush ORM inserts inside the
block.
…ct limits at the end, keep computed columns out of loads

- model_columns() excludes computed columns, so the reader, processor and required_columns agree
- a primary key is always required in the file and never takes a default
- SQL-expression defaults are applied by COALESCE in the merge and are not written by the processor
- a count limit stops a load at once, a fraction is checked against all rows read, and a load that rejected rows logs a warning
- Numeric is cast up to 38 digits and kept as validated text beyond that or without a precision
- handlers validate the declared column type through validate(), which PassthroughHandler no longer trips over
- the review regression tests move into topical test files
… marker_quoting and LEGACY_TEXT_MARKERS are removed

- readers take no NullPolicy and hand over text as read, so empty_is_null=False keeps empty strings in every format
- text markers match after trimming for every format, and a reader can no longer disagree with the policy
- infer_delim and infer_encoding are no longer exported from orm_loader.loaders
- docs describe how to restore the 2.x words with text_markers and the raw pass-through of normalise=False
…longer uses

- relaxed_foreign_keys sets the replication role per transaction, so disable_fk_check and restore_fk_check are gone from PostgresBackend
- the role check accepts only the role names Postgres reports
@nicoloesch

Copy link
Copy Markdown
Collaborator Author

On the infer_delim re-export: infer_delim and infer_encoding are importable from orm_loader.loaders.readers.detection, and the new path is listed under Breaking changes. Which downstream code imports the old path?

Differs from the suggestion: there is no re-export from orm_loader.loaders. This is a breaking release and consumers adapt anyway. If a real consumer needs it, the export is two lines.

@gkennos

gkennos commented Nov 5, 2026

Copy link
Copy Markdown
Member

On the infer_delim re-export: infer_delim and infer_encoding are importable from orm_loader.loaders.readers.detection, and the new path is listed under Breaking changes. Which downstream code imports the old path?

Differs from the suggestion: there is no re-export from orm_loader.loaders. This is a breaking release and consumers adapt anyway. If a real consumer needs it, the export is two lines.

cava-pipelines uses this but I checked and they have a dependency ceiling version set so no issue to migrate - this is fine

@gkennos
gkennos self-requested a review November 5, 2026 04:40
@gkennos
gkennos merged commit 6961339 into main Nov 5, 2026
4 checks passed
@gkennos
gkennos deleted the 44-null-handling-pipeline branch November 5, 2026 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Incompatible API change. MAJOR: x+1.y.z

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ingestion coerces valid strings that match null sentinels (e.g. "Null", "None", "N/A") to NULL

2 participants