Skip to content

Commit c64e9da

Browse files
authored
fix: adapt CockroachDB DDL for Postgres in migration runner (#2089)
* fix: adapt CockroachDB DDL for Postgres in production migration runner The migration runner executes SQL files as-is, but migrations are written for CockroachDB (DROP INDEX CASCADE for unique constraints). When running against Postgres (demo, local dev), this fails because Postgres requires ALTER TABLE DROP CONSTRAINT. Add adaptCockroachDDLForPostgres to the runner (mirroring the existing test adapter in shared/platform/testdb) so demo deployments work. * test: add multi-statement migration test for DDL adapter * fix: port ADD CONSTRAINT CHECK adapter from testdb to production runner Bring the production migration adapter to parity with the testdb adapter by adding the regex-based ADD CONSTRAINT CHECK rewrite for public-schema tables. --------- Co-authored-by: Ben Coombs <bjcoombs@users.noreply.github.com>
1 parent b4bde58 commit c64e9da

2 files changed

Lines changed: 100 additions & 1 deletion

File tree

internal/migrations/adapt_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package migrations
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
func TestAdaptCockroachDDLForPostgres_NoChanges(t *testing.T) {
10+
input := `CREATE TABLE foo (id UUID PRIMARY KEY);`
11+
assert.Equal(t, input, adaptCockroachDDLForPostgres(input))
12+
}
13+
14+
func TestAdaptCockroachDDLForPostgres_ManifestVersionUniqueConstraint(t *testing.T) {
15+
input := `DROP INDEX IF EXISTS uq_manifest_version_version CASCADE;`
16+
result := adaptCockroachDDLForPostgres(input)
17+
assert.Contains(t, result, "ALTER TABLE manifest_version DROP CONSTRAINT IF EXISTS uq_manifest_version_version")
18+
assert.NotContains(t, result, "DROP INDEX")
19+
}
20+
21+
func TestAdaptCockroachDDLForPostgres_SagaDefinitionUniqueConstraint(t *testing.T) {
22+
input := `DROP INDEX IF EXISTS "public"."uq_platform_saga_definition_name" CASCADE;`
23+
result := adaptCockroachDDLForPostgres(input)
24+
assert.Contains(t, result, `ALTER TABLE "public"."platform_saga_definition" DROP CONSTRAINT IF EXISTS "uq_platform_saga_definition_name"`)
25+
assert.NotContains(t, result, "DROP INDEX")
26+
}
27+
28+
func TestAdaptCockroachDDLForPostgres_AddConstraintCheck(t *testing.T) {
29+
input := `ALTER TABLE public.my_table ADD CONSTRAINT chk_status CHECK (status IN ('a','b'));`
30+
result := adaptCockroachDDLForPostgres(input)
31+
assert.Contains(t, result, "DO $compat$ BEGIN")
32+
assert.Contains(t, result, "EXCEPTION WHEN duplicate_object THEN NULL")
33+
assert.Contains(t, result, "ADD CONSTRAINT chk_status CHECK")
34+
}
35+
36+
func TestAdaptCockroachDDLForPostgres_AddConstraintIfNotExists(t *testing.T) {
37+
input := `ALTER TABLE public.my_table ADD CONSTRAINT IF NOT EXISTS chk_val CHECK (val > 0);`
38+
result := adaptCockroachDDLForPostgres(input)
39+
assert.Contains(t, result, "DO $compat$ BEGIN")
40+
assert.NotContains(t, result, "IF NOT EXISTS")
41+
assert.Contains(t, result, "ADD CONSTRAINT chk_val CHECK")
42+
}
43+
44+
func TestAdaptCockroachDDLForPostgres_MultiStatementMigration(t *testing.T) {
45+
input := `UPDATE manifest_version SET version_new = version::TEXT WHERE version_new IS NULL;
46+
ALTER TABLE manifest_version ALTER COLUMN version_new SET NOT NULL;
47+
DROP INDEX IF EXISTS uq_manifest_version_version CASCADE;
48+
DROP INDEX IF EXISTS idx_manifest_version_version;
49+
ALTER TABLE manifest_version DROP COLUMN version;`
50+
51+
result := adaptCockroachDDLForPostgres(input)
52+
53+
// The DROP INDEX CASCADE for the unique constraint should be rewritten
54+
assert.Contains(t, result, "ALTER TABLE manifest_version DROP CONSTRAINT IF EXISTS uq_manifest_version_version")
55+
// The regular DROP INDEX (non-unique) should be left unchanged
56+
assert.Contains(t, result, "DROP INDEX IF EXISTS idx_manifest_version_version")
57+
// Other statements should be untouched
58+
assert.Contains(t, result, "UPDATE manifest_version SET version_new")
59+
assert.Contains(t, result, "ALTER TABLE manifest_version DROP COLUMN version")
60+
}

internal/migrations/runner.go

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"log/slog"
1818
"net/url"
1919
"os"
20+
"regexp"
2021
"sort"
2122
"strings"
2223
"time"
@@ -400,7 +401,12 @@ func applyDatabaseMigrations(ctx context.Context, dsn, dbName string, migrations
400401

401402
logger.Info("applying migration", "database", dbName, "service", m.service, "file", m.filename)
402403

403-
if _, err := conn.Exec(ctx, m.sql); err != nil {
404+
sql := m.sql
405+
if driver == DriverPostgres {
406+
sql = adaptCockroachDDLForPostgres(sql)
407+
}
408+
409+
if _, err := conn.Exec(ctx, sql); err != nil {
404410
return fmt.Errorf("execute %s/%s: %w", m.service, m.filename, err)
405411
}
406412

@@ -590,3 +596,36 @@ func runPostgresPreMigrationFixups(ctx context.Context, superuserDSN string, dri
590596

591597
return nil
592598
}
599+
600+
// adaptCockroachDDLForPostgres rewrites CockroachDB-specific DDL to work on PostgreSQL.
601+
//
602+
// Migrations are written for CockroachDB (production). When running against
603+
// PostgreSQL (demo, local dev), this function translates known incompatibilities:
604+
// - DROP INDEX CASCADE for unique constraints -> ALTER TABLE DROP CONSTRAINT
605+
// - ADD CONSTRAINT [IF NOT EXISTS] CHECK on public-schema tables -> idempotent DO block
606+
//
607+
// This mirrors shared/platform/testdb/pgx.go:adaptCockroachDDLForPostgres.
608+
func adaptCockroachDDLForPostgres(sql string) string {
609+
// CockroachDB uses DROP INDEX CASCADE for unique constraints;
610+
// PostgreSQL requires ALTER TABLE DROP CONSTRAINT.
611+
sql = strings.ReplaceAll(sql,
612+
`DROP INDEX IF EXISTS "public"."uq_platform_saga_definition_name" CASCADE`,
613+
`ALTER TABLE "public"."platform_saga_definition" DROP CONSTRAINT IF EXISTS "uq_platform_saga_definition_name"`,
614+
)
615+
sql = strings.ReplaceAll(sql,
616+
`DROP INDEX IF EXISTS uq_manifest_version_version CASCADE`,
617+
`ALTER TABLE manifest_version DROP CONSTRAINT IF EXISTS uq_manifest_version_version`,
618+
)
619+
620+
// Wrap ADD CONSTRAINT ... CHECK statements targeting public-schema tables in a
621+
// DO block that ignores duplicate_object errors. In multi-tenant tests, multiple
622+
// schemas apply the same migration against the shared public schema.
623+
re := regexp.MustCompile(`(?s)(ALTER TABLE\s+public\.\S+\s+)ADD CONSTRAINT(?:\s+IF NOT EXISTS)?\s+(\S+\s+CHECK\s*\([^;]+?\));`)
624+
sql = re.ReplaceAllStringFunc(sql, func(match string) string {
625+
inner := strings.Replace(match, "ADD CONSTRAINT IF NOT EXISTS", "ADD CONSTRAINT", 1)
626+
inner = strings.TrimSuffix(strings.TrimSpace(inner), ";")
627+
return "DO $compat$ BEGIN " + inner + "; EXCEPTION WHEN duplicate_object THEN NULL; END $compat$;"
628+
})
629+
630+
return sql
631+
}

0 commit comments

Comments
 (0)