Merge #149449

149449: sql: allow EXPLAIN of mutations in read-only transactions r=yuzefovich a=yuzefovich

Postgres allows EXPLAINing of mutations in read-only transactions, so we should match that behavior. This commit achieves this by temporarily modifying `eval.Context.TxnReadOnly` state when we're building the "pure" EXPLAIN. Out of caution we allowlist DELETE, INSERT, UPDATE, and UPSERT expressions for this, which means that DDLs and ALTERs still get rejected. (PG doesn't support EXPLAIN CREATE TABLE at all, plus EXPLAIN of these is not that useful, unlike for "regular" mutations.)

(We ran into this limitation on a recent incident where running EXPLAIN UPDATE forced us to use higher permissions.)

Fixes: #103754.
Touches: https://github.com/cockroachlabs/support/issues/3365.

Release note (bug fix): CockroachDB now allows EXPLAIN of mutation statements in read-only transaction mode which matches behavior of Postgres. Note that EXPLAIN ANALYZE of mutations is still disallowed since this variant actually executes the statement.

Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: craig[bot]

pkg/sql/logictest/testdata/logic_test/txn

@@ -1502,9 +1502,34 @@ COMMIT
 statement error cannot execute CREATE TABLE in a read-only transaction
 CREATE TABLE tab (a int)
 
+# EXPLAIN of a DDL is disallowed out of caution.
+statement error cannot execute CREATE TABLE in a read-only transaction
+EXPLAIN CREATE TABLE tab (a int)
+
 statement error cannot execute INSERT in a read-only transaction
 INSERT INTO kv VALUES('foo')
 
+# EXPLAIN of a mutation is allowed even in read-only mode.
+statement ok
+EXPLAIN INSERT INTO kv VALUES('foo')
+
+statement ok
+EXPLAIN (OPT) INSERT INTO kv VALUES('foo')
+
+statement ok
+EXPLAIN (DISTSQL) INSERT INTO kv VALUES('foo')
+
+skipif config local-vec-off fakedist-vec-off
+statement ok
+EXPLAIN (VEC) INSERT INTO kv VALUES('foo')
+
+statement ok
+EXPLAIN (GIST) INSERT INTO kv VALUES('foo')
+
+# EXPLAIN ANALYZE is still disallowed.
+statement error cannot execute INSERT in a read-only transaction
+EXPLAIN ANALYZE INSERT INTO kv VALUES('foo')
+
 statement error cannot execute UPDATE in a read-only transaction
 UPDATE kv SET v = 'foo'
 
@@ -1523,6 +1548,13 @@ SELECT * FROM kv FOR SHARE
 statement error cannot execute nextval\(\) in a read-only transaction
 SELECT nextval('a')
 
+# This is just a Values with an expression that hasn't been evaluated.
+statement ok
+EXPLAIN SELECT nextval('a')
+
+statement error pgcode 55000 pq: currval of sequence "test.public.a" is not yet defined in this session
+SELECT currval('a')
+
 statement error cannot execute setval\(\) in a read-only transaction
 SELECT setval('a', 2)
 
@@ -1532,6 +1564,10 @@ CREATE ROLE my_user
 statement error cannot execute ALTER ROLE in a read-only transaction
 ALTER ROLE testuser SET default_int_size = 4
 
+# EXPLAIN of an ALTER is disallowed out of caution.
+statement error cannot execute ALTER ROLE in a read-only transaction
+EXPLAIN ALTER ROLE testuser SET default_int_size = 4
+
 statement error cannot execute DROP ROLE in a read-only transaction
 DROP ROLE testuser
 

pkg/sql/opt/exec/execbuilder/statement.go

@@ -185,6 +185,24 @@ func (b *Builder) buildExplain(
 				b.semaCtx, b.evalCtx, b.initialAllowAutoCommit, b.IsANSIDML,
 			)
 			explainBld.disableTelemetry = true
+			if b.evalCtx.TxnReadOnly {
+				// Since we're building a "pure" EXPLAIN (as opposed to EXPLAIN
+				// ANALYZE variant which the execbuilder is unaware of), we
+				// won't actually execute the plan, and in order to match the
+				// behavior of Postgres we want to allow EXPLAINing the
+				// mutations even in the read-only state.
+				//
+				// Out of caution, we'll only allow this for "regular" mutation
+				// statements (where it's most useful and less risky),
+				// prohibiting EXPLAINing DDLs and ALTERs.
+				switch explainExpr.Input.Op() {
+				case opt.DeleteOp, opt.InsertOp, opt.UpdateOp, opt.UpsertOp:
+					b.evalCtx.TxnReadOnly = false
+					defer func() {
+						b.evalCtx.TxnReadOnly = true
+					}()
+				}
+			}
 			plan, err := explainBld.Build()
 			if err != nil {
 				return nil, err