Skip to content

fix(sqlite): preserve channel references during migration - #4

Merged
D3SOX merged 1 commit into
mainfrom
safe-sqlite-channel-migration
Aug 24, 2026
Merged

fix(sqlite): preserve channel references during migration#4
D3SOX merged 1 commit into
mainfrom
safe-sqlite-channel-migration

Conversation

@D3SOX

@D3SOX D3SOX commented Aug 24, 2026

Copy link
Copy Markdown
Member

The upstream channel-avatar migration rebuilds SQLite's channel table while foreign keys are enabled. Dropping that parent table executes ON DELETE actions; a replay against the production backup deleted all 42,923 video rows.

Run the rebuild outside Diesel's transaction so foreign-key enforcement can be paused, then perform the table replacement inside an explicit transaction and re-enable enforcement afterward. The rollback now copies channel rows instead of dropping them, using an empty avatar when reverting a nullable value.

A regression test applies and reverts the real embedded migration with a video referencing its channel. It checks that both rows survive, foreign keys are re-enabled, and no violations remain.

Verification:

  • cargo test --all-targets: 38 passed
  • cargo clippy --all-targets -- -D warnings: clean
  • PostgreSQL tests and Clippy: clean
  • In-memory replay against the latest production backup preserved 17 accounts, 7,840 channels, 42,923 videos, and 116 encrypted-sync rows; foreign_key_check returned zero violations and integrity_check returned ok

SQLite executes ON DELETE actions when the migration drops the channel table with foreign keys enabled. Pause enforcement around an explicit transactional rebuild, preserve channel rows on rollback, and cover both directions with the real embedded migration.
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8c6ef2c4-e28a-4e10-9375-b3c0db9578fa

📥 Commits

Reviewing files that changed from the base of the PR and between c5485c0 and 91490cb.

📒 Files selected for processing (4)
  • migrations/sqlite/2026-08-18-132346-0000_channel/down.sql
  • migrations/sqlite/2026-08-18-132346-0000_channel/metadata.toml
  • migrations/sqlite/2026-08-18-132346-0000_channel/up.sql
  • src/main.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The SQLite channel migration now rebuilds the table without discarding rows, preserves referencing video rows, manages foreign-key enforcement and transactions, and validates forward and reverse migrations with an in-memory SQLite test.

Changes

SQLite channel migration

Layer / File(s) Summary
Channel table rebuild handling
migrations/sqlite/2026-08-18-132346-0000_channel/up.sql, migrations/sqlite/2026-08-18-132346-0000_channel/down.sql, migrations/sqlite/2026-08-18-132346-0000_channel/metadata.toml
The forward and reverse migrations rebuild channel through a temporary table, preserve rows, convert null avatars to empty strings, manage foreign-key enforcement, and use explicit transaction handling.
Migration preservation test
src/main.rs
SQLite test helpers and a migration test verify channel and video row counts, foreign-key status, nullable avatar handling, and foreign-key integrity after migration and rollback.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 91490

The migration now preserves channel references during SQLite table replacement and restores foreign-key enforcement, with regression and data-integrity checks covering the change. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: unixource

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main SQLite migration fix: preserving channel references.
Description check ✅ Passed The description directly explains the migration fix, regression test, and verification results.
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch safe-sqlite-channel-migration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@macroscopeapp

macroscopeapp Bot commented Aug 24, 2026

Copy link
Copy Markdown

Approvability

Verdict: Would Approve

Macroscope's review found this PR approvable — This is a narrowly scoped SQLite migration correction that preserves channel-dependent rows during the table rebuild and restores foreign-key enforcement afterward. The accompanying regression test covers both referenced data retention and migration cleanup without changing application APIs or introducing new production components.

Not approved because:

  • Credit balance exhausted. Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more.

@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issues identified.

The migration preserves the existing channel schema apart from the intended avatar nullability change, protects dependent rows during both migration directions, and tests the embedded migration’s apply and revert paths with foreign-key enforcement and integrity checks.

Important Files Changed

Filename Overview
migrations/sqlite/2026-08-18-132346-0000_channel/up.sql Rebuilds the channel table with foreign keys paused and an explicit transaction, preserving dependent rows while making avatar nullable.
migrations/sqlite/2026-08-18-132346-0000_channel/down.sql Restores the non-null avatar schema while copying existing channels and safely coercing null avatars.
migrations/sqlite/2026-08-18-132346-0000_channel/metadata.toml Disables Diesel's outer transaction so the migration can change SQLite foreign-key enforcement before beginning its own transaction.
src/main.rs Adds a regression test that applies and reverts the real embedded migration while checking row preservation and foreign-key integrity.

Reviews (1): Last reviewed commit: "fix(sqlite): preserve channel references..." | Re-trigger Greptile

@D3SOX
D3SOX merged commit af3a990 into main Aug 24, 2026
12 checks passed
@D3SOX
D3SOX deleted the safe-sqlite-channel-migration branch August 24, 2026 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant