fix(migrations): stop 7bf4eac76958 renaming link constraints to a sha… - #988
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughChangesMigration constraint changes
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@northdpole here is the PR with the fix as discussed I need a review and merge so I can take a fresh pull to continue with my next PR. |
northdpole
left a comment
There was a problem hiding this comment.
Thanks for tracking this down and fixing it cleanly.
Approved: removing the dual rename to uq_pair is the right fix for empty Postgres flask db upgrade (schema-global unique indexes), upgrade/downgrade stay consistent, and the real section_id / uq_node work is untouched. Models already use uq_cre_link_pair / uq_cre_node_link_pair.
Merging with rebase — appreciate the careful migration surgery.
Problem
A from-scratch
flask db upgradeon PostgreSQL fails:Migration
7bf4eac76958renames the unique constraints on bothcre_links(
uq_cre_link_pair) andcre_node_links(uq_cre_node_link_pair) to the samename
uq_pair. On Postgres a UNIQUE constraint creates a schema-global index,so the second
uq_paircollides with the first — breaking any from-emptymigration on Postgres. (It slips by on SQLite, where
batch_alter_tablerebuilds the table.)
Fix
Remove those two renames from
7bf4eac76958(upgrade + downgrade). The precedingmigration
0d267ae11945already creates the table-specific constraints, and theSQLAlchemy models keep those names (
uq_cre_link_pair/uq_cre_node_link_pair),so we preserve them instead of collapsing both to
uq_pair. The migration'sactual purpose (adding
node.section_idand updatinguq_node) is unchanged.Validation
Full
flask db upgradefrom an empty PostgreSQL database now completes the entirechain to head
c7d8e9f0a1b2.Note: completing the run also requires
CRE_EMBED_EXPECTED_DIMto be set for thepgvector migration (
c7d8e9f0a1b2) on an empty DB — a separate, pre-existingrequirement of that migration, unrelated to this fix.