sql/schemachanger: disable declarative CREATE statements

Previously, the decalrative schema changer had
CREATE SEQUENCE and CREATE SCHEMA enabled by default.
There may not be sufficient testing for both, so we
are at a risk of introducing regressiosn. To dddress this,
this patch will disable both by default to mitigate risk.

Fixes: cockroachdb#107734

Release note: None

Author: fqazi

pkg/sql/logictest/testdata/logic_test/event_log

@@ -708,8 +708,8 @@ FROM system.eventlog
 WHERE "eventType" = 'create_schema'
 ORDER BY "timestamp", info
 ----
-1  {"EventType": "create_schema", "Owner": "root", "SchemaName": "test.sc", "Statement": "CREATE SCHEMA test.sc", "Tag": "CREATE SCHEMA", "User": "root"}
-1  {"EventType": "create_schema", "Owner": "root", "SchemaName": "test.s", "Statement": "CREATE SCHEMA test.s", "Tag": "CREATE SCHEMA", "User": "root"}
+1  {"EventType": "create_schema", "Owner": "root", "SchemaName": "test.sc", "Statement": "CREATE SCHEMA \"\".sc", "Tag": "CREATE SCHEMA", "User": "root"}
+1  {"EventType": "create_schema", "Owner": "root", "SchemaName": "test.s", "Statement": "CREATE SCHEMA \"\".s", "Tag": "CREATE SCHEMA", "User": "root"}
 1  {"EventType": "create_schema", "Owner": "u", "SchemaName": "test.u", "Statement": "CREATE SCHEMA AUTHORIZATION u", "Tag": "CREATE SCHEMA", "User": "root"}
 
 statement ok

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/process.go

@@ -64,8 +64,8 @@ var supportedStatements = map[reflect.Type]supportedStatement{
 	reflect.TypeOf((*tree.DropIndex)(nil)):           {fn: DropIndex, statementTag: tree.DropIndexTag, on: true, checks: isV231Active},
 	reflect.TypeOf((*tree.DropFunction)(nil)):        {fn: DropFunction, statementTag: tree.DropFunctionTag, on: true, checks: isV231Active},
 	reflect.TypeOf((*tree.CreateRoutine)(nil)):       {fn: CreateFunction, statementTag: tree.CreateRoutineTag, on: true, checks: isV231Active},
-	reflect.TypeOf((*tree.CreateSchema)(nil)):        {fn: CreateSchema, statementTag: tree.CreateSchemaTag, on: true, checks: isV232Active},
-	reflect.TypeOf((*tree.CreateSequence)(nil)):      {fn: CreateSequence, statementTag: tree.CreateSequenceTag, on: true, checks: isV232Active},
+	reflect.TypeOf((*tree.CreateSchema)(nil)):        {fn: CreateSchema, statementTag: tree.CreateSchemaTag, on: false, checks: isV232Active},
+	reflect.TypeOf((*tree.CreateSequence)(nil)):      {fn: CreateSequence, statementTag: tree.CreateSequenceTag, on: false, checks: isV232Active},
 }
 
 // supportedStatementTags tracks statement tags which are implemented

pkg/sql/schemachanger/schemachanger_test.go

