diff --git a/internal/migration_acceptance_tests/acceptance_test.go b/internal/migration_acceptance_tests/acceptance_test.go index 2bced4e..988920f 100644 --- a/internal/migration_acceptance_tests/acceptance_test.go +++ b/internal/migration_acceptance_tests/acceptance_test.go @@ -157,7 +157,12 @@ func (suite *acceptanceTestSuite) runSubtest(tc acceptanceTestCase, expects expe for _, stmt := range plan.Statements { generatedDDL = append(generatedDDL, stmt.DDL) } - suite.Equal(tc.ddl, generatedDDL) + // In the future, we might want to allow users to assert expectations for vanilla options and data packing + // + // We can also make the system more advanced by using tokens in place of the "randomly" generated UUIDs, such + // the test case doesn't need to be updated if the UUID generation changes. If we built this functionality, we + // should also integrate it with the schema_migration_plan_test.go tests. + suite.Equal(tc.ddl, generatedDDL, "data packing can change the the generated UUID and DDL") } // Make sure no diff is found if we try to regenerate a plan diff --git a/internal/migration_acceptance_tests/check_constraint_cases_test.go b/internal/migration_acceptance_tests/check_constraint_cases_test.go index e837825..2a2efdb 100644 --- a/internal/migration_acceptance_tests/check_constraint_cases_test.go +++ b/internal/migration_acceptance_tests/check_constraint_cases_test.go @@ -33,7 +33,7 @@ var checkConstraintCases = []acceptanceTestCase{ }, }, { - name: "Add check constraint", + name: "Add check constraint (validate constraint added online)", oldSchemaDDL: []string{ ` CREATE TABLE foobar( @@ -52,6 +52,10 @@ var checkConstraintCases = []acceptanceTestCase{ ); `, }, + ddl: []string{ + "ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"foobar_check\" CHECK((bar > id)) NOT VALID", + "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"foobar_check\"", + }, }, { name: "Add check constraint with UDF dependency should error", @@ -383,7 +387,7 @@ var checkConstraintCases = []acceptanceTestCase{ }, }, { - name: "Alter an invalid check constraint to be valid", + name: "Alter an invalid check constraint to be valid (validate constraint isn't dropped and re-added)", oldSchemaDDL: []string{ ` CREATE TABLE foobar( @@ -404,6 +408,9 @@ var checkConstraintCases = []acceptanceTestCase{ ALTER TABLE foobar ADD CONSTRAINT bar_check CHECK ( bar > id ); `, }, + ddl: []string{ + "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"bar_check\"", + }, }, { name: "Alter a valid check constraint to be invalid", diff --git a/internal/migration_acceptance_tests/column_cases_test.go b/internal/migration_acceptance_tests/column_cases_test.go index 95610ec..8cce47b 100644 --- a/internal/migration_acceptance_tests/column_cases_test.go +++ b/internal/migration_acceptance_tests/column_cases_test.go @@ -283,6 +283,33 @@ var columnAcceptanceTestCases = []acceptanceTestCase{ diff.MigrationHazardTypeDeletesData, }, }, + { + name: "Change BIGINT to TIMESTAMP (validate conversion and ANALYZE)", + oldSchemaDDL: []string{ + ` + CREATE TABLE "Foobar"( + id INT PRIMARY KEY, + some_time_col BIGINT + ); + `, + }, + newSchemaDDL: []string{ + ` + CREATE TABLE "Foobar"( + id INT PRIMARY KEY, + some_time_col TIMESTAMP + ); + `, + }, + ddl: []string{ + "ALTER TABLE \"public\".\"Foobar\" ALTER COLUMN \"some_time_col\" SET DATA TYPE timestamp without time zone using to_timestamp(\"some_time_col\" / 1000)", + "ANALYZE \"Foobar\" (\"some_time_col\")", + }, + expectedHazardTypes: []diff.MigrationHazardType{ + diff.MigrationHazardTypeAcquiresAccessExclusiveLock, + diff.MigrationHazardTypeImpactsDatabasePerformance, + }, + }, { name: "Modify data type (varchar -> TEXT) with compatible default", oldSchemaDDL: []string{ @@ -353,7 +380,7 @@ var columnAcceptanceTestCases = []acceptanceTestCase{ }, }, { - name: "Modify collation (default -> non-default)", + name: "Modify collation (default -> non-default) (validate ANALYZE is run)", oldSchemaDDL: []string{ ` CREATE TABLE foobar( @@ -370,6 +397,10 @@ var columnAcceptanceTestCases = []acceptanceTestCase{ ); `, }, + ddl: []string{ + "ALTER TABLE \"public\".\"foobar\" ALTER COLUMN \"foobar\" SET DATA TYPE character varying(255) COLLATE \"pg_catalog\".\"POSIX\" using \"foobar\"::character varying(255)", + "ANALYZE \"foobar\" (\"foobar\")", + }, expectedHazardTypes: []diff.MigrationHazardType{ diff.MigrationHazardTypeAcquiresAccessExclusiveLock, diff.MigrationHazardTypeImpactsDatabasePerformance, @@ -473,12 +504,7 @@ var columnAcceptanceTestCases = []acceptanceTestCase{ ); `, }, - ddl: []string{ - "ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"pgschemadiff_tmpnn_10111213-1415-4617-9819-1a1b1c1d1e1f\" CHECK(\"foobar\" IS NOT NULL) NOT VALID", - "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"pgschemadiff_tmpnn_10111213-1415-4617-9819-1a1b1c1d1e1f\"", - "ALTER TABLE \"public\".\"foobar\" ALTER COLUMN \"foobar\" SET NOT NULL", - "ALTER TABLE \"public\".\"foobar\" DROP CONSTRAINT \"pgschemadiff_tmpnn_10111213-1415-4617-9819-1a1b1c1d1e1f\"", - }, + ddl: []string{"ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\" CHECK(\"foobar\" IS NOT NULL) NOT VALID", "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\"", "ALTER TABLE \"public\".\"foobar\" ALTER COLUMN \"foobar\" SET NOT NULL", "ALTER TABLE \"public\".\"foobar\" DROP CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\""}, }, { name: "Set NOT NULL (add invalid CC)", @@ -500,11 +526,11 @@ var columnAcceptanceTestCases = []acceptanceTestCase{ `, }, ddl: []string{ - "ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"pgschemadiff_tmpnn_10111213-1415-4617-9819-1a1b1c1d1e1f\" CHECK(\"foobar\" IS NOT NULL) NOT VALID", - "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"pgschemadiff_tmpnn_10111213-1415-4617-9819-1a1b1c1d1e1f\"", + "ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\" CHECK(\"foobar\" IS NOT NULL) NOT VALID", + "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\"", "ALTER TABLE \"public\".\"foobar\" ALTER COLUMN \"foobar\" SET NOT NULL", "ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"foobar\" CHECK((foobar IS NOT NULL)) NOT VALID", - "ALTER TABLE \"public\".\"foobar\" DROP CONSTRAINT \"pgschemadiff_tmpnn_10111213-1415-4617-9819-1a1b1c1d1e1f\"", + "ALTER TABLE \"public\".\"foobar\" DROP CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\"", }, }, @@ -529,10 +555,10 @@ var columnAcceptanceTestCases = []acceptanceTestCase{ `, }, ddl: []string{ - "ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"pgschemadiff_tmpnn_10111213-1415-4617-9819-1a1b1c1d1e1f\" CHECK(\"foobar\" IS NOT NULL) NOT VALID", - "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"pgschemadiff_tmpnn_10111213-1415-4617-9819-1a1b1c1d1e1f\"", + "ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\" CHECK(\"foobar\" IS NOT NULL) NOT VALID", + "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\"", "ALTER TABLE \"public\".\"foobar\" ALTER COLUMN \"foobar\" SET NOT NULL", - "ALTER TABLE \"public\".\"foobar\" DROP CONSTRAINT \"pgschemadiff_tmpnn_10111213-1415-4617-9819-1a1b1c1d1e1f\"", + "ALTER TABLE \"public\".\"foobar\" DROP CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\"", }, }, { @@ -683,17 +709,16 @@ var columnAcceptanceTestCases = []acceptanceTestCase{ diff.MigrationHazardTypeImpactsDatabasePerformance, }, ddl: []string{ - "ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"pgschemadiff_tmpnn_10111213-1415-4617-9819-1a1b1c1d1e1f\" CHECK(\"foobar\" IS NOT NULL) NOT VALID", - "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"pgschemadiff_tmpnn_10111213-1415-4617-9819-1a1b1c1d1e1f\"", + "ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\" CHECK(\"foobar\" IS NOT NULL) NOT VALID", + "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\"", "ALTER TABLE \"public\".\"foobar\" ALTER COLUMN \"foobar\" SET NOT NULL", "ALTER TABLE \"public\".\"foobar\" ALTER COLUMN \"foobar\" SET DATA TYPE integer using \"foobar\"::integer", "ANALYZE \"foobar\" (\"foobar\")", "ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"foobar_foobar_check\" CHECK((foobar > 0)) NOT VALID", "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"foobar_foobar_check\"", - "ALTER TABLE \"public\".\"foobar\" DROP CONSTRAINT \"pgschemadiff_tmpnn_10111213-1415-4617-9819-1a1b1c1d1e1f\"", + "ALTER TABLE \"public\".\"foobar\" DROP CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\"", }, }, - // TODO(bplunkett) Add not null migration where valid cc is being dropped { name: "Remove NOT NULL", oldSchemaDDL: []string{ @@ -918,29 +943,6 @@ var columnAcceptanceTestCases = []acceptanceTestCase{ diff.MigrationHazardTypeImpactsDatabasePerformance, }, }, - { - name: "Change BIGINT to TIMESTAMP, nullability (NOT NULL), and default with current_timestamp", - oldSchemaDDL: []string{ - ` - CREATE TABLE "Foobar"( - id INT PRIMARY KEY, - some_time_col BIGINT - ); - `, - }, - newSchemaDDL: []string{ - ` - CREATE TABLE "Foobar"( - id INT PRIMARY KEY, - some_time_col TIMESTAMP DEFAULT CURRENT_TIMESTAMP NOT NULL - ); - `, - }, - expectedHazardTypes: []diff.MigrationHazardType{ - diff.MigrationHazardTypeAcquiresAccessExclusiveLock, - diff.MigrationHazardTypeImpactsDatabasePerformance, - }, - }, } func (suite *acceptanceTestSuite) TestColumnAcceptanceTestCases() { diff --git a/internal/migration_acceptance_tests/foreign_key_constraint_cases_test.go b/internal/migration_acceptance_tests/foreign_key_constraint_cases_test.go index 7f65974..21af815 100644 --- a/internal/migration_acceptance_tests/foreign_key_constraint_cases_test.go +++ b/internal/migration_acceptance_tests/foreign_key_constraint_cases_test.go @@ -289,7 +289,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{ }, }, { - name: "Alter FK (not valid to valid)", + name: "Alter FK not valid to valid (validate FK isn't dropped and re-added)", oldSchemaDDL: []string{ ` CREATE TABLE foobar( @@ -319,6 +319,9 @@ var foreignKeyConstraintCases = []acceptanceTestCase{ FOREIGN KEY (fk_id) REFERENCES foobar(id); `, }, + ddl: []string{ + "ALTER TABLE \"public\".\"foobar fk\" VALIDATE CONSTRAINT \"some_fk\"", + }, }, { name: "Alter FK (valid to not valid)", diff --git a/internal/migration_acceptance_tests/index_cases_test.go b/internal/migration_acceptance_tests/index_cases_test.go index 1563e81..68e5f5b 100644 --- a/internal/migration_acceptance_tests/index_cases_test.go +++ b/internal/migration_acceptance_tests/index_cases_test.go @@ -351,6 +351,33 @@ var indexAcceptanceTestCases = []acceptanceTestCase{ diff.MigrationHazardTypeIndexDropped, }, }, + { + name: "Delete a normal index and columns (index dropped concurrently first)", + oldSchemaDDL: []string{ + ` + CREATE TABLE foobar( + id INT PRIMARY KEY, + foo VARCHAR(255) NOT NULL + ); + CREATE INDEX some_idx ON foobar(foo); + `, + }, + newSchemaDDL: []string{ + ` + CREATE TABLE foobar( + id INT PRIMARY KEY + ); + `, + }, + expectedHazardTypes: []diff.MigrationHazardType{ + diff.MigrationHazardTypeDeletesData, + diff.MigrationHazardTypeIndexDropped, + }, + ddl: []string{ + "DROP INDEX CONCURRENTLY \"some_idx\"", + "ALTER TABLE \"public\".\"foobar\" DROP COLUMN \"foo\"", + }, + }, { name: "Delete a normal index with quoted names", oldSchemaDDL: []string{ @@ -655,6 +682,36 @@ var indexAcceptanceTestCases = []acceptanceTestCase{ diff.MigrationHazardTypeIndexBuild, }, }, + { + name: "Alter index columns (index replacement and prioritized builds)", + oldSchemaDDL: []string{` + CREATE TABLE foobar( + foo TEXT, + bar INT + ); + CREATE INDEX some_idx_with_a_very_long_name ON foobar(foo); + CREATE INDEX old_idx ON foobar(bar); + `}, + newSchemaDDL: []string{` + CREATE TABLE foobar( + foo TEXT, + bar INT + ); + CREATE INDEX some_idx_with_a_very_long_name ON foobar(foo, bar); + CREATE INDEX new_idx ON foobar(bar); + `}, + ddl: []string{ + "ALTER INDEX \"some_idx_with_a_very_long_name\" RENAME TO \"pgschemadiff_tmpidx_some_idx_with_a_very_EBESExQVRheYGRobHB0eHw\"", + "CREATE INDEX CONCURRENTLY new_idx ON public.foobar USING btree (bar)", + "CREATE INDEX CONCURRENTLY some_idx_with_a_very_long_name ON public.foobar USING btree (foo, bar)", + "DROP INDEX CONCURRENTLY \"old_idx\"", + "DROP INDEX CONCURRENTLY \"pgschemadiff_tmpidx_some_idx_with_a_very_EBESExQVRheYGRobHB0eHw\"", + }, + expectedHazardTypes: []diff.MigrationHazardType{ + diff.MigrationHazardTypeIndexBuild, + diff.MigrationHazardTypeIndexDropped, + }, + }, } func (suite *acceptanceTestSuite) TestIndexAcceptanceTestCases() { diff --git a/internal/migration_acceptance_tests/partitioned_index_cases_test.go b/internal/migration_acceptance_tests/partitioned_index_cases_test.go index a82c458..2b7072c 100644 --- a/internal/migration_acceptance_tests/partitioned_index_cases_test.go +++ b/internal/migration_acceptance_tests/partitioned_index_cases_test.go @@ -489,39 +489,101 @@ var partitionedIndexAcceptanceTestCases = []acceptanceTestCase{ }, }, { - name: "Change an index columns", - oldSchemaDDL: []string{ - ` + name: "Local index and columns deleted (index dropped first)", + oldSchemaDDL: []string{` CREATE TABLE foobar( - id INT, - foo VARCHAR(255), - bar INT + id INT, + bar INT, + foo VARCHAR(255) ) PARTITION BY LIST (foo); + CREATE TABLE foobar_1 PARTITION OF foobar FOR VALUES IN ('foo_1'); - CREATE TABLE foobar_2 PARTITION OF foobar FOR VALUES IN ('foo_2'); - CREATE TABLE foobar_3 PARTITION OF foobar FOR VALUES IN ('foo_3'); - CREATE INDEX some_idx ON foobar(id, foo); - `, + CREATE INDEX some_idx ON foobar(foo, id); + CREATE INDEX foobar_1_some_local_idx ON foobar_1(foo, bar, id); + `}, + newSchemaDDL: []string{` + CREATE TABLE foobar( + bar INT, + foo VARCHAR(255) + ) PARTITION BY LIST (foo); + + CREATE TABLE foobar_1 PARTITION OF foobar FOR VALUES IN ('foo_1'); + `}, + expectedHazardTypes: []diff.MigrationHazardType{ + diff.MigrationHazardTypeAcquiresAccessExclusiveLock, + diff.MigrationHazardTypeIndexDropped, + diff.MigrationHazardTypeDeletesData, }, - newSchemaDDL: []string{ - ` + ddl: []string{ + "DROP INDEX CONCURRENTLY \"foobar_1_some_local_idx\"", + "DROP INDEX \"some_idx\"", + "ALTER TABLE \"public\".\"foobar\" DROP COLUMN \"id\"", + }, + }, + { + name: "Alter index columns (index replacement and prioritized builds)", + oldSchemaDDL: []string{` CREATE TABLE foobar( - id INT, - foo VARCHAR(255), - bar INT + id INT, + bar INT, + foo VARCHAR(255) ) PARTITION BY LIST (foo); + CREATE TABLE foobar_1 PARTITION OF foobar FOR VALUES IN ('foo_1'); CREATE TABLE foobar_2 PARTITION OF foobar FOR VALUES IN ('foo_2'); - CREATE TABLE foobar_3 PARTITION OF foobar FOR VALUES IN ('foo_3'); - CREATE INDEX some_idx ON foobar(foo, bar); - `, - }, + CREATE INDEX some_idx ON foobar(foo, id); + CREATE INDEX old_idx ON foobar_1(foo, bar); + + CREATE INDEX foobar_1_some_local_idx ON foobar_1(foo, bar, id); + `}, + newSchemaDDL: []string{` + CREATE TABLE foobar( + id INT, + bar INT, + foo VARCHAR(255) + ) PARTITION BY LIST (foo); + + CREATE TABLE foobar_1 PARTITION OF foobar FOR VALUES IN ('foo_1'); + CREATE TABLE foobar_2 PARTITION OF foobar FOR VALUES IN ('foo_2'); + + ALTER TABLE foobar ADD CONSTRAINT some_idx PRIMARY KEY (foo, id); + CREATE INDEX new_idx ON foobar_1(foo, bar); + + CREATE INDEX new_foobar_1_some_local_idx ON foobar_1(foo, bar, id); + `}, expectedHazardTypes: []diff.MigrationHazardType{ - diff.MigrationHazardTypeAcquiresAccessExclusiveLock, diff.MigrationHazardTypeIndexDropped, diff.MigrationHazardTypeIndexBuild, + diff.MigrationHazardTypeAcquiresAccessExclusiveLock, + }, + ddl: []string{ + "ALTER INDEX \"some_idx\" RENAME TO \"pgschemadiff_tmpidx_some_idx_MDEyMzQ1Rje4OTo7PD0$Pw\"", + "ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\" CHECK(\"id\" IS NOT NULL) NOT VALID", + "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\"", + "ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"pgschemadiff_tmpnn_ICEiIyQlRieoKSorLC0uLw\" CHECK(\"foo\" IS NOT NULL) NOT VALID", + "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"pgschemadiff_tmpnn_ICEiIyQlRieoKSorLC0uLw\"", + "ALTER TABLE \"public\".\"foobar\" ALTER COLUMN \"foo\" SET NOT NULL", + "ALTER TABLE \"public\".\"foobar\" ALTER COLUMN \"id\" SET NOT NULL", + "ALTER TABLE \"public\".\"foobar\" DROP CONSTRAINT \"pgschemadiff_tmpnn_EBESExQVRheYGRobHB0eHw\"", + "ALTER TABLE \"public\".\"foobar\" DROP CONSTRAINT \"pgschemadiff_tmpnn_ICEiIyQlRieoKSorLC0uLw\"", + "ALTER TABLE ONLY \"public\".\"foobar\" ADD CONSTRAINT \"some_idx\" PRIMARY KEY (foo, id)", + "ALTER TABLE \"public\".\"foobar_1\" ALTER COLUMN \"id\" SET NOT NULL", + "ALTER TABLE \"public\".\"foobar_1\" ALTER COLUMN \"foo\" SET NOT NULL", + "CREATE UNIQUE INDEX CONCURRENTLY foobar_1_pkey ON public.foobar_1 USING btree (foo, id)", + "ALTER TABLE \"public\".\"foobar_1\" ADD CONSTRAINT \"foobar_1_pkey\" PRIMARY KEY USING INDEX \"foobar_1_pkey\"", + "ALTER INDEX \"some_idx\" ATTACH PARTITION \"foobar_1_pkey\"", + "CREATE INDEX CONCURRENTLY new_foobar_1_some_local_idx ON public.foobar_1 USING btree (foo, bar, id)", + "CREATE INDEX CONCURRENTLY new_idx ON public.foobar_1 USING btree (foo, bar)", + "ALTER TABLE \"public\".\"foobar_2\" ALTER COLUMN \"id\" SET NOT NULL", + "ALTER TABLE \"public\".\"foobar_2\" ALTER COLUMN \"foo\" SET NOT NULL", + "CREATE UNIQUE INDEX CONCURRENTLY foobar_2_pkey ON public.foobar_2 USING btree (foo, id)", + "ALTER TABLE \"public\".\"foobar_2\" ADD CONSTRAINT \"foobar_2_pkey\" PRIMARY KEY USING INDEX \"foobar_2_pkey\"", + "ALTER INDEX \"some_idx\" ATTACH PARTITION \"foobar_2_pkey\"", + "DROP INDEX CONCURRENTLY \"foobar_1_some_local_idx\"", + "DROP INDEX CONCURRENTLY \"old_idx\"", + "DROP INDEX \"pgschemadiff_tmpidx_some_idx_MDEyMzQ1Rje4OTo7PD0$Pw\"", }, }, { diff --git a/internal/pgidentifier/identifier.go b/internal/pgidentifier/identifier.go index d50d47c..559d164 100644 --- a/internal/pgidentifier/identifier.go +++ b/internal/pgidentifier/identifier.go @@ -1,6 +1,13 @@ package pgidentifier -import "regexp" +import ( + "encoding/base64" + "fmt" + "regexp" + "strings" + + "github.com/google/uuid" +) var ( // SimpleIdentifierRegex matches identifiers in Postgres that require no quotes @@ -10,3 +17,33 @@ var ( func IsSimpleIdentifier(val string) bool { return SimpleIdentifierRegex.MatchString(val) } + +const encodePostgresIdentifier = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789$_" + +var postgresIdentifierEncoding = base64.NewEncoding(encodePostgresIdentifier).WithPadding(base64.NoPadding) + +// RandomUUID builds a RandomUUID to be used in Postgres identifiers. This RandomUUID cannot be used directly as an identifier +// and must be prefixed with a letter +func RandomUUID() (string, error) { + uuid, err := uuid.NewRandom() + if err != nil { + return "", fmt.Errorf("generating RandomUUID: %w", err) + } + + // Encode in base64 to make the RandomUUID smaller + binary, err := uuid.MarshalBinary() + if err != nil { + return "", fmt.Errorf("marshaling RandomUUID: %w", err) + } + + var sb strings.Builder + encoder := base64.NewEncoder(postgresIdentifierEncoding, &sb) + if _, err := encoder.Write(binary); err != nil { + return "", fmt.Errorf("encoding RandomUUID: %w", err) + } + if err := encoder.Close(); err != nil { + return "", fmt.Errorf("closing encoder: %w", err) + } + + return sb.String(), nil +} diff --git a/pkg/diff/schema_migration_plan_test.go b/pkg/diff/schema_migration_plan_test.go index 2690f11..3135cbc 100644 --- a/pkg/diff/schema_migration_plan_test.go +++ b/pkg/diff/schema_migration_plan_test.go @@ -36,173 +36,8 @@ var ( EscapedName: `"default"`, SchemaName: "pg_catalog", } - cCollation = schema.SchemaQualifiedName{ - EscapedName: `"C"`, - SchemaName: "pg_catalog", - } schemaMigrationPlanTestCases = []schemaMigrationPlanTestCase{ - { - name: "Index replacement", - oldSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, - {Name: "bar", Type: "timestamp without time zone", IsNullable: true, Default: "CURRENT_TIMESTAMP"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - }, - }, - Indexes: []schema.Index{ - { - TableName: "foobar", - Name: "foo_idx", Columns: []string{"foo"}, - GetIndexDefStmt: "CREATE INDEX foo_idx ON public.foobar USING btree (foo)", - }, - { - TableName: "foobar", - Name: "replaced_with_same_name_idx", Columns: []string{"bar"}, - GetIndexDefStmt: "CREATE INDEX replaced_with_same_name_idx ON ONLY public.foobar USING btree (bar)", - }, - }, - }, - newSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, - {Name: "bar", Type: "timestamp without time zone", IsNullable: true, Default: "CURRENT_TIMESTAMP"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - }, - }, - Indexes: []schema.Index{ - { - TableName: "foobar", - Name: "new_foo_idx", Columns: []string{"foo"}, - GetIndexDefStmt: "CREATE INDEX new_foo_idx ON public.foobar USING btree (foo)", - }, - { - TableName: "foobar", - Name: "replaced_with_same_name_idx", Columns: []string{"bar", "foo"}, - GetIndexDefStmt: "CREATE INDEX replaced_with_same_name_idx ON ONLY public.foobar USING btree (bar)", - }, - }, - }, - expectedStatements: []Statement{ - { - DDL: "ALTER INDEX \"replaced_with_same_name_idx\" RENAME TO \"replaced_with_same_name_id_00010203-0405-4607-8809-0a0b0c0d0e0f\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: nil, - }, - { - DDL: "CREATE INDEX CONCURRENTLY new_foo_idx ON public.foobar USING btree (foo)", - Timeout: statementTimeoutConcurrentIndexBuild, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{buildIndexBuildHazard()}, - }, - { - DDL: "CREATE INDEX CONCURRENTLY replaced_with_same_name_idx ON ONLY public.foobar USING btree (bar)", - Timeout: statementTimeoutConcurrentIndexBuild, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{buildIndexBuildHazard()}, - }, - { - DDL: "DROP INDEX CONCURRENTLY \"foo_idx\"", - Timeout: statementTimeoutConcurrentIndexDrop, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{ - {Type: "INDEX_DROPPED", Message: "Dropping this index means queries that use this index might perform worse because they will no longer will be able to leverage it."}, - }, - }, - { - DDL: "DROP INDEX CONCURRENTLY \"replaced_with_same_name_id_00010203-0405-4607-8809-0a0b0c0d0e0f\"", - Timeout: statementTimeoutConcurrentIndexDrop, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{ - {Type: "INDEX_DROPPED", Message: "Dropping this index means queries that use this index might perform worse because they will no longer will be able to leverage it."}, - }, - }, - }, - }, - { - name: "Index dropped concurrently before columns dropped", // If this is not true, the columns will automatically drop the index - oldSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, - {Name: "bar", Type: "timestamp without time zone", IsNullable: true, Default: "CURRENT_TIMESTAMP"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - }, - }, - Indexes: []schema.Index{ - { - TableName: "foobar", - Name: "foobar_pkey", Columns: []string{"id"}, IsUnique: true, - Constraint: &schema.IndexConstraint{Type: schema.PkIndexConstraintType, EscapedConstraintName: "\"foobar_pkey\"", ConstraintDef: "PRIMARY KEY (id)", IsLocal: true}, - GetIndexDefStmt: "CREATE UNIQUE INDEX foobar_pkey ON public.foobar USING btree (id)", - }, - { - TableName: "foobar", - Name: "some_idx", Columns: []string{"foo, bar"}, - GetIndexDefStmt: "CREATE INDEX some_idx ON public.foobar USING btree (foo, bar)", - }, - }, - }, - newSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - }, - }, - Indexes: []schema.Index{ - { - TableName: "foobar", - Name: "foobar_pkey", Columns: []string{"id"}, IsUnique: true, - Constraint: &schema.IndexConstraint{Type: schema.PkIndexConstraintType, EscapedConstraintName: "\"foobar_pkey\"", ConstraintDef: "PRIMARY KEY (id)", IsLocal: true}, - GetIndexDefStmt: "CREATE UNIQUE INDEX foobar_pkey ON public.foobar USING btree (id)", - }, - }, - }, - expectedStatements: []Statement{ - { - DDL: "DROP INDEX CONCURRENTLY \"some_idx\"", - Timeout: statementTimeoutConcurrentIndexDrop, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{buildIndexDroppedQueryPerfHazard()}, - }, - { - DDL: "ALTER TABLE \"public\".\"foobar\" DROP COLUMN \"bar\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{buildColumnDataDeletionHazard()}, - }, - { - DDL: "ALTER TABLE \"public\".\"foobar\" DROP COLUMN \"foo\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{buildColumnDataDeletionHazard()}, - }, - }, - }, { name: "Invalid index re-created", oldSchema: schema.Schema{ @@ -220,405 +55,10 @@ var ( }, Indexes: []schema.Index{ { - TableName: "foobar", - Name: "some_idx", Columns: []string{"foo", "bar"}, - GetIndexDefStmt: "CREATE INDEX some_idx ON public.foobar USING btree (foo, bar)", - IsUnique: true, IsInvalid: true, - }, - }, - }, - newSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, - {Name: "bar", Type: "timestamp without time zone", IsNullable: true, Default: "CURRENT_TIMESTAMP"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - }, - }, - Indexes: []schema.Index{ - - { - TableName: "foobar", - Name: "some_idx", Columns: []string{"foo", "bar"}, - GetIndexDefStmt: "CREATE INDEX some_idx ON public.foobar USING btree (foo, bar)", - IsUnique: true, - }, - }, - }, - expectedStatements: []Statement{ - { - DDL: "ALTER INDEX \"some_idx\" RENAME TO \"some_idx_10111213-1415-4617-9819-1a1b1c1d1e1f\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - }, - { - DDL: "CREATE INDEX CONCURRENTLY some_idx ON public.foobar USING btree (foo, bar)", - Timeout: statementTimeoutConcurrentIndexBuild, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{buildIndexBuildHazard()}, - }, - { - DDL: "DROP INDEX CONCURRENTLY \"some_idx_10111213-1415-4617-9819-1a1b1c1d1e1f\"", - Timeout: statementTimeoutConcurrentIndexDrop, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{buildIndexDroppedQueryPerfHazard()}, - }, - }, - }, - { - name: "Index replacement on partitioned table (replaces index is now also a primary key)", - oldSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, - {Name: "bar", Type: "timestamp without time zone", IsNullable: true, Default: "CURRENT_TIMESTAMP"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - PartitionKeyDef: "LIST(foo)", - }, - { - ParentTableName: "foobar", - Name: "foobar_1", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, - {Name: "bar", Type: "timestamp without time zone", IsNullable: true, Default: "CURRENT_TIMESTAMP"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - ForValues: "FOR VALUES IN ('some_val')", - }, - { - ParentTableName: "foobar", - Name: "foobar_2", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, - {Name: "bar", Type: "timestamp without time zone", IsNullable: true, Default: "CURRENT_TIMESTAMP"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - ForValues: "FOR VALUES IN ('some_other_val')", - }, - }, - Indexes: []schema.Index{ - // foobar indexes - { - TableName: "foobar", - Name: "some_idx", Columns: []string{"foo", "bar"}, - GetIndexDefStmt: "CREATE INDEX some_idx ON ONLY public.foobar USING btree (foo, bar)", - }, - { - TableName: "foobar", - Name: "replaced_with_same_name_idx", Columns: []string{"bar"}, - GetIndexDefStmt: "CREATE INDEX replaced_with_same_name_idx ON ONLY public.foobar USING btree (bar)", - }, - // foobar_1 indexes - { - TableName: "foobar_1", - Name: "foobar_1_some_idx", Columns: []string{"foo", "bar"}, ParentIdxName: "some_idx", - GetIndexDefStmt: "CREATE INDEX foobar_1_some_idx ON public.foobar_1 USING btree (foo, bar)", - }, - { - TableName: "foobar_1", - Name: "foobar_1_replaced_with_same_name_idx", Columns: []string{"bar"}, ParentIdxName: "replaced_with_same_name_idx", - GetIndexDefStmt: "CREATE INDEX foobar_1_replaced_with_same_name_idx ON ONLY public.foobar USING btree (bar)", - }, - { - TableName: "foobar_1", - Name: "foobar_1_some_local_idx", Columns: []string{"foo", "bar", "id"}, - GetIndexDefStmt: "CREATE INDEX foobar_1_some_local_idx ON public.foobar_1 USING btree (foo, bar, id)", - }, - // foobar_2 indexes - { - TableName: "foobar_2", - Name: "foobar_2_some_idx", Columns: []string{"foo", "bar"}, ParentIdxName: "some_idx", - GetIndexDefStmt: "CREATE INDEX foobar_2_some_idx ON public.foobar_2 USING btree (foo, bar)", - }, - { - TableName: "foobar_2", - Name: "foobar_2_replaced_with_same_name_idx", Columns: []string{"bar"}, ParentIdxName: "replaced_with_same_name_idx", - GetIndexDefStmt: "CREATE INDEX foobar_2_replaced_with_same_name_idx ON ONLY public.foobar USING btree (bar)", - }, - }, - }, - newSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, - {Name: "bar", Type: "timestamp without time zone", IsNullable: true, Default: "CURRENT_TIMESTAMP"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - PartitionKeyDef: "LIST(foo)", - }, - { - ParentTableName: "foobar", - Name: "foobar_1", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, - {Name: "bar", Type: "timestamp without time zone", IsNullable: true, Default: "CURRENT_TIMESTAMP"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - ForValues: "FOR VALUES IN ('some_val')", - }, - { - ParentTableName: "foobar", - Name: "foobar_2", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, - {Name: "bar", Type: "timestamp without time zone", IsNullable: true, Default: "CURRENT_TIMESTAMP"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - ForValues: "FOR VALUES IN ('some_other_val')", - }, - }, - Indexes: []schema.Index{ - // foobar indexes - { - TableName: "foobar", - Name: "new_some_idx", Columns: []string{"foo", "bar"}, - GetIndexDefStmt: "CREATE INDEX new_some_idx ON ONLY public.foobar USING btree (foo, bar)", - }, - { - TableName: "foobar", - Name: "replaced_with_same_name_idx", Columns: []string{"bar", "foo"}, IsUnique: true, - Constraint: &schema.IndexConstraint{Type: schema.PkIndexConstraintType, EscapedConstraintName: "\"replaced_with_same_name_idx\"", ConstraintDef: "PRIMARY KEY (foo, id)", IsLocal: true}, - GetIndexDefStmt: "CREATE UNIQUE INDEX replaced_with_same_name_idx ON ONLY public.foobar USING btree (bar, foo)", - }, - // foobar_1 indexes - { - TableName: "foobar_1", - Name: "new_foobar_1_some_idx", Columns: []string{"foo", "bar"}, ParentIdxName: "new_some_idx", - GetIndexDefStmt: "CREATE INDEX new_foobar_1_some_idx ON public.foobar_1 USING btree (foo, bar)", - }, - { - TableName: "foobar_1", - Name: "foobar_1_replaced_with_same_name_idx", Columns: []string{"bar", "foo"}, ParentIdxName: "replaced_with_same_name_idx", IsUnique: true, - Constraint: &schema.IndexConstraint{Type: schema.PkIndexConstraintType, EscapedConstraintName: "\"foobar_1_replaced_with_same_name_idx\"", ConstraintDef: "PRIMARY KEY (foo, id)", IsLocal: true}, - GetIndexDefStmt: "CREATE UNIQUE INDEX foobar_1_replaced_with_same_name_idx ON public.foobar USING btree (bar, foo)", - }, - { - TableName: "foobar_1", - Name: "new_foobar_1_some_local_idx", Columns: []string{"foo", "bar", "id"}, - GetIndexDefStmt: "CREATE INDEX new_foobar_1_some_local_idx ON public.foobar_1 USING btree (foo, bar, id)", - }, - // foobar_2 indexes - { - TableName: "foobar_2", - Name: "new_foobar_2_some_idx", Columns: []string{"foo", "bar"}, ParentIdxName: "new_some_idx", - GetIndexDefStmt: "CREATE INDEX new_foobar_2_some_idx ON public.foobar_2 USING btree (foo, bar)", - }, - { - TableName: "foobar_2", - Name: "foobar_2_replaced_with_same_name_idx", Columns: []string{"bar", "foo"}, ParentIdxName: "replaced_with_same_name_idx", IsUnique: true, - Constraint: &schema.IndexConstraint{Type: schema.PkIndexConstraintType, EscapedConstraintName: "\"foobar_2_replaced_with_same_name_idx\"", ConstraintDef: "PRIMARY KEY (foo, id)", IsLocal: true}, - GetIndexDefStmt: "CREATE UNIQUE INDEX foobar_2_replaced_with_same_name_idx ON public.foobar_2 USING btree (bar, foo)", - }, - }, - }, - expectedStatements: []Statement{ - { - DDL: "ALTER INDEX \"foobar_1_replaced_with_same_name_idx\" RENAME TO \"foobar_1_replaced_with_sam_30313233-3435-4637-b839-3a3b3c3d3e3f\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - }, - { - DDL: "ALTER INDEX \"foobar_2_replaced_with_same_name_idx\" RENAME TO \"foobar_2_replaced_with_sam_40414243-4445-4647-8849-4a4b4c4d4e4f\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - }, - { - DDL: "ALTER INDEX \"replaced_with_same_name_idx\" RENAME TO \"replaced_with_same_name_id_20212223-2425-4627-a829-2a2b2c2d2e2f\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - }, - { - DDL: "CREATE INDEX new_some_idx ON ONLY public.foobar USING btree (foo, bar)", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - }, - { - DDL: "ALTER TABLE ONLY \"public\".\"foobar\" ADD CONSTRAINT \"replaced_with_same_name_idx\" PRIMARY KEY (foo, id)", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - }, - { - DDL: "CREATE UNIQUE INDEX CONCURRENTLY foobar_1_replaced_with_same_name_idx ON public.foobar USING btree (bar, foo)", - Timeout: statementTimeoutConcurrentIndexDrop, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{ - buildIndexBuildHazard(), - }, - }, - { - DDL: "ALTER TABLE \"public\".\"foobar_1\" ADD CONSTRAINT \"foobar_1_replaced_with_same_name_idx\" PRIMARY KEY USING INDEX \"foobar_1_replaced_with_same_name_idx\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - }, - { - DDL: "ALTER INDEX \"replaced_with_same_name_idx\" ATTACH PARTITION \"foobar_1_replaced_with_same_name_idx\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: nil, - }, - { - DDL: "CREATE INDEX CONCURRENTLY new_foobar_1_some_idx ON public.foobar_1 USING btree (foo, bar)", - Timeout: statementTimeoutConcurrentIndexBuild, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{ - buildIndexBuildHazard(), - }, - }, - { - DDL: "ALTER INDEX \"new_some_idx\" ATTACH PARTITION \"new_foobar_1_some_idx\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: nil, - }, - { - DDL: "CREATE INDEX CONCURRENTLY new_foobar_1_some_local_idx ON public.foobar_1 USING btree (foo, bar, id)", - Timeout: statementTimeoutConcurrentIndexBuild, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{ - buildIndexBuildHazard(), - }, - }, - { - DDL: "CREATE UNIQUE INDEX CONCURRENTLY foobar_2_replaced_with_same_name_idx ON public.foobar_2 USING btree (bar, foo)", - Timeout: statementTimeoutConcurrentIndexBuild, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{ - buildIndexBuildHazard(), - }, - }, - { - DDL: "ALTER TABLE \"public\".\"foobar_2\" ADD CONSTRAINT \"foobar_2_replaced_with_same_name_idx\" PRIMARY KEY USING INDEX \"foobar_2_replaced_with_same_name_idx\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: nil, - }, - { - DDL: "ALTER INDEX \"replaced_with_same_name_idx\" ATTACH PARTITION \"foobar_2_replaced_with_same_name_idx\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: nil, - }, - { - DDL: "CREATE INDEX CONCURRENTLY new_foobar_2_some_idx ON public.foobar_2 USING btree (foo, bar)", - Timeout: statementTimeoutConcurrentIndexBuild, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{ - buildIndexBuildHazard(), - }, - }, - { - DDL: "ALTER INDEX \"new_some_idx\" ATTACH PARTITION \"new_foobar_2_some_idx\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: nil, - }, - { - DDL: "DROP INDEX CONCURRENTLY \"foobar_1_some_local_idx\"", - Timeout: statementTimeoutConcurrentIndexDrop, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{ - buildIndexDroppedQueryPerfHazard(), - }, - }, - { - DDL: "DROP INDEX \"replaced_with_same_name_id_20212223-2425-4627-a829-2a2b2c2d2e2f\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{ - buildIndexDroppedAcquiresLockHazard(), - buildIndexDroppedQueryPerfHazard(), - }, - }, - { - DDL: "DROP INDEX \"some_idx\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{ - buildIndexDroppedAcquiresLockHazard(), - buildIndexDroppedQueryPerfHazard(), - }, - }, - }, - }, - { - name: "Local Index dropped concurrently before columns dropped; partitioned index just dropped", - oldSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, - {Name: "bar", Type: "timestamp without time zone", IsNullable: true, Default: "CURRENT_TIMESTAMP"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - PartitionKeyDef: "LIST(foo)", - }, - { - ParentTableName: "foobar", - Name: "foobar_1", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, - {Name: "bar", Type: "timestamp without time zone", IsNullable: true, Default: "CURRENT_TIMESTAMP"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - ForValues: "FOR VALUES IN ('some_val')", - }, - }, - Indexes: []schema.Index{ - // foobar indexes - { - TableName: "foobar", - Name: "foobar_pkey", Columns: []string{"foo", "id"}, IsUnique: true, - Constraint: &schema.IndexConstraint{Type: schema.PkIndexConstraintType, EscapedConstraintName: "\"foobar_pkey\"", ConstraintDef: "PRIMARY KEY (foo, id)", IsLocal: true}, - GetIndexDefStmt: "CREATE UNIQUE INDEX foobar_pkey ON ONLY public.foobar USING btree (foo, id)", - }, - { - TableName: "foobar", - Name: "some_idx", Columns: []string{"foo, bar"}, - GetIndexDefStmt: "CREATE INDEX some_idx ON ONLY public.foobar USING btree (foo, bar)", - }, - // foobar_1 indexes - { - TableName: "foobar_1", - Name: "foobar_1_pkey", Columns: []string{"foo", "id"}, IsUnique: true, ParentIdxName: "foobar_pkey", - Constraint: &schema.IndexConstraint{Type: schema.PkIndexConstraintType, EscapedConstraintName: "\"foobar_1_pkey\"", ConstraintDef: "PRIMARY KEY (foo, id)", IsLocal: true}, - GetIndexDefStmt: "CREATE UNIQUE INDEX foobar_1_pkey ON public.foobar_1 USING btree (foo, id)", - }, - { - TableName: "foobar_1", - Name: "foobar_1_some_idx", Columns: []string{"foo", "bar"}, ParentIdxName: "some_idx", - GetIndexDefStmt: "CREATE INDEX foobar_1_some_idx ON public.foobar_1 USING btree (foo, bar)", - }, - { - TableName: "foobar_1", - Name: "foobar_1_some_local_idx", Columns: []string{"foo", "bar", "id"}, - GetIndexDefStmt: "CREATE INDEX foobar_1_some_local_idx ON public.foobar_1 USING btree (foo, bar, id)", + TableName: "foobar", + Name: "some_idx", Columns: []string{"foo", "bar"}, + GetIndexDefStmt: "CREATE INDEX some_idx ON public.foobar USING btree (foo, bar)", + IsUnique: true, IsInvalid: true, }, }, }, @@ -629,65 +69,39 @@ var ( Columns: []schema.Column{ {Name: "id", Type: "integer"}, {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, + {Name: "bar", Type: "timestamp without time zone", IsNullable: true, Default: "CURRENT_TIMESTAMP"}, }, CheckConstraints: nil, ReplicaIdentity: schema.ReplicaIdentityDefault, - PartitionKeyDef: "LIST(foo)", - }, - { - ParentTableName: "foobar", - Name: "foobar_1", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - {Name: "foo", Type: "character varying(255)", Default: "''::character varying", Collation: defaultCollation}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - ForValues: "FOR VALUES IN ('some_val')", }, }, Indexes: []schema.Index{ - // foobar indexes + { TableName: "foobar", - Name: "foobar_pkey", Columns: []string{"foo", "id"}, IsUnique: true, - Constraint: &schema.IndexConstraint{Type: schema.PkIndexConstraintType, EscapedConstraintName: "\"foobar_pkey\"", ConstraintDef: "PRIMARY KEY (foo, id)", IsLocal: true}, - GetIndexDefStmt: "CREATE UNIQUE INDEX foobar_pkey ON ONLY public.foobar USING btree (foo, id)", - }, - // foobar_1 indexes - { - TableName: "foobar_1", - Name: "foobar_1_pkey", Columns: []string{"foo", "id"}, IsUnique: true, ParentIdxName: "foobar_pkey", - Constraint: &schema.IndexConstraint{Type: schema.PkIndexConstraintType, EscapedConstraintName: "\"foobar_1_pkey\"", ConstraintDef: "PRIMARY KEY (foo, id)", IsLocal: true}, - GetIndexDefStmt: "CREATE UNIQUE INDEX foobar_1_pkey ON public.foobar_1 USING btree (foo, id)", + Name: "some_idx", Columns: []string{"foo", "bar"}, + GetIndexDefStmt: "CREATE INDEX some_idx ON public.foobar USING btree (foo, bar)", + IsUnique: true, }, }, }, expectedStatements: []Statement{ { - DDL: "DROP INDEX CONCURRENTLY \"foobar_1_some_local_idx\"", - Timeout: statementTimeoutConcurrentIndexDrop, + DDL: "ALTER INDEX \"some_idx\" RENAME TO \"pgschemadiff_tmpidx_some_idx_AAECAwQFRgeICQoLDA0ODw\"", + Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{ - buildIndexDroppedQueryPerfHazard(), - }, }, { - DDL: "DROP INDEX \"some_idx\"", - Timeout: statementTimeoutDefault, + DDL: "CREATE INDEX CONCURRENTLY some_idx ON public.foobar USING btree (foo, bar)", + Timeout: statementTimeoutConcurrentIndexBuild, LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{ - buildIndexDroppedAcquiresLockHazard(), - buildIndexDroppedQueryPerfHazard(), - }, + Hazards: []MigrationHazard{buildIndexBuildHazard()}, }, { - DDL: "ALTER TABLE \"public\".\"foobar\" DROP COLUMN \"bar\"", - Timeout: statementTimeoutDefault, + DDL: "DROP INDEX CONCURRENTLY \"pgschemadiff_tmpidx_some_idx_AAECAwQFRgeICQoLDA0ODw\"", + Timeout: statementTimeoutConcurrentIndexDrop, LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{ - buildColumnDataDeletionHazard(), - }, + Hazards: []MigrationHazard{buildIndexDroppedQueryPerfHazard()}, }, }, }, @@ -779,7 +193,7 @@ var ( }, expectedStatements: []Statement{ { - DDL: "ALTER INDEX \"foobar_1_some_idx\" RENAME TO \"foobar_1_some_idx_50515253-5455-4657-9859-5a5b5c5d5e5f\"", + DDL: "ALTER INDEX \"foobar_1_some_idx\" RENAME TO \"pgschemadiff_tmpidx_foobar_1_some_idx_EBESExQVRheYGRobHB0eHw\"", Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, }, @@ -797,7 +211,7 @@ var ( LockTimeout: lockTimeoutDefault, }, { - DDL: "DROP INDEX CONCURRENTLY \"foobar_1_some_idx_50515253-5455-4657-9859-5a5b5c5d5e5f\"", + DDL: "DROP INDEX CONCURRENTLY \"pgschemadiff_tmpidx_foobar_1_some_idx_EBESExQVRheYGRobHB0eHw\"", Timeout: statementTimeoutConcurrentIndexDrop, LockTimeout: lockTimeoutDefault, Hazards: []MigrationHazard{ @@ -837,309 +251,6 @@ var ( expectedStatements: nil, expectedDiffErrIs: errDuplicateIdentifier, }, - { - name: "Online check constraint build", - oldSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - }, - ReplicaIdentity: schema.ReplicaIdentityDefault, - }, - }, - }, - newSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - }, - CheckConstraints: []schema.CheckConstraint{ - {Name: "id_check", Expression: "(id > 0)", IsInheritable: true, IsValid: true}, - }, - ReplicaIdentity: schema.ReplicaIdentityDefault, - }, - }, - }, - expectedStatements: []Statement{ - { - DDL: "ALTER TABLE \"public\".\"foobar\" ADD CONSTRAINT \"id_check\" CHECK((id > 0)) NOT VALID", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: nil, - }, - { - DDL: "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"id_check\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: nil, - }, - }, - }, - { - name: "Invalid check constraint made valid", - oldSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - }, - CheckConstraints: []schema.CheckConstraint{ - {Name: "id_check", Expression: "(id > 0)", IsInheritable: true}, - }, - ReplicaIdentity: schema.ReplicaIdentityDefault, - }, - }, - }, - newSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "id", Type: "integer"}, - }, - CheckConstraints: []schema.CheckConstraint{ - {Name: "id_check", Expression: "(id > 0)", IsInheritable: true, IsValid: true}, - }, - ReplicaIdentity: schema.ReplicaIdentityDefault, - }, - }, - }, - expectedStatements: []Statement{ - { - DDL: "ALTER TABLE \"public\".\"foobar\" VALIDATE CONSTRAINT \"id_check\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: nil, - }, - }, - }, - { - name: "Invalid foreign key constraint constraint made valid", - oldSchema: schema.Schema{ - Extensions: []schema.Extension{ - { - SchemaQualifiedName: schema.SchemaQualifiedName{ - EscapedName: schema.EscapeIdentifier("pg_trgm"), - SchemaName: "public", - }, - Version: "1.6", - }, - }, - Tables: []schema.Table{ - { - Name: "foo", - Columns: []schema.Column{ - {Name: "id", Type: "integer", Size: 4}, - }, - }, - { - Name: "foo_fk", - Columns: []schema.Column{{Name: "id", Type: "integer", Size: 4}}, - }, - }, - ForeignKeyConstraints: []schema.ForeignKeyConstraint{ - { - EscapedName: "\"foo_fk_fk\"", - OwningTable: schema.SchemaQualifiedName{ - SchemaName: "public", - EscapedName: "\"foo_fk\"", - }, - OwningTableUnescapedName: "foo_fk", - ForeignTable: schema.SchemaQualifiedName{ - SchemaName: "public", - EscapedName: "\"foo\"", - }, - ForeignTableUnescapedName: "foo", - ConstraintDef: "FOREIGN KEY (id) REFERENCES foo(id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID", - IsValid: false, - }, - }, - Indexes: []schema.Index{ - { - TableName: "foo", - Name: "foo_pkey", - Columns: []string{"id"}, - IsUnique: true, - Constraint: &schema.IndexConstraint{Type: schema.PkIndexConstraintType, EscapedConstraintName: "\"foobar_pkey\"", ConstraintDef: "PRIMARY KEY (id)", IsLocal: true}, - GetIndexDefStmt: "CREATE UNIQUE INDEX foo_pkey ON public.foo USING btree (id)", - }, - }, - }, - newSchema: schema.Schema{ - Extensions: []schema.Extension{ - { - SchemaQualifiedName: schema.SchemaQualifiedName{ - EscapedName: schema.EscapeIdentifier("pg_trgm"), - SchemaName: "public", - }, - Version: "1.6", - }, - }, - Tables: []schema.Table{ - { - Name: "foo", - Columns: []schema.Column{ - {Name: "id", Type: "integer", Size: 4}, - }, - }, - { - Name: "foo_fk", - Columns: []schema.Column{{Name: "id", Type: "integer", Size: 4}}, - }, - }, - ForeignKeyConstraints: []schema.ForeignKeyConstraint{ - { - EscapedName: "\"foo_fk_fk\"", - OwningTable: schema.SchemaQualifiedName{ - SchemaName: "public", - EscapedName: "\"foo_fk\"", - }, - OwningTableUnescapedName: "foo_fk", - ForeignTable: schema.SchemaQualifiedName{ - SchemaName: "public", - EscapedName: "\"foo\"", - }, - ForeignTableUnescapedName: "foo", - ConstraintDef: "FOREIGN KEY (id) REFERENCES foo(id) ON UPDATE CASCADE ON DELETE CASCADE", - IsValid: true, - }, - }, - Indexes: []schema.Index{ - { - TableName: "foo", - Name: "foo_pkey", - Columns: []string{"id"}, - IsUnique: true, - Constraint: &schema.IndexConstraint{Type: schema.PkIndexConstraintType, EscapedConstraintName: "\"foobar_pkey\"", ConstraintDef: "PRIMARY KEY (id)", IsLocal: true}, - GetIndexDefStmt: "CREATE UNIQUE INDEX foo_pkey ON public.foo USING btree (id)", - }, - }, - }, - expectedStatements: []Statement{ - { - DDL: "ALTER TABLE \"public\".\"foo_fk\" VALIDATE CONSTRAINT \"foo_fk_fk\"", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: nil, - }, - }, - }, - { - name: "BIGINT to TIMESTAMP type conversion", - oldSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "baz", Type: "bigint"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - }, - }, - Indexes: nil, - }, - newSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "baz", Type: "timestamp without time zone", Default: "current_timestamp"}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - }, - }, - Indexes: nil, - }, - expectedStatements: []Statement{ - { - DDL: "ALTER TABLE \"public\".\"foobar\" ALTER COLUMN \"baz\" SET DATA TYPE timestamp without time zone using to_timestamp(\"baz\" / 1000)", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{{ - Type: MigrationHazardTypeAcquiresAccessExclusiveLock, - Message: "This will completely lock the table while the data is being " + - "re-written for a duration of time that scales with the size of your " + - "data. The values previously stored as BIGINT will be translated into a " + - "TIMESTAMP value via the PostgreSQL to_timestamp() function. This " + - "translation will assume that the values stored in BIGINT represent a " + - "millisecond epoch value.", - }}, - }, - { - DDL: "ANALYZE \"foobar\" (\"baz\")", - Timeout: statementTimeoutAnalyzeColumn, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{buildAnalyzeColumnMigrationHazard()}, - }, - { - DDL: "ALTER TABLE \"public\".\"foobar\" ALTER COLUMN \"baz\" SET DEFAULT current_timestamp", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - }, - }, - }, - { - name: "Collation migration and Type Migration", - oldSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "migrate_to_c_coll", Type: "text", Collation: defaultCollation}, - {Name: "migrate_type", Type: "text", Collation: defaultCollation}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - }, - }, - }, - newSchema: schema.Schema{ - Tables: []schema.Table{ - { - Name: "foobar", - Columns: []schema.Column{ - {Name: "migrate_to_c_coll", Type: "text", Collation: cCollation}, - {Name: "migrate_type", Type: "character varying(255)", Collation: defaultCollation}, - }, - CheckConstraints: nil, - ReplicaIdentity: schema.ReplicaIdentityDefault, - }, - }, - }, - expectedStatements: []Statement{ - { - DDL: "ALTER TABLE \"public\".\"foobar\" ALTER COLUMN \"migrate_to_c_coll\" SET DATA TYPE text COLLATE \"pg_catalog\".\"C\" using \"migrate_to_c_coll\"::text", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{buildColumnTypeChangeHazard()}, - }, - { - DDL: "ANALYZE \"foobar\" (\"migrate_to_c_coll\")", - Timeout: statementTimeoutAnalyzeColumn, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{buildAnalyzeColumnMigrationHazard()}, - }, - { - DDL: "ALTER TABLE \"public\".\"foobar\" ALTER COLUMN \"migrate_type\" SET DATA TYPE character varying(255) COLLATE \"pg_catalog\".\"default\" using \"migrate_type\"::character varying(255)", - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{buildColumnTypeChangeHazard()}, - }, - { - DDL: "ANALYZE \"foobar\" (\"migrate_type\")", - Timeout: statementTimeoutAnalyzeColumn, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{buildAnalyzeColumnMigrationHazard()}, - }, - }, - }, { name: "Handle infinite index loop without panicking", oldSchema: schema.Schema{ @@ -1263,32 +374,6 @@ func TestSchemaMigrationPlanTest(t *testing.T) { } } -func buildColumnDataDeletionHazard() MigrationHazard { - return MigrationHazard{ - Type: MigrationHazardTypeDeletesData, - Message: "Deletes all values in the column", - } -} - -func buildColumnTypeChangeHazard() MigrationHazard { - return MigrationHazard{ - Type: MigrationHazardTypeAcquiresAccessExclusiveLock, - Message: "This will completely lock the table while the data is being re-written. The duration of this " + - "conversion depends on if the type conversion is trivial or not. A non-trivial conversion will require a " + - "table rewrite. A trivial conversion is one where the binary values are coercible and the column contents " + - "are not changing.", - } -} - -func buildAnalyzeColumnMigrationHazard() MigrationHazard { - return MigrationHazard{ - Type: MigrationHazardTypeImpactsDatabasePerformance, - Message: "Running analyze will read rows from the table, putting increased load " + - "on the database and consuming database resources. It won't prevent reads/writes to " + - "the table, but it could affect performance when executing queries.", - } -} - func buildIndexBuildHazard() MigrationHazard { return MigrationHazard{ Type: MigrationHazardTypeIndexBuild, @@ -1305,10 +390,3 @@ func buildIndexDroppedQueryPerfHazard() MigrationHazard { "they will no longer will be able to leverage it.", } } - -func buildIndexDroppedAcquiresLockHazard() MigrationHazard { - return MigrationHazard{ - Type: MigrationHazardTypeAcquiresAccessExclusiveLock, - Message: "Index drops will lock out all accesses to the table. They should be fast", - } -} diff --git a/pkg/diff/sql_generator.go b/pkg/diff/sql_generator.go index 3eceaf0..3b9a9d6 100644 --- a/pkg/diff/sql_generator.go +++ b/pkg/diff/sql_generator.go @@ -8,7 +8,7 @@ import ( "time" "github.com/google/go-cmp/cmp" - "github.com/google/uuid" + "github.com/stripe/pg-schema-diff/internal/pgidentifier" "github.com/stripe/pg-schema-diff/internal/schema" ) @@ -924,12 +924,12 @@ func isValidNotNullCC(cc schema.CheckConstraint) bool { } func buildTempNotNullConstraint(colDiff columnDiff) (schema.CheckConstraint, error) { - uuid, err := uuid.NewRandom() + uuid, err := pgidentifier.RandomUUID() if err != nil { return schema.CheckConstraint{}, fmt.Errorf("generating uuid: %w", err) } return schema.CheckConstraint{ - Name: fmt.Sprintf("%snn_%s", tmpObjNamePrefix, uuid.String()), + Name: fmt.Sprintf("%snn_%s", tmpObjNamePrefix, uuid), KeyColumns: []string{colDiff.new.Name}, Expression: fmt.Sprintf("%s IS NOT NULL", schema.EscapeIdentifier(colDiff.new.Name)), IsValid: true, @@ -1167,18 +1167,21 @@ func (rsg *renameConflictingIndexSQLVertexGenerator) Add(index schema.Index) ([] } func (rsg *renameConflictingIndexSQLVertexGenerator) generateNonConflictingName(index schema.Index) (string, error) { - uuid, err := uuid.NewRandom() + uuid, err := pgidentifier.RandomUUID() if err != nil { - return "", fmt.Errorf("generating UUID: %w", err) + return "", fmt.Errorf("generating RandomUUID: %w", err) } - newNameSuffix := fmt.Sprintf("_%s", uuid.String()) + prefix := fmt.Sprintf("%sidx_", tmpObjNamePrefix) + suffix := fmt.Sprintf("_%s", uuid) + prefixAndSuffixSize := len(prefix) + len(suffix) + idxNameTruncationIdx := len(index.Name) - if len(index.Name) > maxPostgresIdentifierSize-len(newNameSuffix) { - idxNameTruncationIdx = maxPostgresIdentifierSize - len(newNameSuffix) + if len(index.Name) > maxPostgresIdentifierSize-prefixAndSuffixSize { + idxNameTruncationIdx = maxPostgresIdentifierSize - prefixAndSuffixSize } - return index.Name[:idxNameTruncationIdx] + newNameSuffix, nil + return fmt.Sprintf("%s%s%s", prefix, index.Name[:idxNameTruncationIdx], suffix), nil } // rename gets the rename for the index if it eixsts, otherwise it returns an empty stringa nd false