From bffad122cfdf76011ee27dade596aafe720a0b96 Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Fri, 19 Aug 2022 12:43:39 -0400 Subject: [PATCH] sql: step the sql transaction before commit In #86153 we added logic to step the transaction before auto-commit. This was insufficient to resolve #86132 because we did not step the transaction before committing an explicit transaction. This PR addresses that by moving the step logic to a shared code path. It also adds a test which previously failed. Fixes #86132 Release justification: bug fix Release note: None --- pkg/sql/conn_executor_exec.go | 16 ++++++++++++---- pkg/sql/logictest/testdata/logic_test/schema | 8 ++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/pkg/sql/conn_executor_exec.go b/pkg/sql/conn_executor_exec.go index a488f106a5bc..63419254bfc9 100644 --- a/pkg/sql/conn_executor_exec.go +++ b/pkg/sql/conn_executor_exec.go @@ -926,6 +926,18 @@ func (ex *connExecutor) commitSQLTransactionInternal(ctx context.Context) error ctx, sp := tracing.EnsureChildSpan(ctx, ex.server.cfg.AmbientCtx.Tracer, "commit sql txn") defer sp.Finish() + // We need to step the transaction before committing if it has stepping + // enabled. If it doesn't have stepping enabled, then we just set the + // stepping mode back to what it was. + prevSteppingMode := ex.state.mu.txn.ConfigureStepping(ctx, kv.SteppingEnabled) + if prevSteppingMode == kv.SteppingEnabled { + if err := ex.state.mu.txn.Step(ctx); err != nil { + return err + } + } else { + ex.state.mu.txn.ConfigureStepping(ctx, prevSteppingMode) + } + if err := ex.createJobs(ctx); err != nil { return err } @@ -2096,10 +2108,6 @@ func (ex *connExecutor) handleAutoCommit( if err != nil { return ex.makeErrEvent(err, stmt) } - if err = ex.state.mu.txn.Step(ctx); err != nil { - log.VEventf(ctx, 2, "AutoCommit. err: %v", err) - return ex.makeErrEvent(err, stmt) - } ev, payload := ex.commitSQLTransaction(ctx, stmt, ex.commitSQLTransactionInternal) if perr, ok := payload.(payloadWithError); ok { err = perr.errorCause() diff --git a/pkg/sql/logictest/testdata/logic_test/schema b/pkg/sql/logictest/testdata/logic_test/schema index 3b403fbb4791..2d924b2c80dd 100644 --- a/pkg/sql/logictest/testdata/logic_test/schema +++ b/pkg/sql/logictest/testdata/logic_test/schema @@ -875,6 +875,14 @@ CREATE SCHEMA sc statement ok DROP SCHEMA sc +statement ok +BEGIN; +CREATE SCHEMA sc; +COMMIT + +statement ok +DROP SCHEMA sc + statement ok SET CLUSTER SETTING server.eventlog.enabled = false