-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add if not exists constraint to create table on upgrade paths #3892
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
Conversation
|
@blueoranguran package |
|
@blueorangutan package |
|
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-883 |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
yadvr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, did not check it thoroughly - the changes in the schema-2xx are unnecessary.
|
Trillian test result (tid-1029)
|
|
wow.... what, why? -1 If there was previous failure on upgrade, bunch of other things will fail, like extending the existing table or such. I saw the issue which this PR is supposed to fix, and I vote for closing both. The correct action is failed upgrade --> roll back DB --> start again. |
|
To further explain myself - if you attempt the upgrade on top of DB which previously failed to be upgraded, you might end up with duplicate entries trying to be inserted and that would be chaos. |
|
@andrijapanicsb I almost agree with you. There is a possibility that two upgrade paths produce conflicting table definitions. In those cases (and in principle) the "CREATE IF NOT EXISTS" pattern is very good to hav as a standard. Double entries are a whole different issue. |
Description
Simply add 'IF NOT EXISTS' constraint to 'CREATE TABLE' SQLs on upgrade paths. This avoids upgrade paths to fail if there had been previous failures on the upgrade process.
Fixes: #3891
Types of changes
Screenshots (if appropriate):
How Has This Been Tested?