fix: migration 23 is failing in production and would corrupt data if it ran - #9
Merged
Conversation
Smoke-testing the new /coverage endpoint against production showed the data was
never single-state:
OH 5,775,659 IN 249 PA 170 MI 59
oh 2 BE 9 ON 2 O 1 IH 1 0 1 PJ 1
The backfill read:
SET region = 'OH' WHERE region IS NULL OR region = '' OR region <> 'OH'
which stamps OH over Indiana, Pennsylvania and Michigan. Permanently, with
nothing in the logs. That came from collapsing two statements into one to avoid
a second table rewrite -- an optimisation that changed what the statement
meant. It is two statements again, and the comment says why they cannot be
merged: case normalisation touches only rows whose case differs, the backfill
touches only blanks, and neither is the full-table rewrite the single statement
appeared to avoid.
The guard added alongside it is the only reason this was caught rather than
deployed, but it was too strict in the other direction: it refused outright
whenever any non-OH region existed, so migration 23 could never have run in
production at all. The two conditions are now separate. Other states alone are
fine and are logged. Other states AND blanks is the genuinely ambiguous case,
because a blank could belong to any of them, and that still refuses.
Also removed a pre-check for (hash, region) collisions after case
normalisation. It can never fire: the constraint this migration replaces makes
hash unique on its own, so no two rows share a hash in any region. Verified by
trying to construct the collision and being refused by the old constraint --
which is now a test.
Verified against a database carrying the real region distribution: 'oh' merges
into 'OH', and IN, PA, MI and the junk codes are left exactly as they were.
The junk codes are left alone deliberately. BE, ON, O, IH, 0 and PJ come from
the legacy loader truncating a state name to two characters -- fixed going
forward in this branch -- but inventing a correction for 15 existing rows would
be the same guessing this migration refuses to do elsewhere.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migration 23 has failed on every boot since it deployed, and production is
still on schema 22:
Two separate bugs, both mine, both in #7. This branch carries the fix that was
pushed to that branch after it had already merged, so it never reached
main.1. The guard is right, and too strict
It refuses whenever any non-OH region exists. Production has nine, so the
migration can never run. Other states on their own are fine — what is genuinely
ambiguous is other states and blanks together, because then a blank could
belong to any of them. The two conditions are now separate; other states are
logged and left alone.
2. The backfill would have relabelled every out-of-state row as Ohio
Production is not single-state and never was:
That statement stamps OH over 478 genuine Indiana, Pennsylvania and Michigan
addresses — permanently, with nothing in the logs. The over-strict guard is
the only reason it did not run. Fixing the guard without fixing this would
have shipped the corruption on the next deploy.
It came from collapsing two statements into one to avoid a second table
rewrite — an optimisation that changed what the statement meant. It is two
statements again, with a comment saying why they cannot be merged.
Verified against production's actual data
ohmerges intoOH; everything else is untouched; the migration completes.Also removed
A pre-check for
(hash, region)collisions after case normalisation. It cannever fire — the constraint being replaced makes
hashunique on its own, sono two rows share a hash in any region. Confirmed by trying to construct the
collision and being refused by the old constraint, which is now a test.
The junk codes (
BE,ON,O,IH,0,PJ— 15 rows) are left alonedeliberately. They come from the legacy loader truncating a state name to two
characters, fixed going forward in #7, but inventing a correction for them
would be the same guessing this migration refuses to do elsewhere. A follow-up
endpoint will surface them instead.
Note on current impact
Imports are currently refused with "migrations are still running", which is the
RequireSchemaVersionguard from #7 working as intended — the ingest path needsthe index migration 23 creates. Search and geocoding are unaffected.
🤖 Generated with Claude Code