[feat] add support for outside-of-transaction migrations #269

muir · 2025-09-27T01:11:56Z

Some PostgreSQL commands cannot be used inside transactions.

This change introduces the ability to do PostgreSQL migrations outside of a transaction. When defining a transaction with Generate() or Computed(), transactional or non-transactional will be determined by the type signature of the callback function.

For Script(), the text will be examined and a determination made based on command matching. To force it one way or the other, use the libschema.ForceTransactional() or libschema.ForceNonTransactional() migration options.

MySQL and SingleStore packages were updated to support the same pattern, though they already did most things non-transactionally because they don't support any DDL inside transactions.

Also changed in this PR:

Switched from google.com/pkg/errors to github.com/memsql/errors
Use sentinel errors instead for key situations

codecov · 2025-09-27T23:54:10Z

Codecov Report

❌ Patch coverage is 86.22047% with 70 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.69%. Comparing base (5e8cd23) to head (f1bfd33).

Files with missing lines	Patch %	Lines
lspostgres/postgres.go	74.09%	30 Missing and 13 partials ⚠️
lsmysql/mysql.go	83.21%	16 Missing and 7 partials ⚠️
lssinglestore/singlestore.go	33.33%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #269      +/-   ##
==========================================
+ Coverage   79.79%   81.69%   +1.90%     
==========================================
  Files          12       15       +3     
  Lines        1282     1579     +297     
==========================================
+ Hits         1023     1290     +267     
- Misses        183      201      +18     
- Partials       76       88      +12

