Online check constraint addition #76
Merged
Conversation
This file contains 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
| @@ -1461,20 +1476,20 @@ func (csg *checkConstraintSQLGenerator) Add(con schema.CheckConstraint) ([]State | |||
|
|
|||
| if !con.IsValid { | |||
| sb.WriteString(" NOT VALID") | |||
| } else if !csg.isNewTable { | |||
There was a problem hiding this comment.
We strip the migration hazards for new tables within the TableSQLGenerator, so we don't actually need this conditional
alexaub-stripe
previously approved these changes
Oct 25, 2023
README.md
Outdated
| @@ -43,10 +43,13 @@ $ pg-schema-diff plan --dsn "postgres://postgres:postgres@localhost:5432/postgre | |||
| * The use of postgres native operations for zero-downtime migrations wherever possible: | |||
| * Concurrent index builds | |||
| * Online index replacement: If some index is changed, the new version will be built before the old version is dropped, preventing a window where no index is backing queries | |||
| * Online check constraint addition: Check constraints are added as `INVALID` before being validated, eliminating the need | |||
There was a problem hiding this comment.
Maybe use builds here? Addition is a bit of an awkward term in my head
Suggested change
| * Online check constraint addition: Check constraints are added as `INVALID` before being validated, eliminating the need | |
| * Online check constraint builds: Check constraints are added as `INVALID` before being validated, eliminating the need |
| @@ -838,6 +838,48 @@ var ( | |||
| expectedStatements: nil, | |||
| expectedDiffErrIs: errDuplicateIdentifier, | |||
| }, | |||
| { | |||
| name: "Online check constraint addition", | |||
There was a problem hiding this comment.
thought: Do we already have a case for the validation of a previously invalid constraint?
There was a problem hiding this comment.
This is a different test case:
- one is testing altering a check constraint from invalid to valid
- one is testing that add a constraint is online
| // UDF's in check constraints are a bad idea. Check constraints are not re-validated | ||
| // if the UDF changes, so it's not really a safe practice. We won't support it for now | ||
| if len(con.DependsOnFunctions) > 0 { | ||
| return nil, fmt.Errorf("check constraints that depend on UDFs: %w", ErrNotImplemented) | ||
| } | ||
|
|
||
| var stmts []Statement | ||
| if !con.IsValid || csg.isNewTable { |
There was a problem hiding this comment.
If !con.IsValid shouldn't we have a validateConstraintStatement?
There was a problem hiding this comment.
Nope! Because the user is adding an invalid constraint! (They want it to be invalid)
|
r? |
alexaub-stripe
approved these changes
Oct 25, 2023
bplunkett-stripe
deleted the
bplunkett/online-check-constraint-addition
branch
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.
Description
This was a pretty low-hanging fruit. We now add all check constraints as invalid and then validate them. This allows us to build check constraints "online".
Motivation
Closes #66
Testing
Tested with unit tests