Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DB schema migrations #1797

Merged
merged 1 commit into from
May 10, 2024
Merged

Fix DB schema migrations #1797

merged 1 commit into from
May 10, 2024

Conversation

apiraino
Copy link
Contributor

r? @jackh726

- Fix a wrong migration introduced in rust-lang#1781
- Clarify how DB migrations should be added to the triagebot
Comment on lines -335 to +340
CREATE EXTENSION intarray;
CREATE UNIQUE INDEX review_prefs_user_id ON review_prefs(user_id);
CREATE EXTENSION IF NOT EXISTS intarray;",
"
CREATE UNIQUE INDEX IF NOT EXISTS review_prefs_user_id ON review_prefs(user_id);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't check the schema of the triagebot in prod but the clauses IF NOT EXISTS should ensure nothing bad happens when applied.

@jackh726
Copy link
Member

I think we should get eyes on this from @Mark-Simulacrum as I'm not very familiar with how migrations work and what the current state of the db is.

@ehuss
Copy link
Contributor

ehuss commented May 1, 2024

Will this fix the errors about function sort(integer[]) does not exist?

@apiraino
Copy link
Contributor Author

apiraino commented May 3, 2024

Will this fix the errors about function sort(integer[]) does not exist?

@ehuss Yes and now I know why. Without this patch the migration would fail:

$ cargo run --bin triagebot
...
Failed to run server: database migrations

Caused by:
    0: executing 12th migration
    1: db error: ERROR: cannot insert multiple commands into a prepared statement
    2: ERROR: cannot insert multiple commands into a prepared statement

unsure why it didn't happen (or wasn't catched) during the triagebot deployment at the time 🤷‍♂️

(I probably missed that on my local workstation because I manually fiddled with the DB schema without proberly testing the migration process)

EDIT: and that error we see in triagebot logs is triggered when from Zulip I request my PR work queue

Copy link
Member

@jackh726 jackh726 left a comment

Choose a reason for hiding this comment

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

After talking with @Kobzol seems like this is fine

@jackh726 jackh726 merged commit fb2e814 into rust-lang:master May 10, 2024
2 checks passed
@apiraino apiraino deleted the fix-db-schema branch May 10, 2024 15:05
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.

3 participants