Flag	Coverage Δ
go_tests	`20.39% <37.40%> (+11.42%)`	⬆️
mysql_tests	`48.62% <50.80%> (-0.68%)`	⬇️
pg_tests	`50.31% <46.10%> (-0.61%)`	⬇️
singlestore_tests	`45.17% <45.54%> (+0.79%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull Request Overview

Adds support for declaring and executing migrations outside of transactions (idempotent/non-transactional) across Postgres, MySQL, and SingleStore by introducing generic migration helpers (Script/Generate/Computed with *sql.Tx vs *sql.DB), statement classification (new internal/stmtclass), and a shared finalization orchestration (internal/migfinalize). Also replaces github.com/pkg/errors with github.com/memsql/errors and introduces sentinel errors for unsafe migration patterns.

Introduces generic APIs to infer transactional vs non-transactional behavior.
Adds SQL statement classifier and migration finalizer utilities.
Adds extensive new tests and documentation (NON_TRANSACTIONAL.md) plus sentinel errors & updated README.

Reviewed Changes

Copilot reviewed 48 out of 49 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
lstesting/testing_test.go	Adds cleanup nil safety test.
lstesting/testing.go	Adds defensive db.Ping and updated cleanup logic.
lssinglestore/*	Adapts SingleStore driver & tests to generic migrations and new options.
lspostgres/postgres.go	Major refactor: generic migrations, non-tx inference, statement classification, server version handling, finalizer integration.
lspostgres/* tests & docs	New tests for inference, non-tx behavior, schema override, classification, failures; adds NON_TRANSACTIONAL.md.
lsmysql/mysql.go & related tests	Mirrors Postgres generic migration pattern; classification & finalizer integration; mismatch tests.
lsmysql/check.go	Re-implements script checks via stmtclass; re-exports sentinel errors.
internal/stmtclass/*	New statement classification package and tests.
internal/migfinalize/*	New reusable migration finalization utility and tests.
api.go / abstractions.go	Adds sentinel errors, non-tx flags, ExecConn abstractions, RepeatUntilNoOp doc expansion, Force(Non)Transactional options.
apply.go / driver.go / dgorder/dependencies.go	Updates to use new errors package; minor return cleanup.
README.md	Adds non-transactional & RepeatUntilNoOp guidance (typo present).
logger.go	Minor import ordering tweak.
go.mod	Adds github.com/memsql/errors, sqltoken, updates indirect deps.
misc test files	Parallelization and new coverage tests.

Comments suppressed due to low confidence (1)

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

lspostgres/postgres.go

lstesting/testing.go

internal/migfinalize/finalize.go

lsmysql/mysql.go

Copilot

Pull Request Overview

Copilot reviewed 48 out of 49 changed files in this pull request and generated 8 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

lspostgres/postgres.go

lsmysql/mysql.go

internal/stmtclass/stmtclass.go

lspostgres/config_test.go

lspostgres/postgres_test.go

README.md

lspostgres/postgres.go

dncohen

Nice. Looks like it does more than just out-of-transaction migrations. It looks like it increases test coverage a lot. All good, just wondering should it be split into smaller PRs?

abstractions.go

dncohen · 2025-10-08T01:26:57Z

abstractions.go

+// WARNING (foot-gun): On MySQL / SingleStore this DOES NOT make DDL atomic. Those engines
+// autocommit each DDL statement regardless of any BEGIN you issue. By forcing transactional mode:


Should this make the migration fail on those engines? I'm asking is this comment enough?

api.go

internal/migfinalize/finalize.go

lsmysql/check.go

Copilot

Pull Request Overview

Copilot reviewed 56 out of 57 changed files in this pull request and generated 8 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

lstesting/testing_test.go

lspostgres/postgres.go

internal/stmtclass/stmtclass.go

lspostgres/non_tx_test.go

lspostgres/schema_override_test.go

lspostgres/generate_test.go

lsmysql/generate_test.go

internal/stmtclass/stmtclass_extra_test.go

…error cases; remove obsolete tests

…te; remove error_cases_test.go

Copilot

Pull Request Overview

Copilot reviewed 55 out of 56 changed files in this pull request and generated 3 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

lstesting/testing.go

lspostgres/postgres.go

lsmysql/mysql.go

…ersion parsing to regex

Copilot

Pull Request Overview

Copilot reviewed 55 out of 56 changed files in this pull request and generated no new comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

dncohen

still LGTM overall. Minor comments/questions in other threads.

dncohen · 2025-10-23T17:55:02Z

api.go

+		v := false // false means non-transactional
+		b.forcedTx = &v


should this use SetTransactional?

dncohen · 2025-10-23T17:58:50Z

internal/migfinalize/finalize.go

+
+	defer func() {
+		if tx != nil {
+			f.RollbackTx(tx)


looks weird to always rollback. I guess the idea is that it will do nothing after a commit, but still reads funny to me.

dncohen · 2025-10-23T18:02:00Z

internal/stmtclass/stmtclass.go

+// flagsInOrder provides a single stable ordering for iteration & presentation.
+// (Replaces previous duplicated allFlags / flagNameOrder slices.)
+var flagsInOrder = []Flag{IsMultipleStatements, IsDDL, IsDML, IsNonIdempotent, IsEasilyIdempotentFix, IsMustNonTx}


why not just define them in order?

muir temporarily deployed to singlestore September 27, 2025 23:53 — with GitHub Actions Inactive

muir temporarily deployed to singlestore September 29, 2025 15:53 — with GitHub Actions Inactive

muir temporarily deployed to singlestore September 29, 2025 19:15 — with GitHub Actions Inactive

muir temporarily deployed to singlestore September 29, 2025 20:34 — with GitHub Actions Inactive

muir temporarily deployed to singlestore September 30, 2025 15:21 — with GitHub Actions Inactive

muir temporarily deployed to singlestore September 30, 2025 19:18 — with GitHub Actions Inactive

muir temporarily deployed to singlestore October 1, 2025 01:37 — with GitHub Actions Inactive

muir temporarily deployed to singlestore October 1, 2025 18:30 — with GitHub Actions Inactive

muir temporarily deployed to singlestore October 1, 2025 20:51 — with GitHub Actions Inactive

muir temporarily deployed to singlestore October 1, 2025 21:02 — with GitHub Actions Inactive

muir temporarily deployed to singlestore October 3, 2025 23:15 — with GitHub Actions Inactive

muir temporarily deployed to singlestore October 4, 2025 19:14 — with GitHub Actions Inactive

muir temporarily deployed to singlestore October 4, 2025 19:42 — with GitHub Actions Inactive

muir temporarily deployed to singlestore October 4, 2025 20:26 — with GitHub Actions Inactive

muir temporarily deployed to singlestore October 6, 2025 20:25 — with GitHub Actions Inactive

muir temporarily deployed to singlestore October 6, 2025 23:28 — with GitHub Actions Inactive

muir temporarily deployed to singlestore October 7, 2025 18:19 — with GitHub Actions Inactive

muir temporarily deployed to singlestore October 7, 2025 19:04 — with GitHub Actions Inactive

muir requested a review from Copilot October 7, 2025 19:27

Copilot AI reviewed Oct 7, 2025

View reviewed changes

lspostgres/postgres.go Outdated Show resolved Hide resolved

lstesting/testing.go Outdated Show resolved Hide resolved

internal/migfinalize/finalize.go Outdated Show resolved Hide resolved

lsmysql/mysql.go Outdated Show resolved Hide resolved

lsmysql/mysql.go Outdated Show resolved Hide resolved

muir temporarily deployed to singlestore October 7, 2025 20:56 — with GitHub Actions Inactive

muir requested a review from Copilot October 7, 2025 21:04

Copilot AI reviewed Oct 7, 2025

View reviewed changes

dncohen approved these changes Oct 8, 2025

View reviewed changes

muir temporarily deployed to singlestore October 15, 2025 21:49 — with GitHub Actions Inactive

muir temporarily deployed to singlestore October 15, 2025 23:05 — with GitHub Actions Inactive

muir requested a review from Copilot October 15, 2025 23:08

Copilot AI reviewed Oct 15, 2025

View reviewed changes

muir temporarily deployed to singlestore October 15, 2025 23:42 — with GitHub Actions Inactive

muir added 20 commits October 16, 2025 12:09

test updates

51b9e4e

all error generation -> memsql/errors

387708b

tweak readme to retrigger tests

be1f33a

refactor for common code and testability

4406956

lint

251d748

more coverage

bd2f56c

more tests

6f25476

stmtclass

2015e2a

simplify mysql

5179895

simplify mysql.go

e941902

cleanup

1eab28a

copilot feedback

86fe132

wip checkpoint

1583305

redesign

34a1fc5

self review

df5f867

read commited for forced downgrade w/generate

a54e51a

more self review

d8a6586

fake schema everywhere

7673390

test: consolidate postgres non-tx & classification, mysql generate & …

b1b337b

…error cases; remove obsolete tests

test(mysql): fold computed mismatch & failure into bad migrations sui…

1d425d2

…te; remove error_cases_test.go

muir force-pushed the nonTransactional branch from dc79dd5 to 1d425d2 Compare October 16, 2025 19:10

muir temporarily deployed to singlestore October 16, 2025 19:10 — with GitHub Actions Inactive

muir requested a review from Copilot October 16, 2025 19:10

Copilot AI reviewed Oct 16, 2025

View reviewed changes

lstesting/testing.go Outdated Show resolved Hide resolved

lspostgres/postgres.go Show resolved Hide resolved

lsmysql/mysql.go Show resolved Hide resolved

muir added 2 commits October 16, 2025 12:34

chore(testing,postgres): simplify FakeSchema cleanup; switch server v…

753bf0e

…ersion parsing to regex

adjust comments for Script/Generate/Computed; remove GenerateE, a typo

f1bfd33

muir temporarily deployed to singlestore October 16, 2025 19:49 — with GitHub Actions Inactive

muir requested a review from Copilot October 16, 2025 19:52

Copilot AI reviewed Oct 16, 2025

View reviewed changes

dncohen approved these changes Oct 23, 2025

View reviewed changes

		// WARNING (foot-gun): On MySQL / SingleStore this DOES NOT make DDL atomic. Those engines
		// autocommit each DDL statement regardless of any BEGIN you issue. By forcing transactional mode:

[feat] add support for outside-of-transaction migrations #269

Are you sure you want to change the base?

[feat] add support for outside-of-transaction migrations #269

Conversation

muir commented Sep 27, 2025

Uh oh!

codecov bot commented Sep 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dncohen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dncohen Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

dncohen left a comment

Choose a reason for hiding this comment

Uh oh!

dncohen Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

dncohen Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

dncohen Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

codecov bot commented Sep 27, 2025 •

edited

Loading