From b179cf3c728831eb04c843348dccee873645042a Mon Sep 17 00:00:00 2001 From: Bluefly Deployment Agent Date: Tue, 18 Aug 2026 11:34:17 -0400 Subject: [PATCH] Fix client-visible transaction status flag desync after DDL implicit commit ServerInTransaction (SERVER_STATUS_IN_TRANS) was reported based solely on ctx.GetTransaction() != nil, which is also true for the engine's implicit, per-statement transaction used for ordinary autocommit statements. Combined with a second bug -- TransactionCommittingIter.Close() clearing the transaction on an implicit commit (e.g. a DDL statement issued mid-explicit- transaction) without also clearing ctx.GetIgnoreAutoCommit() -- the very next ordinary autocommit statement after such a DDL was misreported to the client as still being inside a transaction. This desyncs any MySQL client whose own BEGIN/COMMIT bookkeeping mirrors the server status flag (e.g. PHP's PDO_MySQL, whose PDO::inTransaction() and PDO::beginTransaction() guard read this exact flag): the client believes no transaction is open (correctly, since it never issued a matching COMMIT/ ROLLBACK for anything), then refuses its own next explicit BEGIN client-side with "There is already an active transaction", even though the server was never asked to begin one. Reproduced against Drupal core's `drush site:install standard`: an unmodified Drupal installation using Drupal's stock mysql PDO driver failed late in install (site-configure step) with exactly this PDOException. MySQL 8.0.46 does not exhibit this: DDL correctly implicit-commits and the transaction-status flag correctly resets for subsequent autocommit statements. Minimal reproduction (both statements executed via PDO against a session with an explicit BEGIN already in effect): BEGIN; CREATE TABLE t (id INT PRIMARY KEY); -- implicit commit INSERT INTO t VALUES (1); -- ordinary autocommit statement MySQL 8.0: PDO::inTransaction() is false after both statements. Dolt (before this fix): PDO::inTransaction() is true after the INSERT, with no BEGIN ever issued for it. Fixes: - sql/rowexec/transaction_iters.go: TransactionCommittingIter.Close() now clears ctx.SetIgnoreAutoCommit(false) when committing an implicit transaction, mirroring what the explicit COMMIT/ROLLBACK handlers in sql/rowexec/transaction.go already do. - server/handler.go: setConnStatusFlags() now additionally requires ctx.GetIgnoreAutoCommit() before reporting ServerInTransaction, so an implicit per-statement transaction can never set the client-visible flag even if a future code path reintroduces a similar gap. Verified: full `server` and `sql/rowexec` package test suites pass, plus targeted `enginetest` transaction/DDL-implicit-commit suites. Re-ran Drupal's `drush site:install standard` end-to-end against a Dolt build with this fix applied (unmodified Drupal core, stock mysql driver, zero Drupal patches): install now completes successfully. --- server/handler.go | 18 +++++++++++- server/handler_test.go | 50 ++++++++++++++++++++++++++++++++ sql/rowexec/transaction_iters.go | 15 ++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) diff --git a/server/handler.go b/server/handler.go index 8abf4d7e8c..a240cfc6cd 100644 --- a/server/handler.go +++ b/server/handler.go @@ -965,7 +965,23 @@ func setConnStatusFlags(ctx *sql.Context, c *mysql.Conn) error { c.StatusFlags &= ^uint16(mysql.ServerStatusAutocommit) } - if t := ctx.GetTransaction(); t != nil { + // A non-nil transaction alone does not mean the client is inside an + // explicit transaction: the engine also opens an implicit, per-statement + // transaction for ordinary autocommit statements. Reporting + // ServerInTransaction for those implicit transactions leaks an internal + // implementation detail onto the wire and desyncs MySQL clients (e.g. + // PDO_MySQL, whose BEGIN/COMMIT bookkeeping mirrors this flag) from the + // server's actual, client-visible transaction state: a client can end up + // believing a transaction is still open immediately after an ordinary + // autocommit statement, and its next explicit BEGIN is then refused + // client-side with "There is already an active transaction" even though + // it never issued a matching COMMIT/ROLLBACK for anything. + // + // GetIgnoreAutoCommit() is true only between an explicit + // BEGIN/START TRANSACTION and its COMMIT/ROLLBACK (see + // sql/rowexec/transaction.go), so it is the correct signal for whether + // the current transaction is client-visible. + if t := ctx.GetTransaction(); t != nil && ctx.GetIgnoreAutoCommit() { c.StatusFlags |= uint16(mysql.ServerInTransaction) } else { c.StatusFlags &= ^uint16(mysql.ServerInTransaction) diff --git a/server/handler_test.go b/server/handler_test.go index 8c12e11660..64f5d9d086 100644 --- a/server/handler_test.go +++ b/server/handler_test.go @@ -2066,3 +2066,53 @@ func TestHandlerNewConnectionProcessListInteractions(t *testing.T) { assert.Equal(t, "test", procs[0].Database) } } + +// fakeTransaction is a minimal sql.Transaction fixture for tests that only +// need a non-nil transaction, not real commit/rollback semantics. +type fakeTransaction struct{} + +func (fakeTransaction) String() string { return "fakeTransaction" } +func (fakeTransaction) IsReadOnly() bool { return false } + +// TestSetConnStatusFlagsInTransaction verifies that the ServerInTransaction +// status flag reflects only client-visible (explicit) transactions, not the +// engine's internal, per-statement implicit transactions used for ordinary +// autocommit statements. A non-nil ctx.GetTransaction() alone is not +// sufficient: an implicit transaction with GetIgnoreAutoCommit() == false +// must not set the flag, or MySQL clients (whose own BEGIN/COMMIT +// bookkeeping mirrors this wire flag, e.g. PHP's PDO_MySQL) end up believing +// a transaction is open when the server considers none to be, and refuse a +// subsequent, legitimate client-issued BEGIN. +func TestSetConnStatusFlagsInTransaction(t *testing.T) { + newCtx := func() *sql.Context { + session := sql.NewBaseSession() + return sql.NewContext(context.Background(), sql.WithSession(session)) + } + + t.Run("no transaction", func(t *testing.T) { + ctx := newCtx() + conn := &mysql.Conn{} + require.NoError(t, setConnStatusFlags(ctx, conn)) + assert.Equal(t, uint16(0), conn.StatusFlags&uint16(mysql.ServerInTransaction)) + }) + + t.Run("implicit per-statement transaction (ordinary autocommit statement)", func(t *testing.T) { + ctx := newCtx() + ctx.SetTransaction(fakeTransaction{}) + // GetIgnoreAutoCommit() defaults to false: no explicit BEGIN was issued. + conn := &mysql.Conn{} + require.NoError(t, setConnStatusFlags(ctx, conn)) + assert.Equal(t, uint16(0), conn.StatusFlags&uint16(mysql.ServerInTransaction), + "an implicit, per-statement transaction must not set ServerInTransaction") + }) + + t.Run("explicit client transaction (after BEGIN)", func(t *testing.T) { + ctx := newCtx() + ctx.SetTransaction(fakeTransaction{}) + ctx.SetIgnoreAutoCommit(true) + conn := &mysql.Conn{} + require.NoError(t, setConnStatusFlags(ctx, conn)) + assert.NotEqual(t, uint16(0), conn.StatusFlags&uint16(mysql.ServerInTransaction), + "an explicit client transaction must set ServerInTransaction") + }) +} diff --git a/sql/rowexec/transaction_iters.go b/sql/rowexec/transaction_iters.go index 99b0041436..7b163f2366 100644 --- a/sql/rowexec/transaction_iters.go +++ b/sql/rowexec/transaction_iters.go @@ -145,6 +145,21 @@ func (t *TransactionCommittingIter) Close(ctx *sql.Context) error { // Clearing out the current transaction will tell us to start a new one the next time this session queries ctx.SetTransaction(nil) + // An implicit commit (e.g. a DDL statement issued mid-transaction) ends + // whatever explicit, client-initiated transaction was in progress, exactly + // like MySQL's own implicit-commit semantics. If that explicit transaction + // had set ctx.SetIgnoreAutoCommit(true) (see rowexec/transaction.go + // buildStartTransaction), that flag must be cleared here too - otherwise it + // stays incorrectly true, causing the *next* ordinary autocommit statement + // to be misreported as inside a client-visible transaction (see + // server/handler.go setConnStatusFlags), which desyncs MySQL clients whose + // BEGIN/COMMIT bookkeeping mirrors that wire status flag (e.g. PHP's + // PDO_MySQL) and makes their next legitimate BEGIN fail client-side with + // "There is already an active transaction". + if t.implicitCommit { + ctx.SetIgnoreAutoCommit(false) + } + return nil }