Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion server/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
50 changes: 50 additions & 0 deletions server/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
}
15 changes: 15 additions & 0 deletions sql/rowexec/transaction_iters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading