From 14ba06b4aa448404cc06816b8d01547c6d07c1e3 Mon Sep 17 00:00:00 2001 From: Bob Bass Date: Mon, 13 Jul 2026 11:42:57 -0500 Subject: [PATCH] fix(drift): stop false-positive on SERIAL/AUTOINCREMENT columns 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. --- src/schema_drift.rs | 55 +++++++++++++++++++++++++++++++++++--- tests/schema_drift_test.rs | 46 ++++++++++++++++++++++++++++++- 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/src/schema_drift.rs b/src/schema_drift.rs index 6173ff4b..f8f9499b 100644 --- a/src/schema_drift.rs +++ b/src/schema_drift.rs @@ -217,12 +217,22 @@ impl SchemaDriftDetector { fn normalize_sqlite_type(type_str: &str) -> String { let upper = type_str.to_uppercase(); - + + // Strip inline column constraints that pgsqlite bakes into the stored + // metadata sqlite_type. SERIAL/BIGSERIAL/IDENTITY columns are stored in + // __pgsqlite_schema as e.g. "INTEGER PRIMARY KEY AUTOINCREMENT", but + // PRAGMA table_info only ever reports the bare declared type token + // ("INTEGER"). Without stripping the constraint suffix the two sides can + // never compare equal, producing a false "schema drift" that refuses to + // open an otherwise-valid database. The drift check only compares the + // column type, not its constraints, so dropping the suffix is safe. + let type_only = Self::strip_column_constraints(&upper); + // Remove any size/precision specifications - let base_type = if let Some(paren_pos) = upper.find('(') { - upper[..paren_pos].trim().to_string() + let base_type = if let Some(paren_pos) = type_only.find('(') { + type_only[..paren_pos].trim().to_string() } else { - upper.trim().to_string() + type_only.trim().to_string() }; // Normalize common variations @@ -239,4 +249,41 @@ impl SchemaDriftDetector { _ => base_type, } } + + /// Drop trailing column constraints from a declared type expression, + /// keeping only the leading type token(s) (e.g. "DOUBLE PRECISION", + /// "CHARACTER VARYING", "NUMERIC(10, 2)"). The input is expected to be + /// already upper-cased. Truncation happens at the first constraint keyword + /// on a token boundary, so "INTEGER PRIMARY KEY AUTOINCREMENT" becomes + /// "INTEGER" while multi-word base types are preserved. + fn strip_column_constraints(upper_type: &str) -> String { + const CONSTRAINT_KEYWORDS: &[&str] = &[ + "PRIMARY", + "KEY", + "AUTOINCREMENT", + "NOT", + "NULL", + "UNIQUE", + "DEFAULT", + "CHECK", + "REFERENCES", + "COLLATE", + "CONSTRAINT", + "GENERATED", + ]; + + let mut kept: Vec<&str> = Vec::new(); + for token in upper_type.split_whitespace() { + if CONSTRAINT_KEYWORDS.contains(&token) { + break; + } + kept.push(token); + } + + if kept.is_empty() { + upper_type.trim().to_string() + } else { + kept.join(" ") + } + } } \ No newline at end of file diff --git a/tests/schema_drift_test.rs b/tests/schema_drift_test.rs index 345f65f9..a9e7cba3 100644 --- a/tests/schema_drift_test.rs +++ b/tests/schema_drift_test.rs @@ -324,6 +324,50 @@ fn test_type_normalization() -> rusqlite::Result<()> { // Should detect no drift despite type variations let drift = SchemaDriftDetector::detect_drift(&conn).unwrap(); assert!(drift.is_empty()); - + + Ok(()) +} + +#[test] +fn test_no_drift_serial_autoincrement_column() -> rusqlite::Result<()> { + // Regression: a SERIAL/BIGSERIAL column is stored in __pgsqlite_schema with + // the constraint-bearing sqlite_type "INTEGER PRIMARY KEY AUTOINCREMENT" + // (see TypeMapper::pg_to_sqlite_for_create_table), while PRAGMA table_info + // reports only the bare "INTEGER" type token. The drift checker must not + // treat this as a type mismatch, otherwise an existing database created via + // `CREATE TABLE ... (id SERIAL PRIMARY KEY)` fails to open with a spurious + // "expected SQLite type 'INTEGER PRIMARY KEY AUTOINCREMENT' but found + // 'INTEGER'" schema-drift error. + let mut conn = Connection::open_in_memory()?; + + TypeMetadata::init(&conn)?; + + // The table as pgsqlite actually creates it from a translated SERIAL DDL. + conn.execute( + "CREATE TABLE drift_test ( + id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, + name TEXT + )", + [] + )?; + + // The metadata as pgsqlite actually records it for a SERIAL column. + let tx = conn.transaction()?; + tx.execute( + "INSERT INTO __pgsqlite_schema (table_name, column_name, pg_type, sqlite_type) + VALUES + ('drift_test', 'id', 'int4', 'INTEGER PRIMARY KEY AUTOINCREMENT'), + ('drift_test', 'name', 'text', 'TEXT')", + [] + )?; + tx.commit()?; + + let drift = SchemaDriftDetector::detect_drift(&conn).unwrap(); + assert!( + drift.is_empty(), + "SERIAL/AUTOINCREMENT column should not be reported as drift: {}", + drift.format_report() + ); + Ok(()) } \ No newline at end of file