fix(server): keep the ongoing tx when a statement fails before execution

[vchaindz]

Fixes #2127. Also very likely fixes #1998, which hits the same error through the pgsql adapter.

The bug

A statement rejected before the SQL engine executes it — a parse error being the common case — returns no transaction while leaving the caller's ongoing one open and uncancelled. The two callers that hold the only reference to that transaction overwrote it with the returned nil:

• pkg/server/sessions/internal/transactions/transactions.go:124 — tx.sqlTx, _, err = tx.db.SQLExec(...)
• pkg/pgsql/server/query_machine.go:1205 — s.tx = ntx, assigned even when err != nil

That orphans an OngoingTx still holding its tbtree snapshots. Snapshots are released only by Commit/Cancel, and tbtree.snapshots shrinks only via Snapshot.Close(), so each failure leaked one permanently. After maxActiveSnapshots (default 100) failures, every new transaction failed with tbtree: max active snapshots limit reached until the server was restarted. The reporter hit this four times in production, with a 1:1 correlation between failed statements and leaked snapshots.

On the session path it was unrecoverable: with sqlTx == nil, IsClosed() reports true, so TxSQLExec removed the transaction from the session — after which neither the client's Rollback nor Session.RollbackTransactions() on close/expiry could reach it. That also explains the reported client signature, where Rollback answers no transaction found.

The fix

Guard both assignment sites on what actually happened to the transaction, not on the error:

ntx, _, err := tx.db.SQLExec(ctx, tx.sqlTx, request)
if ntx != nil || tx.sqlTx.Closed() {
    tx.sqlTx = ntx
}

• Pre-execution error — the engine never touched the tx, so it stays open and owned by the session. Rollback now succeeds instead of answering no transaction found, and close/expiry can reclaim it. Leak gone.
• Execution error — the engine already called currTx.Cancel(), so the tx is closed, the reference is replaced and the session drops it. No behaviour change.
• COMMIT; — returns ntx == nil with a closed tx, so the assignment still happens.

Why here and not in the engine

The issue proposed fixing Engine.Exec. That would not have fixed it: the session path parses in db.SQLExec (pkg/database/sql.go:364) and returns before the engine is ever reached. engine.go:756 is a second, independent instance of the same defect, reachable only by embedded users.

More importantly, the engine returning nil is not itself the leak — a caller that passes tx in still owns it. The leak is assigning that nil over the only reference to a still-open tx. Pre-execution returns that leave the tx open include:

• pkg/database/sql.go:364 / :379 / :386 — parse error, empty statement list, replica
• embedded/sql/engine.go:756 / :795 / :800 / :809 — parse error, empty list, normalizeParams failure, nil statement

Guarding the two reference-owning callers closes all of them at once. Patching individual early returns would keep regressing as new ones are added.

Tests

Three tests in transactions_test.go, written before the fix and confirmed to fail without it:

Test	Without the fix
TestSQLExecPreExecutionErrorKeepsTxOpen	fails — IsClosed() true, Rollback() returns ErrNoOngoingTx
TestSQLExecPreExecutionErrorDoesNotLeakSnapshots	fails at iteration 4 of 4 (maxActiveSnapshots = 4) with the snapshot-limit error, reproducing the reported 1:1 correlation
TestSQLExecExecutionErrorClosesTx	passes — pins existing behaviour so this change cannot silently alter the execution-error path

The leak test mirrors the reported production cycle, including the client carrying on after its rollback fails.

Verification

• go build ./... clean
• go test ./pkg/server/sessions/... ./pkg/server/ ./pkg/pgsql/server/... ./pkg/database/... — 10 packages pass
• go test ./embedded/sql/... ./embedded/tbtree/... pass
• Commit is GPG-signed

The pgsql suite matters here because the second hunk touches the extended-query path recently changed by ce64e941 and c4a5beed.

[coveralls]

coverage: 84.849% (+0.01%) from 84.839% — fix/issue-2127-snapshot-leak into master