Skip to content

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

Merged
ogazboiz merged 2 commits into
LabsCrypt:mainfrom
SharifIbrahimDev:fix/migration-rollback-leak
Jul 3, 2026
Merged

fix: implement down in add-loan-events-missing-indexes to drop indexes#1290
ogazboiz merged 2 commits 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

@ogazboiz ogazboiz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the down() is correct (drops exactly the four indexes the up() creates, all IF EXISTS), and you dropped the stray pr_body file and rebased, so it's clean and green on current main now. merging.

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

@ogazboiz ogazboiz merged commit 06ba454 into LabsCrypt:main Jul 3, 2026
9 checks passed
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