From 71af25f584d7c7a22200c1428db4c2cde4c8f154 Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Wed, 8 Jan 2025 10:40:21 -0500 Subject: [PATCH] fix(db): Remove begin when assembling migrations (#5148) When building the set of migrations to run as part of executing the `database migrate` or `database init`, the migrations are collected and combined to run within a database transaction. As such the `begin` and `commit` statements in the migration files should get removed from the final set of statements that get executed. However, when the migrations files were updated to include copyright and license headers, this broke the logic that would strip out the `begin` statement. While this does not cause any issue with executing the statements in a single transaction, it does result in noise in the database server's logs, with log messages like: BEGIN WARNING: 25001: there is already a transaction in progress BEGIN LOCATION: BeginTransactionBlock, xact.c:3778 This commit fixes the logic to correctly strip the `begin` for files that have the header lines. --- internal/db/schema/internal/edition/edition.go | 11 +++++++---- internal/db/schema/internal/edition/edition_test.go | 4 ++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/internal/db/schema/internal/edition/edition.go b/internal/db/schema/internal/edition/edition.go index 6df4f5ce19..cd61a762bb 100644 --- a/internal/db/schema/internal/edition/edition.go +++ b/internal/db/schema/internal/edition/edition.go @@ -78,6 +78,9 @@ func (e Editions) Sort() { // 01_add_new_table.up.sql // 02_refactor_views.up.sql func New(name string, dialect Dialect, m embed.FS, priority int, opt ...Option) (Edition, error) { + const beginStmt = "begin;" + const commitStmt = "commit;" + var largestSchemaVersion int migrations := make(migration.Migrations) @@ -121,11 +124,11 @@ func New(name string, dialect Dialect, m embed.FS, priority int, opt ...Option) } contents := strings.TrimSpace(string(cbts)) - if strings.ToLower(contents[:len("begin;")]) == "begin;" { - contents = contents[len("begin;"):] + if beginIdx := strings.Index(contents, beginStmt); beginIdx != -1 { + contents = contents[beginIdx+len(beginStmt):] } - if strings.ToLower(contents[len(contents)-len("commit;"):]) == "commit;" { - contents = contents[:len(contents)-len("commit;")] + if strings.ToLower(contents[len(contents)-len(commitStmt):]) == commitStmt { + contents = contents[:len(contents)-len(commitStmt)] } contents = strings.TrimSpace(contents) diff --git a/internal/db/schema/internal/edition/edition_test.go b/internal/db/schema/internal/edition/edition_test.go index bbf98dc318..4073c1977b 100644 --- a/internal/db/schema/internal/edition/edition_test.go +++ b/internal/db/schema/internal/edition/edition_test.go @@ -62,6 +62,10 @@ func TestNew(t *testing.T) { assert.Equal(t, e.LatestVersion, tt.expectedVersion, "Version") assert.Equal(t, e.Priority, tt.priority, "Priority") assert.Equal(t, len(e.Migrations), tt.expectedMigrationCount, "Number of migrations") + for _, m := range e.Migrations { + assert.NotContains(t, string(m.Statements), "begin;") + assert.NotContains(t, string(m.Statements), "commit;") + } }) } }