Added evolution file to apply foreign key constraints to database#3692
Open
Jetpackjules wants to merge 4 commits intodevelopfrom
Open
Added evolution file to apply foreign key constraints to database#3692Jetpackjules wants to merge 4 commits intodevelopfrom
Jetpackjules wants to merge 4 commits intodevelopfrom
Conversation
misaugstad
requested changes
Oct 18, 2024
Member
misaugstad
left a comment
There was a problem hiding this comment.
@Jetpackjules these are small things to clean up, but I'll throw them to you now so that you can change them before I get a chance to fully review/test this!
- I don't see the comments as adding anything, really. It's basically as verbose as the SQL itself, and the SQL is pretty self explanatory, so I think that we can just remove these comments.
- Let's reduce the length of the file slightly by combining the last two lines of each:
ALTER TABLE audit_task ADD CONSTRAINT fk_audit_task_amt_assignment FOREIGN KEY (amt_assignment_id) REFERENCES amt_assignment(amt_assignment_id); - And for the Down section, let's just make each of those one line!
- For the Downs section, technically they should be in reverse order compared to the Ups... If the Downs are "undoing" the Ups, then we go in reverse order! Doesn't have any practical implications for this file, but we should stick to it because it sometimes matters!
I changed the evolution number to fix the conflict and did the simple requested reformatting.
Collaborator
Author
|
Thanks for the preliminary feedback, I've implemented the changes! |
misaugstad
requested changes
Nov 4, 2024
Member
misaugstad
left a comment
There was a problem hiding this comment.
Another couple things to change before I test!
Used default constraint names and fixed downs by specifying table
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #3574
The issue was that missing foreign key constraints would allow for certain incorrect insertions or deletions (Like null), and "removing data wasn't cascading to all the tables you'd expect." I've added a DB conf evolution that should manually add all the proper foreign key constraints and fix the issue.
BEFORE:
AFTER:
Testing instructions
To check if the fix worked, check foreign key constraints in the relevant database with:
Run these lines in a separate WSL terminal in the root directory of the project:
(The above should output a table listing all foreign key constraints)
Things to check before submitting the PR