@@ -493,6 +493,7 @@ func TestSchemaChangeWaitsForOtherSchemaChanges(t *testing.T) {
 		defer s.Stopper().Stop(ctx)
 
 		tdb := sqlutils.MakeSQLRunner(sqlDB)
+		tdb.Exec(t, "SET CLUSTER SETTING sql.schema.force_declarative_statements='+CREATE SCHEMA'")
 		tdb.Exec(t, `CREATE DATABASE db`)
 		tdb.Exec(t, `CREATE SCHEMA db.s1`)
 		tdb.Exec(t, `CREATE SCHEMA db.s2`)

pkg/sql/schemachanger/testdata/end_to_end/add_column_default_seq/add_column_default_seq.side_effects

@@ -134,8 +134,8 @@ upsert descriptor #107
   +    columnIds:
   +    - 2
   +    id: 106
-     formatVersion: 3
-     id: 107
+     families:
+     - columnIds:
   ...
        start: "1"
      unexposedParentSchemaId: 105
@@ -289,8 +289,8 @@ upsert descriptor #107
   +    columnIds:
   +    - 2
   +    id: 106
-     formatVersion: 3
-     id: 107
+     families:
+     - columnIds:
   ...
        start: "1"
      unexposedParentSchemaId: 105

pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt.side_effects

@@ -34,8 +34,8 @@ upsert descriptor #104
   +  dependedOnBy:
   +  - byId: true
   +    id: 107
-     formatVersion: 3
-     id: 104
+     families:
+     - columnIds:
   ...
        start: "1"
      unexposedParentSchemaId: 101
@@ -120,8 +120,8 @@ upsert descriptor #104
   +  dependedOnBy:
   +  - byId: true
   +    id: 107
-     formatVersion: 3
-     id: 104
+     families:
+     - columnIds:
   ...
        start: "1"
      unexposedParentSchemaId: 101

pkg/sql/schemachanger/testdata/end_to_end/create_function/create_function.side_effects

@@ -144,8 +144,8 @@ upsert descriptor #105
   +  dependedOnBy:
   +  - byId: true
   +    id: 110
-     formatVersion: 3
-     id: 105
+     families:
+     - columnIds:
   ...
        start: "1"
      unexposedParentSchemaId: 101
@@ -288,8 +288,8 @@ upsert descriptor #105
   +  dependedOnBy:
   +  - byId: true
   +    id: 110
-     formatVersion: 3
-     id: 105
+     families:
+     - columnIds:
   ...
        start: "1"
      unexposedParentSchemaId: 101

pkg/sql/schemachanger/testdata/end_to_end/drop_function/drop_function.side_effects

@@ -92,8 +92,8 @@ upsert descriptor #105
   -  dependedOnBy:
   -  - byId: true
   -    id: 109
-     formatVersion: 3
-     id: 105
+     families:
+     - columnIds:
   ...
        start: "1"
      unexposedParentSchemaId: 101
@@ -226,8 +226,8 @@ upsert descriptor #105
   +    nameMapping:
   +      id: 105
   +      name: sq1
-     formatVersion: 3
-     id: 105
+     families:
+     - columnIds:
   ...
        start: "1"
      unexposedParentSchemaId: 101
@@ -377,8 +377,8 @@ upsert descriptor #105
   -    nameMapping:
   -      id: 105
   -      name: sq1
-     formatVersion: 3
-     id: 105
+     families:
+     - columnIds:
   ...
        start: "1"
      unexposedParentSchemaId: 101

pkg/sql/testdata/telemetry/schema

@@ -154,8 +154,7 @@ sql.schema.schema_changer_mode.legacy
 feature-usage
 CREATE SCHEMA a
 ----
-sql.schema.create_schema
-sql.schema.schema_changer_mode.declarative
+sql.schema.schema_changer_mode.legacy
 
 feature-usage
 CREATE INDEX ON t(b)

pkg/workload/schemachange/operation_generator.go

@@ -34,7 +34,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
-	"github.com/cockroachdb/cockroach/pkg/workload"
 	"github.com/cockroachdb/errors"
 	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgconn"
@@ -358,18 +357,6 @@ func init() {
 	}
 }
 
-// adjustOpWeightsForActiveVersion adjusts the weights for the active cockroach
-// version, allowing us to disable certain operations in mixed version scenarios.
-func adjustOpWeightsForCockroachVersion(
-	ctx context.Context, pool *workload.MultiConnPool, opWeights []int,
-) error {
-	tx, err := pool.Get().Begin(ctx)
-	if err != nil {
-		return err
-	}
-	return tx.Rollback(ctx)
-}
-
 // getSupportedDeclarativeOp generates declarative operations until,
 // a fully supported one is found. This is required for mixed version testing
 // support, where statements may be partially supproted.

pkg/workload/schemachange/schemachange.go

@@ -22,8 +22,10 @@ import (
 	"os"
 	"regexp"
 	"sync"
+	"sync/atomic"
 	"time"
 
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -79,6 +81,7 @@ type schemaChange struct {
 	logFilePath                     string
 	logFile                         *os.File
 	dumpLogsOnce                    *sync.Once
+	declarativeStatementsEnabled    atomic.Bool
 	workers                         []*schemaChangeWorker
 	fkParentInvalidPct              int
 	fkChildInvalidPct               int
@@ -181,11 +184,6 @@ func (s *schemaChange) Ops(
 	if err != nil {
 		return workload.QueryLoad{}, err
 	}
-
-	err = adjustOpWeightsForCockroachVersion(ctx, pool, opWeights)
-	if err != nil {
-		return workload.QueryLoad{}, err
-	}
 	ops := newDeck(rand.New(rand.NewSource(timeutil.Now().UnixNano())), opWeights...)
 	// A separate deck is constructed of only schema changes supported
 	// by the declarative schema changer. This deck has equal weights,
@@ -209,7 +207,6 @@ func (s *schemaChange) Ops(
 		}
 		artifactsLog = makeAtomicLog(s.logFile)
 	}
-
 	s.dumpLogsOnce = &sync.Once{}
 
 	for i := 0; i < s.connFlags.Concurrency; i++ {
@@ -447,6 +444,21 @@ func (w *schemaChangeWorker) run(ctx context.Context) error {
 		return errors.Wrap(err, "cannot get a connection and begin a txn")
 	}
 
+	// Enable extra schema changes, if they are available this moment.
+	if !w.workload.declarativeStatementsEnabled.Load() {
+		cannotEnableSchemaChanges, err := isClusterVersionLessThan(ctx, tx, clusterversion.ByKey(clusterversion.V23_2))
+		if err != nil {
+			return errors.Wrap(err, "cannot to get active")
+		}
+		if !cannotEnableSchemaChanges {
+			_, err = w.pool.Get().Exec(ctx, `SET CLUSTER SETTING sql.schema.force_declarative_statements="+CREATE SCHEMA, +CREATE SEQUENCE"`)
+			if err != nil {
+				return errors.Wrap(err, "cannot to enable extra schema changes")
+			}
+			w.workload.declarativeStatementsEnabled.Store(true)
+		}
+	}
+
 	// Release log entry locks if holding all.
 	defer w.releaseLocksIfHeld()