Skip to content

Commit

Permalink
fix: postgresql migration not working with other languages (#1013)
Browse files Browse the repository at this point in the history
* fix: word check on migration error

* refactor: Improve PostgreSQL migration error handling using error codes

* refactor: Improve PostgreSQL migration error handling and transaction management

* refactor: Remove unused compareWordsInString function and its tests

* style: Clean up migrations.go imports and comments

* style: Fix gofmt formatting in migrations.go

* chore: remove migrations_test file

* ci: address golangci-lint warning

* test: stop tests if creating database fails

* fix: ensure migration transaction is commited or rolled back
  • Loading branch information
fmartingr authored Dec 11, 2024
1 parent 7b6fad8 commit 4aa0f51
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 20 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ linters-settings:
linters:
disable-all: true
enable:
- copyloopvar
- gofmt
- gosimple
# - govet # Re-enable when all shadow declarations are fixed
- ineffassign
- predeclared
- exportloopref
- staticcheck
- unconvert
- unused
2 changes: 1 addition & 1 deletion internal/database/database_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func testDatabase(t *testing.T, dbFactory testDatabaseFactory) {
t.Run(testName, func(tInner *testing.T) {
ctx := context.TODO()
db, err := dbFactory(t, ctx)
assert.NoError(tInner, err, "Error recreating database")
require.NoError(tInner, err, "Error recreating database")
testCase(tInner, db)
})
}
Expand Down
4 changes: 3 additions & 1 deletion internal/database/migrations.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Package database implements database operations and migrations
package database

import (
Expand All @@ -13,13 +14,14 @@ import (
//go:embed migrations/*
var migrationFiles embed.FS

// migration represents a database schema migration
type migration struct {
fromVersion semver.Version
toVersion semver.Version
migrationFunc func(db *sql.DB) error
}

// txFunc is a function that runs in a transaction.
// txFn is a function that runs in a transaction.
type txFn func(tx *sql.Tx) error

// runInTransaction runs the given function in a transaction.
Expand Down
44 changes: 27 additions & 17 deletions internal/database/pg.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"github.com/pkg/errors"
"golang.org/x/crypto/bcrypt"

_ "github.com/lib/pq"
"github.com/lib/pq"
)

var postgresMigrations = []migration{
Expand All @@ -25,33 +25,43 @@ var postgresMigrations = []migration{
if err != nil {
return fmt.Errorf("failed to start transaction: %w", err)
}
defer tx.Rollback()

_, err = tx.Exec(`ALTER TABLE bookmark ADD COLUMN has_content BOOLEAN DEFAULT FALSE NOT NULL`)
if err != nil && strings.Contains(err.Error(), `column "has_content" of relation "bookmark" already exists`) {
tx.Rollback()
} else if err != nil {
return fmt.Errorf("failed to add has_content column to bookmark table: %w", err)
} else if err == nil {
if errCommit := tx.Commit(); errCommit != nil {
return fmt.Errorf("failed to commit transaction: %w", errCommit)
if err != nil {
// Check if this is a "column already exists" error (PostgreSQL error code 42701)
// If it's not, return error.
// This is needed for users upgrading from >1.5.4 directly into this version.
pqErr, ok := err.(*pq.Error)
if ok && pqErr.Code == "42701" {
tx.Rollback()
} else {
return fmt.Errorf("failed to add has_content column to bookmark table: %w", err)
}
} else {
if err := tx.Commit(); err != nil {
return fmt.Errorf("failed to commit transaction: %w", err)
}
}

tx, err = db.Begin()
if err != nil {
return fmt.Errorf("failed to start transaction: %w", err)
}
defer tx.Rollback()

_, err = tx.Exec(`ALTER TABLE account ADD COLUMN config JSONB NOT NULL DEFAULT '{}'`)
if err != nil && strings.Contains(err.Error(), `column "config" of relation "account" already exists`) {
tx.Rollback()
} else if err != nil {
return fmt.Errorf("failed to add config column to account table: %w", err)
} else if err == nil {
if errCommit := tx.Commit(); errCommit != nil {
return fmt.Errorf("failed to commit transaction: %w", errCommit)
if err != nil {
// Check if this is a "column already exists" error (PostgreSQL error code 42701)
// If it's not, return error
// This is needed for users upgrading from >1.5.4 directly into this version.
pqErr, ok := err.(*pq.Error)
if ok && pqErr.Code == "42701" {
tx.Rollback()
} else {
return fmt.Errorf("failed to add config column to account table: %w", err)
}
} else {
if err := tx.Commit(); err != nil {
return fmt.Errorf("failed to commit transaction: %w", err)
}
}

Expand Down

0 comments on commit 4aa0f51

Please sign in to comment.