Fix schema-drift false positive on SERIAL/AUTOINCREMENT columns#84
Open
robertjbass wants to merge 1 commit into
Open
Fix schema-drift false positive on SERIAL/AUTOINCREMENT columns#84robertjbass wants to merge 1 commit into
robertjbass wants to merge 1 commit into
Conversation
The startup schema-drift check compared the metadata sqlite_type stored in __pgsqlite_schema against the type reported by PRAGMA table_info. For a SERIAL/BIGSERIAL/IDENTITY column pgsqlite records the constraint-bearing string "INTEGER PRIMARY KEY AUTOINCREMENT", but PRAGMA table_info only returns the bare declared type token "INTEGER". normalize_sqlite_type did not strip the constraint suffix, so the two never compared equal and any existing database with a SERIAL column refused to open with a spurious "expected SQLite type 'INTEGER PRIMARY KEY AUTOINCREMENT' but found 'INTEGER'" drift error. Strip trailing column constraints (PRIMARY KEY, AUTOINCREMENT, NOT NULL, etc.) before comparing types. The drift check only compares column type, not constraints, so dropping the suffix is safe and still catches real type drift. Add a regression test that reproduces the SERIAL case.
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.
Problem
The startup schema-drift check added in v0.0.22 refuses to open any existing database that has a
SERIAL/BIGSERIAL/IDENTITYcolumn:Cause: when a SERIAL column is created, pgsqlite stores the constraint-bearing string
INTEGER PRIMARY KEY AUTOINCREMENTas the column'ssqlite_typein__pgsqlite_schema, but the drift check reads the live column type viaPRAGMA table_info, which only reports the bare declared type token (INTEGER).normalize_sqlite_typestrips size specs but not constraint keywords, so the two sides can never compare equal and every database containing a SERIAL column fails to open on its next start.We hit this in production: a database created normally refused to boot on its first restart, with the exact error above.
Fix
Strip trailing column constraints (
PRIMARY KEY,AUTOINCREMENT,NOT NULL,UNIQUE,DEFAULT,CHECK,REFERENCES,COLLATE,CONSTRAINT,GENERATED) from the declared type before normalizing. Truncation happens on token boundaries, so multi-word base types (DOUBLE PRECISION,CHARACTER VARYING,NUMERIC(10, 2)) are preserved. The drift check only ever compared column types, not constraints, so no real drift signal is lost - genuine type mismatches (e.g. metadataTEXTvs actualINTEGER) are still detected, and there is an existing test asserting that.Testing
test_no_drift_serial_autoincrement_columnreproduces the SERIAL case end to end (metadata row as written by the SERIAL translation path, real table withid INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL) and asserts no drift is reported.cargo test --test schema_drift_test: 9/9 pass (all pre-existing drift-detection behaviors unchanged).