Skip to content

Commit 80d28a7

Browse files
PingoLeeclaude
andcommitted
fix(#414, #415): PostgreSQL introspection reads spaced identifiers and con.confkey
Two defects in the PostgreSQL schema reader, both found during #389 and deferred there with in-place comments. Landed together because they share a file, a test surface and a verification cost. #414 — the `columns` aggregate is `quote_ident(name) || ' ' || format_type(...)` and the reader split it on `" "`, so an identifier CONTAINING a space was torn in half before anything else saw it: `"Parent Id" bigint` yielded the phantom name `"Parent` (the leading quote survives — `_unquote_ident` correctly refuses to strip a lone unbalanced one), the non-type `Id"` so the column degraded to TextField, and a LOST relation, because `fk_map`/`pk_set`/`index_map` all key on names that survive a space. makemigrations then proposed adding the phantom and dropping the real column on every run, forever. SQLite was never affected. Fixed in the PARSER, not the aggregate: `quote_ident` output is self-delimiting, so scanning to the closing quote recovers every name that reaches the function whole. The flag/type scans move onto the post-name remainder in the same change, which closes the sibling class where a column NAMED like a marker marked itself. #415 — the `foreign_keys` CTE derived an FK's referenced column from the parent's PRIMARY KEY INDEX; `con.confkey` was not selected anywhere in the file. An FK pointing at a non-PK UNIQUE column reported the wrong column, and a composite parent key fanned every aggregate out once per parent key column, so the last entry silently won the `fk_map` assignment and the delete_rules alignment #292 established broke with it. Now correlated through `unnest(con.conkey, con.confkey)` — bound-agnostic, so unlike a `conkey[1]` subscript it carries none of #347's lower-bound hazard. Adds no version floor: the `indexes` CTE in the same statement already requires PG 11+. Three behavior changes fall out, all deliberate and documented: * a genuine MULTI-column FK is skipped rather than guessed at, on BOTH engines (PG by `array_length(con.conkey, 1) = 1`, SQLite by grouping PRAGMA foreign_key_list on `id`, which used to split one constraint into N relations). PormG has no composite-FK field type; reading one approximately would regenerate a different schema. * an FK referencing a non-PK UNIQUE column reads that column. * an FK whose parent has NO primary key is visible at all — the dropped `pg_index` join was INNER, so such a key vanished from the PG read entirely while SQLite reported it. Also restores `db_column = "driver ref"` on the Odd_identifier_scratch fixture, which #394 had to leave out precisely because of #414 — the global no-drift assertion in test_db_table_db.jl is now the live gate for this parse. And test_importers_introspection.jl gains the standalone guard it uniquely lacked, so it no longer forces the full runtests.jl prologue to see one assertion. Verification: unit 8874/8874; full SQLite integration suite 2172 pass / 1 pre-existing broken / 0 fail; db_sl introspection slice 78/78; docs build clean. Mutation-checked — reverting the parser fails 13 assertions, removing the SQLite skip fails 4. NOT verified: the PostgreSQL behavior of #415. A read-only probe confirms the new CTE parses and executes against db_2 and that none of its 28 existing FK tables regress, but measured side by side the old and new CTE agree on all 28 — db_2 carries neither affected shape today, so that probe is a no-regression check, not a gate. Sections 3f/3g/3h and the spaced-db_column no-drift assertion need a full db_2 run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 26b9a70 commit 80d28a7

9 files changed

Lines changed: 857 additions & 51 deletions

File tree

UPGRADING.md

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,122 @@ _Changes merged but not yet cut into a release. A consumer dev'ing PormG at HEAD
4545
and `PormG.upgrade_guide` surfaces them by default. When the maintainer next rolls changes into a
4646
consuming app, `/pormg-cut-release` stamps every entry below with `0.5.0`, dates them, and tags it._
4747

48+
## Schema introspection reads two column shapes differently — spaced identifiers, and multi-column foreign keys (#414, #415)
49+
50+
- **Version**: Unreleased
51+
- **PormG ref**: #414, #415; `src/migrations/introspection.jl`, `docs/src/schema_conventions.md`
52+
- **Recorded**: 2026-08-26
53+
- **Severity**: **behavior change (narrow — only apps whose live schema has one of these two shapes)**.
54+
Nothing changes for a schema without a spaced identifier or a composite foreign key. Where one
55+
exists there are **two** ways it reaches you, and the second needs no action on your part to fire:
56+
a **regenerated** model file names different fields than before, **and** `makemigrations` — which
57+
runs introspection on every invocation — can now start proposing a change on every run against a
58+
model file you have not touched. Part of the `0.5.x` pre-publish wave.
59+
60+
### What changed
61+
62+
Both are corrections to what PormG **reads back** from a live database. Neither changes anything you
63+
declare, and neither alters your schema.
64+
65+
**1. A column name containing a space is no longer torn in half (#414).** The PostgreSQL reader split
66+
its `columns` aggregate on the space between the name and the type, so an identifier that *contained*
67+
a space was destroyed before anything else saw it. A column named `driver ref` came back as the
68+
phantom `"driver` — leading quote and all — and `Parent Id` came back as `"Parent`, typed
69+
`TextField`, **with its foreign key dropped**, because the FK lookup keyed on the real name and no
70+
longer matched. `makemigrations` then proposed adding the phantom and dropping the real column on
71+
every run, forever. SQLite was never affected, so one schema introspected correctly on one engine and
72+
corruptly on the other.
73+
74+
**2. A multi-column foreign key is now skipped rather than guessed at (#415).** PormG has no
75+
composite-`ForeignKey` field type. PostgreSQL used to derive the referenced column from the parent's
76+
*primary key index* instead of the constraint's own `confkey`, binding a composite FK to an arbitrary
77+
column; SQLite split it into one independent single-column relation per member. Both now import the
78+
member columns as **ordinary fields with no relation** and leave the constraint alone in the
79+
database — the same reject-rather-than-reinterpret rule that keeps a non-default index out of an
80+
imported model. Re-emitting either previous reading produced a *different* schema than the live one.
81+
82+
Two more single-column cases now read **correctly** where they used to read wrong — but "correct"
83+
is still a *change*, so your declared model is the stale side and both need action, in the opposite
84+
direction from change 2:
85+
86+
- **An FK referencing a non-primary-key `UNIQUE` column** (`REFERENCES driver_registry(licence_no)`)
87+
now reports that column. The old importer wrote its wrong answer explicitly into your model file —
88+
`Model_to_str` emits `pk_field=` on every introspected FK — so a model still declaring
89+
`pk_field="id"` disagrees with a live read of `licence_no`, and the planner proposes `:pk_field` on
90+
every run. Update the declaration to the column the constraint really names.
91+
- **An FK whose parent table has no primary key at all** was previously *invisible* to PostgreSQL
92+
introspection (the referenced column only needs a `UNIQUE` constraint, but the old query reached it
93+
through the parent's primary-key index with an inner join). Your model therefore has a plain column
94+
where the live read now reports a relation. Declare the `ForeignKey`.
95+
96+
### How to find the calls to migrate
97+
98+
**Do not assume you are safe because you have not re-run the importer.** `makemigrations` calls the
99+
same introspection (`convert_schema_to_models`, `src/migrations/planner.jl`), so change 2 can bite a
100+
model file you never regenerate: if your model still declares `ForeignKey` on a composite-FK member
101+
column, the live read now says "plain integer", the declared side still says "relation",
102+
`describes_same_column` refuses to equate them, and the planner proposes a change **on every
103+
`makemigrations`** — on SQLite, a full table rebuild each time.
104+
105+
```bash
106+
# 1. Which of the FOUR shapes does your schema actually have? (PostgreSQL)
107+
# a. multi-column foreign keys — change 2
108+
psql -c "SELECT conrelid::regclass, conname FROM pg_constraint
109+
WHERE contype = 'f' AND array_length(conkey, 1) > 1;"
110+
# b. identifiers containing a space — change 1
111+
psql -c "SELECT table_name, column_name FROM information_schema.columns
112+
WHERE column_name LIKE '% %';"
113+
# c. foreign keys NOT pointing at the parent's primary key
114+
psql -c "SELECT c.conrelid::regclass, c.conname FROM pg_constraint c
115+
JOIN pg_index i ON i.indrelid = c.confrelid AND i.indisprimary
116+
WHERE c.contype = 'f' AND c.confkey::int[] <> i.indkey::int[];"
117+
# d. foreign keys into a parent with no primary key at all
118+
psql -c "SELECT c.conrelid::regclass, c.conname FROM pg_constraint c
119+
WHERE c.contype = 'f' AND NOT EXISTS (
120+
SELECT 1 FROM pg_index i WHERE i.indrelid = c.confrelid AND i.indisprimary);"
121+
122+
# 2. For anything those returned, grep your models for the affected columns.
123+
grep -n "ForeignKey" <your db_def_folder>/*.jl
124+
125+
# 3. Then run makemigrations and read the plan BEFORE applying it. A proposal against a table you
126+
# did not change is this entry — and step 3 catches all four shapes even if you skipped step 1.
127+
```
128+
129+
### Migrate your app
130+
131+
```julia
132+
# ── change 2: a composite-FK member column must stop declaring a relation ──
133+
# ✗ before — ONE constraint, `FOREIGN KEY (raceid, driverid) REFERENCES result(raceid, driverid)`,
134+
# imported as two single-column relations. Note both name the SAME parent: a composite FK has one
135+
# referenced table, so this is the shape to look for. Two relations pointing at DIFFERENT parents
136+
# are two ordinary foreign keys, which this change does not touch — do not delete those.
137+
# (The `pk_field` values below are what the old SQLite reader wrote. The old PostgreSQL reader
138+
# named the parent's primary-key column on both members instead; either way, both lines go.)
139+
Lap_time = Models.Model("lap_time",
140+
raceid = Models.ForeignKey("result", pk_field="raceid"),
141+
driverid = Models.ForeignKey("result", pk_field="driverid"),
142+
)
143+
144+
# ✓ after — plain columns; the constraint stays in the database, PormG just stops modelling it.
145+
# REGENERATE if you can: hand-declaring means matching the column exactly, and two attributes
146+
# drift if you get them wrong. Type — introspection maps `bigint` to BigIntegerField and `integer`
147+
# to IntegerField (src/constants.jl), so IntegerField() over a BIGINT column drifts on `:type`
148+
# forever. And nullability — `:null` has no exemption in `_NON_SCHEMA_FIELD_ATTRS`, so a bare
149+
# field declared over a NULLable column drifts on `:null` the same way.
150+
Lap_time = Models.Model("lap_time",
151+
raceid = Models.BigIntegerField(),
152+
driverid = Models.BigIntegerField(),
153+
)
154+
155+
# ── change 1: a spaced column's regenerated FIELD NAME is sanitized, and gains a db_column ──
156+
# ✗ before — the phantom name, truncated at the space
157+
M.Driver.objects.filter("driver" => "senna")
158+
159+
# ✓ after — `Model_to_str` emits `driver_ref = CharField(db_column="driver ref", …)`,
160+
# so the field is `driver_ref` and the physical column keeps its space
161+
M.Driver.objects.filter("driver_ref" => "senna")
162+
```
163+
48164
## `values()` refuses two projections that would render the same output name (#441)
49165

50166
- **Version**: Unreleased

docs/src/schema_conventions.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,33 @@ category = Models.ForeignKey("constructor", on_delete=CASCADE)
488488
| `DO_NOTHING` | `NO ACTION` |
489489
| `PROTECT` | `RESTRICT` |
490490

491+
### A foreign key may reference any unique column, not only the key
492+
493+
`pk_field` records the column the constraint actually names, which does not have to be the parent's
494+
primary key — referencing any column with a `UNIQUE` constraint is legal on both engines, and
495+
introspection reads it back as declared:
496+
497+
```julia
498+
# live: parent_key VARCHAR(20) REFERENCES driver_registry(licence_no)
499+
# …where driver_registry's primary key is `id` and `licence_no` is UNIQUE
500+
registration = Models.ForeignKey("driver_registry", pk_field="licence_no", on_delete=CASCADE)
501+
```
502+
503+
!!! warning "A multi-column foreign key is not read back"
504+
PormG has no composite-`ForeignKey` field type, so `FOREIGN KEY (a, b) REFERENCES parent(x, y)`
505+
is **skipped** by `inspectdb`/`makemigrations` introspection on both PostgreSQL and SQLite: the
506+
member columns are imported as ordinary fields with no relation, and the constraint is left
507+
alone in the database.
508+
509+
This is deliberate — the same reject-rather-than-reinterpret rule that keeps a non-default index
510+
out of an imported model. The alternatives both regenerate a *different* schema: bind to one
511+
arbitrary member column, or emit one single-column constraint per member, which the parent will
512+
usually refuse because no member is unique on its own. Being skipped means the column reads as
513+
"no relation" on both sides of the diff, so a re-diff proposes nothing rather than churning.
514+
515+
A **single**-column foreign key into a composite-keyed parent is unaffected and reads back
516+
normally — it names one column, and that column carries its own `UNIQUE`.
517+
491518
## Timestamp fields
492519

493520
PormG does **not** implicitly add `created` / `modified` columns. Auto-managed timestamps are

0 commit comments

Comments
 (0)