mirror of
https://tangled.org/evan.jarrett.net/at-container-registry
synced 2026-09-10 04:06:06 +00:00
appview: stop the schema-drift check reporting phantom index differences
reportSchemaDrift logged 76 differences on every boot, all false positives: 38 indexes each reported twice, once as missing from the database and once as present but undeclared. The only difference was a single space before the column list. The two sides of the comparison are built differently. referenceSnapshot() applies schema.sql to a local in-memory libsql, which stores the CREATE text verbatim as "ON t(col)". Production is an embedded replica syncing to Bunny, whose parser re-emits normalized DDL as "ON t (col)". describeIndexes collapsed whitespace runs but could not normalize a space that exists on one side only, and SchemaDrift compares by exact string. Normalize whitespace adjacent to ( ) and , so both spellings converge. Space after ) is deliberately left alone, and the space before DESC is untouched, so column order and direction still have to match. This mattered because the check exists to catch a migration recorded but not executed (0004 was, which is why 0009 exists). At 76 phantom findings, real drift would have been one line among 77, under a hint telling the operator to write a corrective migration that is not needed. The first boot after this lands is the first honest reading of that warning. The existing tests all passed because they run local-only, which is exactly how this shipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9
This commit is contained in:
co-authored by
Claude Opus 5
parent
a63b839613
commit
2ee5a35525
+37
-4
@@ -22,7 +22,42 @@ type schemaSnapshot struct {
|
||||
Indexes []string
|
||||
}
|
||||
|
||||
var driftWhitespaceRun = regexp.MustCompile(`\s+`)
|
||||
var (
|
||||
driftWhitespaceRun = regexp.MustCompile(`\s+`)
|
||||
// Space that sits immediately before "(", ")" or "," carries no meaning in
|
||||
// index DDL, and space immediately after "(" or "," carries none either.
|
||||
// Space *after* ")" is deliberately left alone, since removing it would run
|
||||
// a trailing clause into the column list.
|
||||
driftSpaceBeforePunct = regexp.MustCompile(` ([(),])`)
|
||||
driftSpaceAfterPunct = regexp.MustCompile(`([(,]) `)
|
||||
)
|
||||
|
||||
// normalizeIndexDDL reduces an index definition to a form that can be compared
|
||||
// by string equality regardless of who emitted the text.
|
||||
//
|
||||
// This matters because the two sides of the comparison do not come from the same
|
||||
// engine. referenceSnapshot applies schema.sql to a local libsql database, which
|
||||
// stores the CREATE statement verbatim; a production database is an embedded
|
||||
// replica whose writes go to the remote first, and the remote re-emits the DDL
|
||||
// through its own parser. The two spellings differ only in spacing:
|
||||
//
|
||||
// schema.sql: CREATE INDEX idx_webhooks_user ON webhooks(user_did)
|
||||
// replica: CREATE INDEX idx_webhooks_user ON webhooks (user_did)
|
||||
//
|
||||
// Collapsing whitespace *runs* cannot fix that, because the difference is a
|
||||
// space that exists on one side and not the other. Left unhandled, every index
|
||||
// was reported twice on every boot (missing, and unexpected), burying any real
|
||||
// drift under the noise.
|
||||
//
|
||||
// Only the spacing is touched; identifiers, ordering (DESC) and uniqueness all
|
||||
// still have to match exactly.
|
||||
func normalizeIndexDDL(ddl string) string {
|
||||
norm := driftWhitespaceRun.ReplaceAllString(strings.TrimSpace(ddl), " ")
|
||||
norm = strings.ReplaceAll(norm, "IF NOT EXISTS ", "")
|
||||
norm = driftSpaceBeforePunct.ReplaceAllString(norm, "$1")
|
||||
norm = driftSpaceAfterPunct.ReplaceAllString(norm, "$1")
|
||||
return norm
|
||||
}
|
||||
|
||||
// introspectSchema reads the structure of db.
|
||||
//
|
||||
@@ -129,9 +164,7 @@ func describeIndexes(db DBTX) ([]string, error) {
|
||||
if err := rows.Scan(&ddl); err != nil {
|
||||
return nil, fmt.Errorf("scan index sql: %w", err)
|
||||
}
|
||||
norm := driftWhitespaceRun.ReplaceAllString(strings.TrimSpace(ddl), " ")
|
||||
norm = strings.ReplaceAll(norm, "IF NOT EXISTS ", "")
|
||||
out = append(out, norm)
|
||||
out = append(out, normalizeIndexDDL(ddl))
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -130,6 +130,150 @@ func TestSchemaDriftDetectsMissingColumn(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestSchemaDriftIgnoresIndexSpacing covers the case that made the boot-time
|
||||
// check useless in production.
|
||||
//
|
||||
// referenceSnapshot applies schema.sql to a LOCAL libsql database, which stores
|
||||
// the CREATE statement byte for byte. Production runs an embedded replica: the
|
||||
// write goes to the remote first and the remote re-emits the DDL through its own
|
||||
// parser, which puts a space before the column list. Every index then failed the
|
||||
// equality check from both directions at once, so all 38 were reported twice and
|
||||
// the warning read "differences=76" on every single boot. Real drift would have
|
||||
// been one line in seventy-seven.
|
||||
//
|
||||
// Nothing caught it because every test runs local-only, where both sides are
|
||||
// produced by the same writer and therefore agree by accident. This test injects
|
||||
// the remote's spelling by hand so the discrimination is actually exercised.
|
||||
func TestSchemaDriftIgnoresIndexSpacing(t *testing.T) {
|
||||
db := freshSchemaDB(t)
|
||||
defer db.Close()
|
||||
|
||||
// Each pair is (index to drop, spaced respelling of the identical index).
|
||||
// The spellings on the right are what a remote-parsed database stores.
|
||||
respell := []struct{ drop, create string }{
|
||||
{"idx_webhooks_user", `CREATE INDEX idx_webhooks_user ON webhooks (user_did)`},
|
||||
{"idx_manifests_did_repo", `CREATE INDEX idx_manifests_did_repo ON manifests (did, repository)`},
|
||||
{"idx_manifests_created_at", `CREATE INDEX idx_manifests_created_at ON manifests ( created_at DESC )`},
|
||||
{"idx_instance_leases_expires", `CREATE INDEX idx_instance_leases_expires ON instance_leases (expires_at)`},
|
||||
}
|
||||
for _, r := range respell {
|
||||
if _, err := db.Exec("DROP INDEX " + r.drop); err != nil {
|
||||
t.Fatalf("drop %s: %v", r.drop, err)
|
||||
}
|
||||
if _, err := db.Exec(r.create); err != nil {
|
||||
t.Fatalf("recreate %s: %v", r.drop, err)
|
||||
}
|
||||
}
|
||||
|
||||
findings, err := SchemaDrift(db)
|
||||
if err != nil {
|
||||
t.Fatalf("SchemaDrift: %v", err)
|
||||
}
|
||||
if len(findings) != 0 {
|
||||
t.Errorf("index spacing is not a schema difference, but SchemaDrift reported %d:\n %s",
|
||||
len(findings), strings.Join(findings, "\n "))
|
||||
}
|
||||
}
|
||||
|
||||
// TestSchemaDriftDetectsChangedIndex is the other half of the pair above.
|
||||
// Normalizing spacing must not blunt the check: an index that genuinely differs
|
||||
// in its columns, its table or its uniqueness still has to be reported, in the
|
||||
// spaced spelling as much as the unspaced one.
|
||||
func TestSchemaDriftDetectsChangedIndex(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
drop string
|
||||
create string
|
||||
wantHit string
|
||||
}{
|
||||
{
|
||||
name: "different column",
|
||||
drop: "idx_webhooks_user",
|
||||
create: `CREATE INDEX idx_webhooks_user ON webhooks (url)`,
|
||||
wantHit: "idx_webhooks_user",
|
||||
},
|
||||
{
|
||||
name: "extra column",
|
||||
drop: "idx_manifests_did_repo",
|
||||
create: `CREATE INDEX idx_manifests_did_repo ON manifests (did, repository, digest)`,
|
||||
wantHit: "idx_manifests_did_repo",
|
||||
},
|
||||
{
|
||||
name: "uniqueness changed",
|
||||
drop: "idx_stars_starrer",
|
||||
create: `CREATE UNIQUE INDEX idx_stars_starrer ON stars (starrer_did)`,
|
||||
wantHit: "idx_stars_starrer",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
db := freshSchemaDB(t)
|
||||
defer db.Close()
|
||||
|
||||
if _, err := db.Exec("DROP INDEX " + tc.drop); err != nil {
|
||||
t.Fatalf("drop %s: %v", tc.drop, err)
|
||||
}
|
||||
if _, err := db.Exec(tc.create); err != nil {
|
||||
t.Fatalf("create replacement index: %v", err)
|
||||
}
|
||||
|
||||
findings, err := SchemaDrift(db)
|
||||
if err != nil {
|
||||
t.Fatalf("SchemaDrift: %v", err)
|
||||
}
|
||||
if !anyContains(findings, tc.wantHit) {
|
||||
t.Errorf("expected a finding naming %s, got: %v", tc.wantHit, findings)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestNormalizeIndexDDL pins the normalization itself, without a database in the
|
||||
// way: spacing around punctuation is noise, everything else is signal.
|
||||
func TestNormalizeIndexDDL(t *testing.T) {
|
||||
same := [][]string{
|
||||
{
|
||||
`CREATE INDEX idx_webhooks_user ON webhooks(user_did)`,
|
||||
`CREATE INDEX idx_webhooks_user ON webhooks (user_did)`,
|
||||
`CREATE INDEX IF NOT EXISTS idx_webhooks_user ON webhooks ( user_did )`,
|
||||
"CREATE INDEX idx_webhooks_user\n ON webhooks\n (user_did)",
|
||||
},
|
||||
{
|
||||
`CREATE INDEX i ON t(a, b)`,
|
||||
`CREATE INDEX i ON t(a,b)`,
|
||||
`CREATE INDEX i ON t ( a , b )`,
|
||||
},
|
||||
{
|
||||
`CREATE INDEX i ON t(created_at DESC)`,
|
||||
`CREATE INDEX i ON t (created_at DESC)`,
|
||||
},
|
||||
}
|
||||
for _, group := range same {
|
||||
want := normalizeIndexDDL(group[0])
|
||||
for _, variant := range group[1:] {
|
||||
if got := normalizeIndexDDL(variant); got != want {
|
||||
t.Errorf("spelling difference survived normalization:\n %q -> %q\n %q -> %q",
|
||||
group[0], want, variant, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
different := [][2]string{
|
||||
{`CREATE INDEX i ON t(a)`, `CREATE INDEX i ON t(b)`},
|
||||
{`CREATE INDEX i ON t(a)`, `CREATE INDEX i ON u(a)`},
|
||||
{`CREATE INDEX i ON t(a)`, `CREATE UNIQUE INDEX i ON t(a)`},
|
||||
{`CREATE INDEX i ON t(a, b)`, `CREATE INDEX i ON t(b, a)`},
|
||||
{`CREATE INDEX i ON t(a)`, `CREATE INDEX i ON t(a DESC)`},
|
||||
}
|
||||
for _, pair := range different {
|
||||
if normalizeIndexDDL(pair[0]) == normalizeIndexDDL(pair[1]) {
|
||||
t.Errorf("normalization collapsed two genuinely different indexes: %q and %q",
|
||||
pair[0], pair[1])
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestInitDBWarnsOnDriftAndStillBoots covers the integration rather than the
|
||||
// comparison: an existing database whose schema has drifted must still open.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user