Skip to content

fix: align child column type on the referenced key in PostgreSQL output - #36

Merged
TheCrab13 merged 1 commit into
mainfrom
fix/pg-foreign-key-type-alignment
Sep 21, 2026
Merged

TheCrab13 merged 1 commit into
mainfrom
fix/pg-foreign-key-type-alignment

Conversation

@TheCrab13

Copy link
Copy Markdown
Contributor

Summary

PostgresSqlEmitter mapped every column in isolation, so the two sides of a
foreign key could land on PostgreSQL types the server cannot compare. The DDL
and the data were valid, but the final ALTER TABLE ... ADD FOREIGN KEY
failed:

ERROR:  foreign key constraint "dossier_client_id_fkey" cannot be implemented
DETAIL:  Key columns "client_id" and "id" are of incompatible types: character varying and uuid.

Since a dump is wrapped in a single transaction, that one error rolled the
whole import back and the user got no data at all. Source schemas declare the
two sides with different types more often than one would expect —
uniqueidentifier / varchar(36), int / numeric(18,0) and bit /
tinyint were all seen in the field.

A child column carrying a foreign key now takes the type of the key it
references, but only when PostgreSQL refuses the pair. Every other column keeps
its declared type, so the output is unchanged for schemas that were already
importable.

The compatibility rule is encoded in three module tables, probed pair by pair
on a live PostgreSQL 15 server. Three of its properties are not obvious:

  • the declared size plays no part — varchar(50) may reference varchar(10);
  • the numeric rule is one way — an integer column may reference a numeric
    key, but a numeric column may not reference an integer key;
  • char / varchar / text accept each other, and so do date / timestamp
    / timestamptz.

The defect was reported against 1.0.1 with a patch against that version. The
patch could not be applied as it stood: it read column.foreign_key, which no
longer exists since foreign keys became table-level, possibly composite
constraints. The resolution step was rewritten around table.foreign_keys,
zipping columns and ref_columns to find what a given local column points
at. Composite keys are therefore handled — each half is aligned on its own
counterpart, which matches the pairwise check PostgreSQL performs.

Both the dump mode and the migrate mode go through emit_tables, so both get
the fix.

Type of change

  • Bug fix (fix:) — non-breaking change that fixes an issue
  • Feature (feat:) — non-breaking change that adds capability
  • Performance (perf:)
  • Refactor (refactor:) — no behaviour change
  • Documentation (docs:)
  • CI / build / tooling (ci: / chore:)
  • Breaking change — describe the migration path below

column_definition gains a second parameter with a default value, so the
signature stays source-compatible; emit_tables is its only caller that passes
it.

Linked issues

How was this tested?

Fourteen new test cases (29 parametrized runs) in
tests/unit/infrastructure/emit/test_postgres_emitter.py, covering:

  • the five pairs PostgreSQL refuses and five it accepts;
  • the canonical PostgreSQL spellings a PG source reports (character varying,
    int4, bpchar, timestamp without time zone), which must not trigger a
    rewrite;
  • a size difference on its own, which must change nothing;
  • an identity key, referenced as integer / bigint rather than
    serial / bigserial, and an identity child, which is left alone;
  • nullability and defaults preserved through the rewrite;
  • a reference that resolves to nothing — missing schema, table or column, as a
    filter may leave behind;
  • a chain of three tables, resolved down to the root key;
  • a self-referencing table and a two-table cycle, which must terminate;
  • a composite key where only one half needs alignment.

The reproduction from the report was replayed end to end on the SQLite driver:
dump.sql now declares "client_id" uuid instead of "client_id" varchar(36).

  • pytest tests/unit tests/cli passes locally — 627 passed
  • lint-imports passes (no driver leaked into domain/application) — 2 contracts kept
  • tox -e syntax equivalent passes — black, isort, flake8, mypy clean, pylint 10.00/10
  • For changes touching readers/writers/emitters: tests/functional ran against the docker stack

The functional suite was not run: no Docker and no PostgreSQL server were
available in the environment used to prepare this change. The live import check
therefore rests on the compatibility table supplied with the report, probed on
PostgreSQL 15.19, rather than on an execution here. Running
tests/functional against the stack before merge is advisable.

Checklist

  • Commit messages follow the Conventional Commits / Angular preset
  • Public API changes are documented in docs/ and CHANGELOG.md — no
    public API change; CHANGELOG.md is generated by semantic-release
  • Coverage is maintained at or above the current threshold (80%) — 92%
    overall, 99% on the changed file
  • No credentials, hostnames, or other sensitive data leaked in tests / fixtures
  • If this is a breaking change, the README / docs migration notes are updated — n/a

Notes for the reviewer

  • Where the alignment is decided. _aligned_type returns None whenever
    the declared type is already acceptable, which is the case for every column
    without a foreign key. The emitted output is byte-for-byte identical for
    schemas that did not hit the defect.

  • Type aliases matter. A PostgreSQL source reports character varying, and
    the type map leaves that spelling alone because it is already valid
    PostgreSQL. Without _TYPE_ALIASES folding it to varchar, the emitter would
    see two different types and rewrite columns that need no change. That is what
    keeps the PG-to-PG path inert.

  • Identity keys. A key column reads serial or bigserial in the DDL, but
    a column referencing it must be declared integer or bigint. _key_type
    returns the storage type for that reason, and _aligned_type refuses to touch
    an identity child.

  • Cycles. _key_type follows a key that is itself a foreign key down to the
    root. The seen set stops a self-reference or a cycle between two tables;
    both are covered by tests.

  • A column named by two constraints keeps the first one. Aligning on both is
    not possible in the general case, and the situation does not arise in any
    schema we have seen.

  • Side effect worth knowing. The DEFAULT clause is now translated against
    the aligned type, so a source 0 on a column aligned to boolean emits
    FALSE instead of an integer PostgreSQL would reject. A test locks this in.

  • Deliberately left out. The report named two neighbouring defects that this
    PR does not address:

    1. the emitter never writes a UNIQUE constraint, so a foreign key pointing
      at a unique non-primary-key column always fails with there is no unique constraint matching given keys for referenced table;
    2. emit_foreign_keys validates the referenced schema and table but not the
      referenced column, so a filter that removes a column leaves an
      ALTER TABLE naming a column that does not exist.

    Both are reproducible and deserve their own PRs.

  • The MSSQL emitter has the same shape of defect and was not touched here.

@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #36      +/-   ##
==========================================
+ Coverage   92.09%   92.24%   +0.15%     
==========================================
  Files          79       79              
  Lines        3161     3225      +64     
==========================================
+ Hits         2911     2975      +64     
  Misses        250      250              
Files with missing lines Coverage Δ
db2sql/infrastructure/emit/postgres/emitter.py 99.23% <100.00%> (+0.24%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@TheCrab13
TheCrab13 merged commit 47cda4e into main Sep 21, 2026
7 checks passed
@TheCrab13
TheCrab13 deleted the fix/pg-foreign-key-type-alignment branch September 21, 2026 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants