tree: correct mutation property for some opaque operators

In ed733ad, a framework was added to
mark certain opaque operators as DDL or mutations.

This was enhanced in 06581b3, but that
change wasn't exhaustive since it marked some statements as read-only,
even if they could perform DDL.

With the addition of `StatementType()` in
8962176, we can make this a little more
correct.

This allows the check at
https://github.com/cockroachdb/cockroach/blob/48ef0d89e6179c0d348a5236ad308d81fa392f7c/pkg/sql/opt/exec/execbuilder/relational.go#L163
to work correctly, and reject operations that shouldn't be allowed when
using a read-only transaction.

To explain each change:
- BACKUP can modify job state and write to userfiles, so shouldn't be
  allowed in read-only mode.
- SET commands are always allowed in read-only mode in order to match
  Postgres behavior, and since those changes are all in-memory and
  session setting modifications don't respect transactions anyway.
- The crdb_internal tenant functions modify system tables.
- GRANT, REVOKE, and many other privilege-related statements are "DCL"
  (data control language), and all modify system tables or descriptors.
- Declaring cursors is allowed in Postgres read-only transactions.

Release note (bug fix): CREATE ROLE, DELETE ROLE, GRANT, and REVOKE
statements no longer work when the transaction is in read-only mode.

pkg/sql/logictest/testdata/logic_test/txn

@@ -1028,6 +1028,50 @@ SELECT nextval('a')
 statement error cannot execute setval\(\) in a read-only transaction
 SELECT setval('a', 2)
 
+statement error cannot execute CREATE ROLE in a read-only transaction
+CREATE ROLE my_user
+
+statement error cannot execute ALTER ROLE in a read-only transaction
+ALTER ROLE testuser SET default_int_size = 4
+
+statement error cannot execute DROP ROLE in a read-only transaction
+DROP ROLE testuser
+
+statement error cannot execute SET CLUSTER SETTING in a read-only transaction
+SET CLUSTER SETTING sql.auth.change_own_password.enabled = true
+
+statement error cannot execute GRANT in a read-only transaction
+GRANT admin TO testuser
+
+statement error cannot execute REVOKE in a read-only transaction
+REVOKE admin FROM testuser
+
+statement error cannot execute GRANT in a read-only transaction
+GRANT CONNECT ON DATABASE test TO testuser
+
+# SET session variable should work in a read-only txn.
+statement ok
+SET intervalstyle = 'postgres'
+
+statement ok
+SET SESSION CHARACTERISTICS AS TRANSACTION PRIORITY NORMAL
+
+statement ok
+SET SESSION AUTHORIZATION DEFAULT
+
+statement ok
+BEGIN
+
+# DECLARE and FETCH CURSOR should work in a read-only txn.
+statement ok
+DECLARE foo CURSOR FOR SELECT 1
+
+statement ok
+FETCH 1 foo
+
+statement ok
+COMMIT
+
 query T
 SHOW TRANSACTION STATUS
 ----

pkg/sql/sem/tree/stmt.go

