-
Notifications
You must be signed in to change notification settings - Fork 106
Make column duplication idempotent #946
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
Merged
Merged
Conversation
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
2e2786b
to
e4632d4
Compare
e4632d4
to
9bd2749
Compare
9bd2749
to
0162b2b
Compare
0162b2b
to
0038ea3
Compare
kvch
commented
Aug 7, 2025
kvch
commented
Aug 7, 2025
andrew-farries
approved these changes
Aug 12, 2025
pkg/migrations/coordinator.go
Outdated
@@ -33,7 +33,7 @@ func NewCoordinator(actions []DBAction) *Coordinator { | |||
} | |||
} | |||
|
|||
// Execute runs all actions in the order they were added to the coordinator. | |||
// Execute runs all actions in the orderedActions they were added to the coordinator. |
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.
don't need this change I think
Merging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
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.
This PR makes column duplication idempotent. Previously, if the same column was altered in multiple operations in the same migration was not working. The modified column could not be duplicated in the second operation because the
ALTER TABLE
statement has failed. I addedIF NOT EXISTS
to make the column creation idempotent. Now the column is duplicated once, and other changes are added on top of it like setting not null constraints, changing default values, etc.The only edge case this PR does not address is when the column type is changed in a later operation. This limitation can be worked around by changing the type first in an
alter_column
operation and then adding other changes to the migration. I do opted for this trade-off because it would increase the complexity of the duplicator but not much of an upside.This is working:
This is not supported: