fix: backfill missing request_stats columns on existing databases - #179
Open
marcuslannister wants to merge 1 commit into
Open
fix: backfill missing request_stats columns on existing databases#179marcuslannister wants to merge 1 commit into
marcuslannister wants to merge 1 commit into
Conversation
CREATE TABLE IF NOT EXISTS is a no-op on an existing table, so trace_id and timing_spans were never added to databases created before those columns existed. On SQLite this crashed the service at startup (CREATE INDEX on the missing column); on Postgres it would silently fail per-request inserts. Split each backend's schema into tables/indexes and run a column migration in between, driven by one shared column list so future columns only need one entry instead of a new backend-specific patch. Both migrations check existing columns first, so an already-migrated database never issues an ALTER (avoids requiring ALTER privilege on every startup for least-privilege Postgres roles).
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.
Summary
Databases created before the
trace_idandtiming_spanscolumns existed onrequest_statsnever got them, because the schema init usesCREATE TABLE IF NOT EXISTS— a no-op when the table already exists:CREATE INDEX ix_request_stats_trace_id ON request_stats(trace_id)fails outright when the column doesn't exist.request_statswithtiming_spansset (logged asrust_persistence_write_error, not fatal, but stats stopped recording).Hit this in production on an upgrade from an older image against a pre-existing
stats.db.Changes
*_SCHEMA_TABLES/*_SCHEMA_INDEXES, and run a column-backfill migration in between table creation and index creation.REQUEST_STATS_COLUMNSis a single(name, sqlite_type, postgres_type)list shared by both backends — adding a future column only needs one entry here instead of a new one-offALTER TABLEin two places.migrate_sqlite_columnsandmigrate_postgres_columnscheck existing columns first (PRAGMA table_info/information_schema.columns) and only issueALTER TABLE ADD COLUMNfor what's actually missing. This matters for Postgres in particular: an unconditionalALTER TABLE ... ADD COLUMN IF NOT EXISTSstill requiresALTERprivilege even when the column already exists, which would break startup for a least-privilege application role on an already-migrated database.ALTER TABLE ADD COLUMN, no drops/rewrites — no data loss.Test plan
cargo test --bin uni-api-front persistence— existing tests pass, plus a new test covering backfill from an old schema and idempotency on re-run.cargo clippy --bin uni-api-front— clean on this file.