feat(db): add migration 028 for discussions.merge_request_id FK constraint
Add foreign key constraint on discussions.merge_request_id to prevent orphaned discussions when MRs are deleted. SQLite doesn't support ALTER TABLE ADD CONSTRAINT, so this migration recreates the table with: 1. New table with FK: REFERENCES merge_requests(id) ON DELETE CASCADE 2. Data copy with FK validation (only copies rows with valid MR references) 3. Table swap (DROP old, RENAME new) 4. Full index recreation (all 10 indexes from migrations 002-022) The migration also includes a CHECK constraint ensuring mutual exclusivity: - Issue discussions have issue_id NOT NULL and merge_request_id NULL - MR discussions have merge_request_id NOT NULL and issue_id NULL Also fixes run_migrations() to properly propagate query errors instead of silently returning unwrap_or defaults, improving error diagnostics. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
58
migrations/028_discussions_mr_fk.sql
Normal file
58
migrations/028_discussions_mr_fk.sql
Normal file
@@ -0,0 +1,58 @@
|
||||
-- Migration 028: Add FK constraint on discussions.merge_request_id
|
||||
-- Schema version: 28
|
||||
-- Fixes missing foreign key that causes orphaned discussions when MRs are deleted
|
||||
|
||||
-- SQLite doesn't support ALTER TABLE ADD CONSTRAINT, so we must recreate the table.
|
||||
|
||||
-- Step 1: Create new table with the FK constraint
|
||||
CREATE TABLE discussions_new (
|
||||
id INTEGER PRIMARY KEY,
|
||||
gitlab_discussion_id TEXT NOT NULL,
|
||||
project_id INTEGER NOT NULL REFERENCES projects(id) ON DELETE CASCADE,
|
||||
issue_id INTEGER REFERENCES issues(id) ON DELETE CASCADE,
|
||||
merge_request_id INTEGER REFERENCES merge_requests(id) ON DELETE CASCADE, -- FK was missing!
|
||||
noteable_type TEXT NOT NULL CHECK (noteable_type IN ('Issue', 'MergeRequest')),
|
||||
individual_note INTEGER NOT NULL DEFAULT 0,
|
||||
first_note_at INTEGER,
|
||||
last_note_at INTEGER,
|
||||
last_seen_at INTEGER NOT NULL,
|
||||
resolvable INTEGER NOT NULL DEFAULT 0,
|
||||
resolved INTEGER NOT NULL DEFAULT 0,
|
||||
raw_payload_id INTEGER REFERENCES raw_payloads(id), -- Added in migration 004
|
||||
CHECK (
|
||||
(noteable_type = 'Issue' AND issue_id IS NOT NULL AND merge_request_id IS NULL) OR
|
||||
(noteable_type = 'MergeRequest' AND merge_request_id IS NOT NULL AND issue_id IS NULL)
|
||||
)
|
||||
);
|
||||
|
||||
-- Step 2: Copy data (only rows with valid FK references to avoid constraint violations)
|
||||
INSERT INTO discussions_new
|
||||
SELECT d.* FROM discussions d
|
||||
WHERE (d.merge_request_id IS NULL OR EXISTS (SELECT 1 FROM merge_requests m WHERE m.id = d.merge_request_id));
|
||||
|
||||
-- Step 3: Drop old table and rename
|
||||
DROP TABLE discussions;
|
||||
ALTER TABLE discussions_new RENAME TO discussions;
|
||||
|
||||
-- Step 4: Recreate ALL indexes that were on the discussions table
|
||||
-- From migration 002 (original table)
|
||||
CREATE UNIQUE INDEX uq_discussions_project_discussion_id ON discussions(project_id, gitlab_discussion_id);
|
||||
CREATE INDEX idx_discussions_issue ON discussions(issue_id);
|
||||
CREATE INDEX idx_discussions_mr ON discussions(merge_request_id);
|
||||
CREATE INDEX idx_discussions_last_note ON discussions(last_note_at);
|
||||
-- From migration 003 (orphan detection)
|
||||
CREATE INDEX idx_discussions_last_seen ON discussions(last_seen_at);
|
||||
-- From migration 006 (MR indexes)
|
||||
CREATE INDEX idx_discussions_mr_id ON discussions(merge_request_id);
|
||||
CREATE INDEX idx_discussions_mr_resolved ON discussions(merge_request_id, resolved, resolvable);
|
||||
-- From migration 017 (who command indexes)
|
||||
CREATE INDEX idx_discussions_unresolved_recent ON discussions(project_id, last_note_at) WHERE resolvable = 1 AND resolved = 0;
|
||||
CREATE INDEX idx_discussions_unresolved_recent_global ON discussions(last_note_at) WHERE resolvable = 1 AND resolved = 0;
|
||||
-- From migration 019 (list performance)
|
||||
CREATE INDEX idx_discussions_issue_resolved ON discussions(issue_id, resolvable, resolved);
|
||||
-- From migration 022 (notes query optimization)
|
||||
CREATE INDEX idx_discussions_issue_id ON discussions(issue_id);
|
||||
|
||||
-- Record migration
|
||||
INSERT INTO schema_version (version, applied_at, description)
|
||||
VALUES (28, strftime('%s', 'now') * 1000, 'Add FK constraint on discussions.merge_request_id');
|
||||
@@ -93,6 +93,10 @@ const MIGRATIONS: &[(&str, &str)] = &[
|
||||
"027",
|
||||
include_str!("../../migrations/027_surgical_sync_runs.sql"),
|
||||
),
|
||||
(
|
||||
"028",
|
||||
include_str!("../../migrations/028_discussions_mr_fk.sql"),
|
||||
),
|
||||
];
|
||||
|
||||
pub fn create_connection(db_path: &Path) -> Result<Connection> {
|
||||
@@ -130,21 +134,20 @@ pub fn create_connection(db_path: &Path) -> Result<Connection> {
|
||||
}
|
||||
|
||||
pub fn run_migrations(conn: &Connection) -> Result<()> {
|
||||
let has_version_table: bool = conn
|
||||
.query_row(
|
||||
"SELECT COUNT(*) > 0 FROM sqlite_master WHERE type='table' AND name='schema_version'",
|
||||
[],
|
||||
|row| row.get(0),
|
||||
)
|
||||
.unwrap_or(false);
|
||||
// Note: sqlite_master always exists, so errors here indicate real DB problems
|
||||
// (corruption, locked, etc.) - we must not silently treat them as "fresh DB"
|
||||
let has_version_table: bool = conn.query_row(
|
||||
"SELECT COUNT(*) > 0 FROM sqlite_master WHERE type='table' AND name='schema_version'",
|
||||
[],
|
||||
|row| row.get(0),
|
||||
)?;
|
||||
|
||||
let current_version: i32 = if has_version_table {
|
||||
conn.query_row(
|
||||
"SELECT COALESCE(MAX(version), 0) FROM schema_version",
|
||||
[],
|
||||
|row| row.get(0),
|
||||
)
|
||||
.unwrap_or(0)
|
||||
)?
|
||||
} else {
|
||||
0
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user