Repository: supabase/cli
Pull request #2023 (2024-03-06 16:18:10)

fix: set lock timeout when migrating history schema
## What kind of change does this PR introduce?

Bug fix

## What is the new behavior?

Adds lock timeout when altering `schema_migrations` table.

## Additional context

Add any other context or screenshots.


coveralls: ## Pull Request Test Coverage Report for [Build 8175278752](https://coveralls.io/builds/66121733)


### Details

* **4** of **4**   **(100.0%)**  changed or added relevant lines in **3** files are covered.
* **5** unchanged lines in **1** file lost coverage.
* Overall coverage decreased (**-0.02%**) to **58.86%**

---


|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [internal/gen/keys/keys.go](https://coveralls.io/builds/66121733/source?filename=internal%2Fgen%2Fkeys%2Fkeys.go#L101) | 5 | 11.76% |
<!-- | **Total:** | **5** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/66121733/badge)](https://coveralls.io/builds/66121733) |
| :-- | --: |
| Change from base [Build 8170096106](https://coveralls.io/builds/66112721): |  -0.02% |
| Covered Lines: | 6195 |
| Relevant Lines: | 10525 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/internal/gen/types/typescript/typescript.go b/internal/gen/types/typescript/typescript.go
index e2ab071c..b979992d 100644
--- a/internal/gen/types/typescript/typescript.go
+++ b/internal/gen/types/typescript/typescript.go
@@ -50,6 +50,7 @@ func Run(ctx context.Context, projectId string, dbConfig pgconn.Config, schemas
 	}
 
 	fmt.Fprintln(os.Stderr, "Connecting to", dbConfig.Host, dbConfig.Port)
+	// pg-meta does not set username as the default database, ie. postgres
 	if len(dbConfig.Database) == 0 {
 		dbConfig.Database = "postgres"
 	}
diff --git a/internal/link/link.go b/internal/link/link.go
index f21079e2..6bb9e342 100644
--- a/internal/link/link.go
+++ b/internal/link/link.go
@@ -15,7 +15,7 @@ import (
 	"github.com/jackc/pgx/v4"
 	"github.com/spf13/afero"
 	"github.com/spf13/viper"
-	"github.com/supabase/cli/internal/migration/repair"
+	"github.com/supabase/cli/internal/migration/history"
 	"github.com/supabase/cli/internal/utils"
 	"github.com/supabase/cli/internal/utils/credentials"
 	"github.com/supabase/cli/internal/utils/flags"
@@ -184,7 +184,7 @@ func linkDatabase(ctx context.Context, config pgconn.Config, options ...func(*pg
 	defer conn.Close(context.Background())
 	updatePostgresConfig(conn)
 	// If `schema_migrations` doesn't exist on the remote database, create it.
-	return repair.CreateMigrationTable(ctx, conn)
+	return history.CreateMigrationTable(ctx, conn)
 }
 
 func linkDatabaseVersion(ctx context.Context, projectRef string, fsys afero.Fs) error {
diff --git a/internal/migration/apply/apply.go b/internal/migration/apply/apply.go
index 38544219..1fab45b9 100644
--- a/internal/migration/apply/apply.go
+++ b/internal/migration/apply/apply.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/jackc/pgx/v4"
 	"github.com/spf13/afero"
+	"github.com/supabase/cli/internal/migration/history"
 	"github.com/supabase/cli/internal/migration/list"
 	"github.com/supabase/cli/internal/migration/repair"
 	"github.com/supabase/cli/internal/utils"
@@ -40,7 +41,7 @@ func SeedDatabase(ctx context.Context, conn *pgx.Conn, fsys afero.Fs) error {
 
 func MigrateUp(ctx context.Context, conn *pgx.Conn, pending []string, fsys afero.Fs) error {
 	if len(pending) > 0 {
-		if err := repair.CreateMigrationTable(ctx, conn); err != nil {
+		if err := history.CreateMigrationTable(ctx, conn); err != nil {
 			return err
 		}
 	}
diff --git a/internal/migration/history/history.go b/internal/migration/history/history.go
index 7a5e2853..6de64ff0 100644
--- a/internal/migration/history/history.go
+++ b/internal/migration/history/history.go
@@ -1,8 +1,15 @@
 package history
 
-import "github.com/jackc/pgconn"
+import (
+	"context"
+
+	"github.com/go-errors/errors"
+	"github.com/jackc/pgconn"
+	"github.com/jackc/pgx/v4"
+)
 
 const (
+	SET_LOCK_TIMEOUT         = "SET LOCAL lock_timeout = '4s'"
 	CREATE_VERSION_SCHEMA    = "CREATE SCHEMA IF NOT EXISTS supabase_migrations"
 	CREATE_VERSION_TABLE     = "CREATE TABLE IF NOT EXISTS supabase_migrations.schema_migrations (version text NOT NULL PRIMARY KEY)"
 	ADD_STATEMENTS_COLUMN    = "ALTER TABLE supabase_migrations.schema_migrations ADD COLUMN IF NOT EXISTS statements text[]"
@@ -13,10 +20,17 @@ const (
 	TRUNCATE_VERSION_TABLE   = "TRUNCATE supabase_migrations.schema_migrations"
 )
 
-func AddCreateTableStatements(batch *pgconn.Batch) {
-	// Create history table if not exists
+func CreateMigrationTable(ctx context.Context, conn *pgx.Conn) error {
+	// This must be run without prepared statements because each statement in the batch depends on
+	// the previous schema change. The lock timeout will be reset when implicit transaction ends.
+	batch := pgconn.Batch{}
+	batch.ExecParams(SET_LOCK_TIMEOUT, nil, nil, nil, nil)
 	batch.ExecParams(CREATE_VERSION_SCHEMA, nil, nil, nil, nil)
 	batch.ExecParams(CREATE_VERSION_TABLE, nil, nil, nil, nil)
 	batch.ExecParams(ADD_STATEMENTS_COLUMN, nil, nil, nil, nil)
 	batch.ExecParams(ADD_NAME_COLUMN, nil, nil, nil, nil)
+	if _, err := conn.PgConn().ExecBatch(ctx, &batch).ReadAll(); err != nil {
+		return errors.Errorf("failed to create migration table: %w", err)
+	}
+	return nil
 }
diff --git a/internal/migration/repair/repair.go b/internal/migration/repair/repair.go
index 0e61edb7..e81aa71a 100644
--- a/internal/migration/repair/repair.go
+++ b/internal/migration/repair/repair.go
@@ -56,10 +56,8 @@ func Run(ctx context.Context, config pgconn.Config, version []string, status str
 }
 
 func UpdateMigrationTable(ctx context.Context, conn *pgx.Conn, version []string, status string, repairAll bool, fsys afero.Fs) error {
-	schema := &pgconn.Batch{}
-	history.AddCreateTableStatements(schema)
-	if _, err := conn.PgConn().ExecBatch(ctx, schema).ReadAll(); err != nil {
-		return errors.Errorf("failed to create migration table: %w", err)
+	if err := history.CreateMigrationTable(ctx, conn); err != nil {
+		return err
 	}
 	// Data statements don't mutate schemas, safe to use statement cache
 	batch := &pgx.Batch{}
@@ -90,15 +88,6 @@ func UpdateMigrationTable(ctx context.Context, conn *pgx.Conn, version []string,
 	return nil
 }
 
-func CreateMigrationTable(ctx context.Context, conn *pgx.Conn) error {
-	batch := pgconn.Batch{}
-	history.AddCreateTableStatements(&batch)
-	if _, err := conn.PgConn().ExecBatch(ctx, &batch).ReadAll(); err != nil {
-		return errors.Errorf("failed to create migration table: %w", err)
-	}
-	return nil
-}
-
 func InsertVersionSQL(batch *pgconn.Batch, version, name string, stats []string) {
 	encoded := []byte{'{'}
 	for i, line := range stats {
diff --git a/internal/migration/squash/squash.go b/internal/migration/squash/squash.go
index e96ab708..4f8cac7c 100644
--- a/internal/migration/squash/squash.go
+++ b/internal/migration/squash/squash.go
@@ -115,7 +115,7 @@ func baselineMigrations(ctx context.Context, config pgconn.Config, version strin
 		return err
 	}
 	defer conn.Close(context.Background())
-	if err := repair.CreateMigrationTable(ctx, conn); err != nil {
+	if err := history.CreateMigrationTable(ctx, conn); err != nil {
 		return err
 	}
 	m, err := repair.NewMigrationFromVersion(version, fsys)


Test patch:
diff --git a/internal/link/link_test.go b/internal/link/link_test.go
index 81cf643e..f27008c5 100644
--- a/internal/link/link_test.go
+++ b/internal/link/link_test.go
@@ -386,7 +386,8 @@ func TestLinkDatabase(t *testing.T) {
 		// Setup mock postgres
 		conn := pgtest.NewConn()
 		defer conn.Close(t)
-		conn.Query(history.CREATE_VERSION_SCHEMA).
+		conn.Query(history.SET_LOCK_TIMEOUT).
+			Query(history.CREATE_VERSION_SCHEMA).
 			Reply("CREATE SCHEMA").
 			Query(history.CREATE_VERSION_TABLE).
 			ReplyError(pgerrcode.InsufficientPrivilege, "permission denied for relation supabase_migrations").
diff --git a/internal/testing/pgtest/helper.go b/internal/testing/pgtest/helper.go
index 12197944..c8734636 100644
--- a/internal/testing/pgtest/helper.go
+++ b/internal/testing/pgtest/helper.go
@@ -5,7 +5,8 @@ import (
 )
 
 func MockMigrationHistory(conn *MockConn) {
-	conn.Query(history.CREATE_VERSION_SCHEMA).
+	conn.Query(history.SET_LOCK_TIMEOUT).
+		Query(history.CREATE_VERSION_SCHEMA).
 		Reply("CREATE SCHEMA").
 		Query(history.CREATE_VERSION_TABLE).
 		Reply("CREATE TABLE").