Skip to content

Commit 8a49b0c

Browse files
committed
sql: allow EXPLAIN of mutations in read-only transactions
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.) 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.
1 parent 1206bb5 commit 8a49b0c

File tree

2 files changed

+54
-0
lines changed

2 files changed

+54
-0
lines changed

pkg/sql/logictest/testdata/logic_test/txn

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1502,9 +1502,34 @@ COMMIT
15021502
statement error cannot execute CREATE TABLE in a read-only transaction
15031503
CREATE TABLE tab (a int)
15041504

1505+
# EXPLAIN of a DDL is disallowed out of caution.
1506+
statement error cannot execute CREATE TABLE in a read-only transaction
1507+
EXPLAIN CREATE TABLE tab (a int)
1508+
15051509
statement error cannot execute INSERT in a read-only transaction
15061510
INSERT INTO kv VALUES('foo')
15071511

1512+
# EXPLAIN of a mutation is allowed even in read-only mode.
1513+
statement ok
1514+
EXPLAIN INSERT INTO kv VALUES('foo')
1515+
1516+
statement ok
1517+
EXPLAIN (OPT) INSERT INTO kv VALUES('foo')
1518+
1519+
statement ok
1520+
EXPLAIN (DISTSQL) INSERT INTO kv VALUES('foo')
1521+
1522+
skipif config local-vec-off fakedist-vec-off
1523+
statement ok
1524+
EXPLAIN (VEC) INSERT INTO kv VALUES('foo')
1525+
1526+
statement ok
1527+
EXPLAIN (GIST) INSERT INTO kv VALUES('foo')
1528+
1529+
# EXPLAIN ANALYZE is still disallowed.
1530+
statement error cannot execute INSERT in a read-only transaction
1531+
EXPLAIN ANALYZE INSERT INTO kv VALUES('foo')
1532+
15081533
statement error cannot execute UPDATE in a read-only transaction
15091534
UPDATE kv SET v = 'foo'
15101535

@@ -1523,6 +1548,13 @@ SELECT * FROM kv FOR SHARE
15231548
statement error cannot execute nextval\(\) in a read-only transaction
15241549
SELECT nextval('a')
15251550

1551+
# This is just a Values with an expression that hasn't been evaluated.
1552+
statement ok
1553+
EXPLAIN SELECT nextval('a')
1554+
1555+
statement error pgcode 55000 pq: currval of sequence "test.public.a" is not yet defined in this session
1556+
SELECT currval('a')
1557+
15261558
statement error cannot execute setval\(\) in a read-only transaction
15271559
SELECT setval('a', 2)
15281560

@@ -1532,6 +1564,10 @@ CREATE ROLE my_user
15321564
statement error cannot execute ALTER ROLE in a read-only transaction
15331565
ALTER ROLE testuser SET default_int_size = 4
15341566

1567+
# EXPLAIN of an ALTER is disallowed out of caution.
1568+
statement error cannot execute ALTER ROLE in a read-only transaction
1569+
EXPLAIN ALTER ROLE testuser SET default_int_size = 4
1570+
15351571
statement error cannot execute DROP ROLE in a read-only transaction
15361572
DROP ROLE testuser
15371573

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,24 @@ func (b *Builder) buildExplain(
185185
b.semaCtx, b.evalCtx, b.initialAllowAutoCommit, b.IsANSIDML,
186186
)
187187
explainBld.disableTelemetry = true
188+
if b.evalCtx.TxnReadOnly {
189+
// Since we're building a "pure" EXPLAIN (as opposed to EXPLAIN
190+
// ANALYZE variant which the execbuilder is unaware of), we
191+
// won't actually execute the plan, and in order to match the
192+
// behavior of Postgres we want to allow EXPLAINing the
193+
// mutations even in the read-only state.
194+
//
195+
// Out of caution, we'll only allow this for "regular" mutation
196+
// statements (where it's most useful and less risky),
197+
// prohibiting EXPLAINing DDLs and ALTERs.
198+
switch explainExpr.Input.Op() {
199+
case opt.DeleteOp, opt.InsertOp, opt.UpdateOp, opt.UpsertOp:
200+
b.evalCtx.TxnReadOnly = false
201+
defer func() {
202+
b.evalCtx.TxnReadOnly = true
203+
}()
204+
}
205+
}
188206
plan, err := explainBld.Build()
189207
if err != nil {
190208
return nil, err

0 commit comments

Comments
 (0)