Skip to content

header handling - #35

Merged
gkennos merged 3 commits into
mainfrom
autoincrement
Aug 17, 2026
Merged

header handling#35
gkennos merged 3 commits into
mainfrom
autoincrement

Conversation

@gkennos

@gkennos gkennos commented Aug 17, 2026

Copy link
Copy Markdown
Member

Summary

Resolves: #33 and #34

  • For autoincrement integer-type PKs: these should not be required in source data files
  • If header is pre-quoted, we need to strip these so that we don't end up with doubly quoted strings that are rejected

Checklist

  • Applied exactly one label (breaking, feature, fix, dependencies, or chore)
  • Tests pass locally (uv run pytest -q)
  • Lint passes (uv run ruff check .)

@gkennos gkennos added the fix Bug fix, backwards-compatible. PATCH: x.y.z+1 label Aug 17, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses two ingestion issues: (1) autoincrement integer primary keys should not be treated as required in inbound files, and (2) CSV headers that are literally double-quoted should be normalized to avoid generating invalid PostgreSQL COPY SQL.

Changes:

  • Add autoincrement-aware logic to ORMTableBase.required_columns() and expose required_columns() on the ORM table typing protocol.
  • Strip literal double-quotes from CSV header tokens when constructing COPY (...columns...) and add a regression test for quoted headers.
  • Update pandas/pyarrow casting paths to use required_columns() for “required field” handling.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/loaders/test_pg_loader.py Adds regression coverage for literally-quoted CSV headers in quick_load_pg.
src/orm_loader/tables/typing.py Extends ORMTableProtocol with required_columns() to align typing with the new API.
src/orm_loader/tables/orm_table.py Implements autoincrement-aware required column detection via _resolves_to_autoincrement().
src/orm_loader/loaders/loading_helpers.py Normalizes quoted CSV header tokens for COPY column lists (and header rewriting stream).
src/orm_loader/loaders/loader_interface.py Switches required-column logic in pandas/arrow casting to use ctx.tableclass.required_columns().
Suppressed comments (1)

src/orm_loader/loaders/loader_interface.py:197

  • Same issue as the pandas path: filtering required_cols to only schema-present columns skips validation when required columns are missing from the Parquet/Arrow input, deferring failures to the DB layer. Detect missing required columns up front and raise a clear error.
        required_cols = [c for c in ctx.tableclass.required_columns() if c in out.schema.names]

        if required_cols:
            masks = [pc.is_valid(out[c]) for c in required_cols]            # type: ignore

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/orm_loader/loaders/loading_helpers.py Outdated
Comment thread src/orm_loader/loaders/loader_interface.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/orm_loader/loaders/loader_interface.py:61

  • _normalise_columns is being decorated with @staticmethod at module scope. That turns the function into a staticmethod object, so calls like chunk = _normalise_columns(chunk) will fail at runtime with TypeError: 'staticmethod' object is not callable. Remove the decorator (or, if it was meant to be a class method, move it inside the class and call it via cls).
@staticmethod
def _normalise_columns(df: pd.DataFrame) -> pd.DataFrame:
    df = df.copy()
    df.columns = [c.lower().replace('_hash', '').strip() for c in df.columns]
    return df

@gkennos
gkennos merged commit 6d37c9e into main Aug 17, 2026
3 checks passed
@gkennos
gkennos deleted the autoincrement branch August 17, 2026 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Bug fix, backwards-compatible. PATCH: x.y.z+1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

required_columns() should exclude autoincrement PK columns

2 participants