From 5d116b36c042e9e97659e5437fd19f02db2c729a Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Thu, 19 Dec 2024 17:05:32 +0000 Subject: [PATCH] fix(db): Remove begin when assembling migrations 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;") + } }) } }