sql: mark implicit transactions with AOST as read-only

pkg/sql/conn_executor.go

@@ -1107,8 +1107,12 @@ func (s *Server) newConnExecutor(
 
 	// The transaction_read_only variable is special; its updates need to be
 	// hooked-up to the executor.
-	ex.dataMutatorIterator.setCurTxnReadOnly = func(val bool) {
-		ex.state.readOnly.Swap(val)
+	ex.dataMutatorIterator.setCurTxnReadOnly = func(readOnly bool) error {
+		mode := tree.ReadWrite
+		if readOnly {
+			mode = tree.ReadOnly
+		}
+		return ex.state.setReadOnlyMode(mode)
 	}
 	ex.dataMutatorIterator.onTempSchemaCreation = func() {
 		ex.hasCreatedTemporarySchema = true

pkg/sql/conn_executor_exec.go

@@ -1292,6 +1292,10 @@ func (ex *connExecutor) handleAOST(ctx context.Context, stmt tree.Statement) err
 					return err
 				}
 			}
+			if err := ex.state.setReadOnlyMode(tree.ReadOnly); err != nil {
+				return err
+			}
+			p.extendedEvalCtx.TxnReadOnly = ex.state.readOnly.Load()
 			return nil
 		}
 		if *p.extendedEvalCtx.AsOfSystemTime == *asOf {

pkg/sql/exec_util.go

@@ -2973,7 +2973,7 @@ type sessionDataMutatorCallbacks struct {
 	paramStatusUpdater paramStatusUpdater
 	// setCurTxnReadOnly is called when we execute SET transaction_read_only = ...
 	// It can be nil, in which case nothing triggers on execution.
-	setCurTxnReadOnly func(val bool)
+	setCurTxnReadOnly func(readOnly bool) error
 	// upgradedIsolationLevel is called whenever the transaction isolation
 	// session variable is configured and the isolation level is automatically
 	// upgraded to a stronger one.
@@ -3297,16 +3297,16 @@ func (m *sessionDataMutator) SetCustomOption(name, val string) {
 	m.data.CustomOptions[name] = val
 }
 
-func (m *sessionDataMutator) SetReadOnly(val bool) {
+func (m *sessionDataMutator) SetReadOnly(val bool) error {
 	// The read-only state is special; it's set as a session variable (SET
 	// transaction_read_only=<>), but it represents per-txn state, not
 	// per-session. There's no field for it in the SessionData struct. Instead, we
-	// call into the connEx, which modifies its TxnState.
-	// NOTE(andrei): I couldn't find good documentation on transaction_read_only,
-	// but I've tested its behavior in Postgres 11.
+	// call into the connEx, which modifies its TxnState. This is similar to
+	// transaction_isolation.
 	if m.setCurTxnReadOnly != nil {
-		m.setCurTxnReadOnly(val)
+		return m.setCurTxnReadOnly(val)
 	}
+	return nil
 }
 
 func (m *sessionDataMutator) SetStmtTimeout(timeout time.Duration) {

pkg/sql/internal.go

@@ -389,9 +389,11 @@ func (ie *InternalExecutor) newConnExecutorWithTxn(
 	if txn.Type() == kv.LeafTxn {
 		// If the txn is a leaf txn it is not allowed to perform mutations. For
 		// sanity, set read only on the session.
-		ex.dataMutatorIterator.applyOnEachMutator(func(m sessionDataMutator) {
-			m.SetReadOnly(true)
-		})
+		if err := ex.dataMutatorIterator.applyOnEachMutatorError(func(m sessionDataMutator) error {
+			return m.SetReadOnly(true)
+		}); err != nil {
+			return nil, err
+		}
 	}
 
 	// The new transaction stuff below requires active monitors and traces, so

pkg/sql/logictest/testdata/logic_test/as_of

@@ -160,3 +160,10 @@ select * from t as of system time '-1s'; select * from t as of system time '-2s'
 # Specifying the AOST in the first statement (and no others) is allowed.
 statement ok
 select * from t as of system time '-1s'; select * from t;
+
+# Verify that statements with AOST are read-only.
+statement error cannot execute UPDATE in a read-only transaction
+WITH x AS (UPDATE t SET i = 3 WHERE i = 2 RETURNING i) SELECT * FROM x AS OF SYSTEM TIME '-1ms'
+
+statement error cannot execute SELECT FOR UPDATE in a read-only transaction
+SELECT * FROM t AS OF SYSTEM TIME '-1ms' FOR UPDATE

pkg/sql/logictest/testdata/logic_test/txn

@@ -1330,6 +1330,12 @@ UPSERT INTO kv VALUES('foo')
 statement error cannot execute DELETE in a read-only transaction
 DELETE FROM kv
 
+statement error cannot execute SELECT FOR UPDATE in a read-only transaction
+SELECT * FROM kv FOR UPDATE
+
+statement error cannot execute SELECT FOR SHARE in a read-only transaction
+SELECT * FROM kv FOR SHARE
+
 statement error cannot execute nextval\(\) in a read-only transaction
 SELECT nextval('a')
 
@@ -1414,6 +1420,26 @@ ROLLBACK
 statement ok
 SET default_transaction_read_only = false
 
+# Test that we cannot change to READ WRITE during AS OF SYSTEM TIME transactions.
+
+statement ok
+BEGIN AS OF SYSTEM TIME '-1us'
+
+statement error AS OF SYSTEM TIME specified with READ WRITE mode
+SET transaction_read_only = false
+
+statement ok
+ROLLBACK
+
+statement ok
+BEGIN AS OF SYSTEM TIME '-1us'
+
+statement error AS OF SYSTEM TIME specified with READ WRITE mode
+SET TRANSACTION READ WRITE
+
+statement ok
+ROLLBACK
+
 # Transaction AS OF SYSTEM TIME clauses can be assigned a default value.
 
 query T

pkg/sql/vars.go

@@ -1623,8 +1623,7 @@ var varGen = map[string]sessionVar{
 			if err != nil {
 				return err
 			}
-			m.SetReadOnly(b)
-			return nil
+			return m.SetReadOnly(b)
 		},
 		Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
 			return formatBoolAsPostgresSetting(evalCtx.TxnReadOnly), nil