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

Online not null constraints: SET NOT NULL #83

Merged
merged 6 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ $ pg-schema-diff plan --dsn "postgres://postgres:postgres@localhost:5432/postgre
* 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 builds: Check constraints are added as `INVALID` before being validated, eliminating the need
for a long access-exclusive lock on the table
* Online `ALTER ... SET NOT NULL` using check constraints to eliminate the need for an access-exclusive lock on the table
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Once this gets merged in, I'll cut a follow-up PR that highlights this behavior via an example, since I think showing rather than telling will work better.

* Prioritized index builds: Building new indexes is always prioritized over deleting old indexes
* A comprehensive set of features to ensure the safety of planned migrations:
* Operators warned of dangerous operations.
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/google/uuid v1.3.0
github.com/jackc/pgx/v4 v4.14.0
github.com/kr/pretty v0.3.1
github.com/lib/pq v1.10.2
github.com/manifoldco/promptui v0.9.0
github.com/mitchellh/hashstructure/v2 v2.0.2
github.com/spf13/cobra v1.7.0
Expand Down
27 changes: 27 additions & 0 deletions internal/migration_acceptance_tests/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"testing"

"github.com/google/uuid"
_ "github.com/jackc/pgx/v4/stdlib"
"github.com/kr/pretty"
"github.com/stretchr/testify/suite"
Expand Down Expand Up @@ -42,6 +43,10 @@ type (
oldSchemaDDL []string
newSchemaDDL []string

// ddl is used to assert the exact DDL (of the statements) that generated. This is useful when asserting
// exactly how a migration is performed
ddl []string

// expectedHazardTypes should contain all the unique migration hazard types that are expected to be within the
// generated plan
expectedHazardTypes []diff.MigrationHazardType
Expand Down Expand Up @@ -86,6 +91,8 @@ func (suite *acceptanceTestSuite) runTestCases(acceptanceTestCases []acceptanceT
}

func (suite *acceptanceTestSuite) runSubtest(tc acceptanceTestCase, expects expectations, planOpts []diff.PlanOpt) {
uuid.SetRand(&deterministicRandReader{})

// normalize the subtest
if expects.outputState == nil {
expects.outputState = tc.newSchemaDDL
Expand Down Expand Up @@ -145,6 +152,14 @@ func (suite *acceptanceTestSuite) runSubtest(tc acceptanceTestCase, expects expe
newDbDump := suite.directlyRunDDLAndGetDump(expects.outputState)
suite.Equal(newDbDump, oldDbDump, prettySprintPlan(plan))

if tc.ddl != nil {
var generatedDDL []string
for _, stmt := range plan.Statements {
generatedDDL = append(generatedDDL, stmt.DDL)
}
suite.Equal(tc.ddl, generatedDDL)
}

// Make sure no diff is found if we try to regenerate a plan
plan, err = diff.GeneratePlan(context.Background(), oldDbConn, tempDbFactory, tc.newSchemaDDL, planOpts...)
suite.Require().NoError(err)
Expand Down Expand Up @@ -204,6 +219,18 @@ func prettySprintPlan(plan diff.Plan) string {
return fmt.Sprintf("%# v", pretty.Formatter(plan.Statements))
}

type deterministicRandReader struct {
counter int8
}

func (r *deterministicRandReader) Read(p []byte) (int, error) {
for i := 0; i < len(p); i++ {
p[i] = byte(r.counter)
r.counter++
}
return len(p), nil
}

func TestAcceptanceSuite(t *testing.T) {
suite.Run(t, new(acceptanceTestSuite))
}
48 changes: 48 additions & 0 deletions internal/migration_acceptance_tests/check_constraint_cases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,29 @@ var checkConstraintCases = []acceptanceTestCase{
`,
},
},
{
name: "Add check constraint and change data type",
oldSchemaDDL: []string{
`
CREATE TABLE foobar(
id INT PRIMARY KEY,
foo VARCHAR(255)
);
`,
},
newSchemaDDL: []string{
`
CREATE TABLE foobar(
id INT PRIMARY KEY,
foo INT CHECK ( foo > 0 )
);
`,
},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
diff.MigrationHazardTypeImpactsDatabasePerformance,
},
},
{
name: "Add check constraint with quoted identifiers",
oldSchemaDDL: []string{
Expand Down Expand Up @@ -258,6 +281,31 @@ var checkConstraintCases = []acceptanceTestCase{
`,
},
},
{
name: "Drop check constraint and change data type",
oldSchemaDDL: []string{
`
CREATE TABLE foobar(
"ID" INT PRIMARY KEY,
foo VARCHAR(255),
"Bar" BIGINT CHECK ( "Bar" > 0 )
);
`,
},
newSchemaDDL: []string{
`
CREATE TABLE foobar(
"ID" INT PRIMARY KEY,
foo VARCHAR(255),
"Bar" TEXT
);
`,
},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
diff.MigrationHazardTypeImpactsDatabasePerformance,
},
},
{
name: "Drop column with check constraints",
oldSchemaDDL: []string{
Expand Down
Loading