@@ -116,7 +116,7 @@ type canModifySchema interface {
 // CanModifySchema returns true if the statement can modify
 // the database schema.
 func CanModifySchema(stmt Statement) bool {
-	if stmt.StatementReturnType() == DDL {
+	if stmt.StatementReturnType() == DDL || stmt.StatementType() == TypeDDL {
 		return true
 	}
 	scm, ok := stmt.(canModifySchema)
@@ -125,13 +125,20 @@ func CanModifySchema(stmt Statement) bool {
 
 // CanWriteData returns true if the statement can modify data.
 func CanWriteData(stmt Statement) bool {
+	if stmt.StatementType() == TypeDCL {
+		// Commands like GRANT and REVOKE modify system tables.
+		return true
+	}
 	switch stmt.(type) {
 	// Normal write operations.
 	case *Insert, *Delete, *Update, *Truncate:
 		return true
 	// Import operations.
 	case *CopyFrom, *Import, *Restore:
 		return true
+	// Backup creates a job and allows you to write into userfiles.
+	case *Backup:
+		return true
 	// CockroachDB extensions.
 	case *Split, *Unsplit, *Relocate, *RelocateRange, *Scatter:
 		return true
@@ -477,7 +484,7 @@ func (*AlterSequence) StatementTag() string { return "ALTER SEQUENCE" }
 func (*AlterRole) StatementReturnType() StatementReturnType { return Ack }
 
 // StatementType implements the Statement interface.
-func (*AlterRole) StatementType() StatementType { return TypeDDL }
+func (*AlterRole) StatementType() StatementType { return TypeDCL }
 
 // StatementTag returns a short string identifying the type of statement.
 func (*AlterRole) StatementTag() string { return "ALTER ROLE" }
@@ -488,7 +495,7 @@ func (*AlterRole) hiddenFromShowQueries() {}
 func (*AlterRoleSet) StatementReturnType() StatementReturnType { return Ack }
 
 // StatementType implements the Statement interface.
-func (*AlterRoleSet) StatementType() StatementType { return TypeDDL }
+func (*AlterRoleSet) StatementType() StatementType { return TypeDCL }
 
 // StatementTag returns a short string identifying the type of statement.
 func (*AlterRoleSet) StatementTag() string { return "ALTER ROLE" }
@@ -844,7 +851,7 @@ func (*CreateType) modifiesSchema() bool { return true }
 func (*CreateRole) StatementReturnType() StatementReturnType { return Ack }
 
 // StatementType implements the Statement interface.
-func (*CreateRole) StatementType() StatementType { return TypeDDL }
+func (*CreateRole) StatementType() StatementType { return TypeDCL }
 
 // StatementTag returns a short string identifying the type of statement.
 func (*CreateRole) StatementTag() string { return "CREATE ROLE" }
@@ -912,7 +919,7 @@ func (d *Discard) StatementTag() string {
 func (n *DeclareCursor) StatementReturnType() StatementReturnType { return Ack }
 
 // StatementType implements the Statement interface.
-func (*DeclareCursor) StatementType() StatementType { return TypeDCL }
+func (*DeclareCursor) StatementType() StatementType { return TypeDML }
 
 // StatementTag returns a short string identifying the type of statement.
 func (*DeclareCursor) StatementTag() string { return "DECLARE CURSOR" }
@@ -975,7 +982,7 @@ func (*DropSequence) StatementTag() string { return "DROP SEQUENCE" }
 func (*DropRole) StatementReturnType() StatementReturnType { return Ack }
 
 // StatementType implements the Statement interface.
-func (*DropRole) StatementType() StatementType { return TypeDDL }
+func (*DropRole) StatementType() StatementType { return TypeDCL }
 
 // StatementTag returns a short string identifying the type of statement.
 func (*DropRole) StatementTag() string { return "DROP ROLE" }
@@ -1342,7 +1349,7 @@ func (*SelectClause) StatementTag() string { return "SELECT" }
 func (*SetVar) StatementReturnType() StatementReturnType { return Ack }
 
 // StatementType implements the Statement interface.
-func (*SetVar) StatementType() StatementType { return TypeDCL }
+func (*SetVar) StatementType() StatementType { return TypeDML }
 
 // StatementTag returns a short string identifying the type of statement.
 func (n *SetVar) StatementTag() string {
@@ -1365,7 +1372,7 @@ func (*SetClusterSetting) StatementTag() string { return "SET CLUSTER SETTING" }
 func (*SetTransaction) StatementReturnType() StatementReturnType { return Ack }
 
 // StatementType implements the Statement interface.
-func (*SetTransaction) StatementType() StatementType { return TypeDCL }
+func (*SetTransaction) StatementType() StatementType { return TypeTCL }
 
 // StatementTag returns a short string identifying the type of statement.
 func (*SetTransaction) StatementTag() string { return "SET TRANSACTION" }
@@ -1395,7 +1402,7 @@ func (*SetZoneConfig) StatementTag() string { return "CONFIGURE ZONE" }
 func (*SetSessionAuthorizationDefault) StatementReturnType() StatementReturnType { return Ack }
 
 // StatementType implements the Statement interface.
-func (*SetSessionAuthorizationDefault) StatementType() StatementType { return TypeDCL }
+func (*SetSessionAuthorizationDefault) StatementType() StatementType { return TypeDML }
 
 // StatementTag returns a short string identifying the type of statement.
 func (*SetSessionAuthorizationDefault) StatementTag() string { return "SET" }
@@ -1404,7 +1411,7 @@ func (*SetSessionAuthorizationDefault) StatementTag() string { return "SET" }
 func (*SetSessionCharacteristics) StatementReturnType() StatementReturnType { return Ack }
 
 // StatementType implements the Statement interface.
-func (*SetSessionCharacteristics) StatementType() StatementType { return TypeDCL }
+func (*SetSessionCharacteristics) StatementType() StatementType { return TypeDML }
 
 // StatementTag returns a short string identifying the type of statement.
 func (*SetSessionCharacteristics) StatementTag() string { return "SET" }

pkg/sql/tenant.go

@@ -321,6 +321,9 @@ func getAvailableTenantID(
 func (p *planner) CreateTenant(
 	ctx context.Context, name roachpb.TenantName,
 ) (roachpb.TenantID, error) {
+	if p.EvalContext().TxnReadOnly {
+		return roachpb.TenantID{}, readOnlyError("create_tenant()")
+	}
 	const op = "create tenant"
 	if err := p.RequireAdminRole(ctx, op); err != nil {
 		return roachpb.TenantID{}, err
@@ -343,6 +346,9 @@ func (p *planner) CreateTenant(
 func (p *planner) CreateTenantWithID(
 	ctx context.Context, tenantID uint64, tenantName roachpb.TenantName,
 ) error {
+	if p.EvalContext().TxnReadOnly {
+		return readOnlyError("create_tenant()")
+	}
 	if err := p.RequireAdminRole(ctx, "create tenant"); err != nil {
 		return err
 	}
@@ -552,6 +558,10 @@ func (p *planner) DestroyTenantByID(
 }
 
 func (p *planner) validateDestroyTenant(ctx context.Context) error {
+	if p.EvalContext().TxnReadOnly {
+		return readOnlyError("destroy_tenant()")
+	}
+
 	const op = "destroy"
 	if err := p.RequireAdminRole(ctx, "destroy tenant"); err != nil {
 		return err
@@ -794,6 +804,10 @@ func TestingUpdateTenantRecord(
 func (p *planner) RenameTenant(
 	ctx context.Context, tenantID uint64, tenantName roachpb.TenantName,
 ) error {
+	if p.EvalContext().TxnReadOnly {
+		return readOnlyError("rename_tenant()")
+	}
+
 	if err := tenantName.IsValid(); err != nil {
 		return pgerror.WithCandidateCode(err, pgcode.Syntax)
 	}