Skip to content

fix: implement down in add-loan-events-missing-indexes to drop indexes#1290

Open
SharifIbrahimDev wants to merge 1 commit into
LabsCrypt:mainfrom
SharifIbrahimDev:fix/migration-rollback-leak
Open

fix: implement down in add-loan-events-missing-indexes to drop indexes#1290
SharifIbrahimDev wants to merge 1 commit into
LabsCrypt:mainfrom
SharifIbrahimDev:fix/migration-rollback-leak

Conversation

@SharifIbrahimDev

Copy link
Copy Markdown

Closes #1193

What does this PR do?

This PR implements the missing down() function in the 1788000000018_add-loan-events-missing-indexes.js migration file, which previously leaked four database indexes on rollback.

Description

  • Clean Rollbacks: Replaced the empty down = () => {} with explicit DROP INDEX IF EXISTS statements for all four indexes (idx_loan_events_borrower, idx_loan_events_event_type, idx_loan_events_loan_id_event_type, and idx_loan_events_created_at) created in the up() function.
  • Idempotency: Ensures that rolling back and re-applying migrations works flawlessly without index collision errors.

@ogazboiz

ogazboiz commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

reviewed the content and it's good: the down() drops exactly the four indexes the up() creates (created_at, loan_id_event_type, event_type, borrower) with IF EXISTS, and i verified the full up -> down-all -> up-again cycle applies cleanly on postgres against the current migration set. two things before it can merge:

  1. this branch is ~40 commits behind main (it predates the commonjs->esm migration conversion), which is why ci is red, it's stale-base, not your change. please rebase: git fetch origin && git rebase origin/main && git push --force-with-lease.
  2. drop the stray pr_body_1193.md from the repo root (that belongs in the pr description, not the tree).

non-blocking: idx_loan_events_loan_id_event_type is also created by the earlier 1777 composite-indexes migration, so a partial rollback of just 1788 would drop an index 1777 also owns, the IF EXISTS guards keep it safe but it's worth a one-line comment. rebase + drop the md and i'll merge.

if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0

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.

[Backend] add-loan-events-missing-indexes has an empty down(), leaking four indexes on rollback

2 participants