Merge #128833 #128834 #128851 #128855

128833: workload/schemachange: avoid commmenting on transient check constraints r=rafiss a=rafiss

These check constraints are added temporarily while adding a non-nullable column. We have seen flakes caused by the workload trying to refer to them.

informs #128615
Release note: None

128834: workload/schemachange: only allow PK change in single statement transactions r=rafiss a=rafiss

This will avoid errors of the form: "cannot perform other schema changes in the same transaction as a primary key change."

informs: #128615
Release note: None

128851: roachtest: Fix ycsb workload to avoid column families r=dt a=navsetlur

The update_heavy test had failures because the ycsb workload was using column families, which aren't supported for LDR.

Release note: none
Fixes: #128740

128855: schemachanger: remove unneeded function r=rafiss a=rafiss

fallBackIfDescColInRowLevelTTLTables is no longer needed as of e5b30e4.

Epic: None
Release note: None

Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: Naveen Setlur <naveen.setlur@cockroachlabs.com>

Author: 3 people

pkg/cmd/roachtest/tests/logical_data_replication.go

@@ -44,15 +44,15 @@ type ycsbWorkload struct {
 }
 
 func (ycsb ycsbWorkload) sourceInitCmd(tenantName string, nodes option.NodeListOption) string {
-	cmd := roachtestutil.NewCommand(`./cockroach workload init ycsb`).
+	cmd := roachtestutil.NewCommand(`./cockroach workload init ycsb --families=false`).
 		MaybeFlag(ycsb.initRows > 0, "insert-count", ycsb.initRows).
 		MaybeFlag(ycsb.initSplits > 0, "splits", ycsb.initSplits).
 		Arg("{pgurl%s:%s}", nodes, tenantName)
 	return cmd.String()
 }
 
 func (ycsb ycsbWorkload) sourceRunCmd(tenantName string, nodes option.NodeListOption) string {
-	cmd := roachtestutil.NewCommand(`./cockroach workload run ycsb`).
+	cmd := roachtestutil.NewCommand(`./cockroach workload run ycsb --families=false`).
 		Option("tolerate-errors").
 		Flag("workload", ycsb.workloadType).
 		MaybeFlag(ycsb.debugRunDuration > 0, "duration", ycsb.debugRunDuration).

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

@@ -71,7 +71,6 @@ func alterPrimaryKey(b BuildCtx, tn *tree.TableName, tbl *scpb.Table, t alterPri
 	fallBackIfShardedIndexExists(b, t, tbl.TableID)
 	fallBackIfPartitionedIndexExists(b, t, tbl.TableID)
 	fallBackIfRegionalByRowTable(b, t.n, tbl.TableID)
-	fallBackIfDescColInRowLevelTTLTables(b, tbl.TableID, t)
 	fallBackIfSubZoneConfigExists(b, t.n, tbl.TableID)
 
 	inflatedChain := getInflatedPrimaryIndexChain(b, tbl.TableID)
@@ -446,15 +445,6 @@ func fallBackIfRegionalByRowTable(b BuildCtx, t tree.NodeFormatter, tableID cati
 	}
 }
 
-// fallBackIfDescColInRowLevelTTLTables panics with an unimplemented
-// error if the table is a (row-level-ttl table && (it has a descending
-// key column || it has any inbound/outbound FK constraint)).
-func fallBackIfDescColInRowLevelTTLTables(b BuildCtx, tableID catid.DescID, t alterPrimaryKeySpec) {
-	if _, _, rowLevelTTLElem := scpb.FindRowLevelTTL(b.QueryByID(tableID)); rowLevelTTLElem == nil {
-		return
-	}
-}
-
 func mustRetrieveCurrentPrimaryIndexElement(
 	b BuildCtx, tableID catid.DescID,
 ) (res *scpb.PrimaryIndex) {

pkg/workload/schemachange/operation_generator.go

@@ -182,7 +182,7 @@ func (og *operationGenerator) getSupportedDeclarativeOp(
 // stochastic attempts and if verbosity is >= 2 the unsuccessful attempts are
 // recorded in `log` to help with debugging of the workload.
 func (og *operationGenerator) randOp(
-	ctx context.Context, tx pgx.Tx, useDeclarativeSchemaChanger, allowDML bool,
+	ctx context.Context, tx pgx.Tx, useDeclarativeSchemaChanger bool, numOpsInTxn int,
 ) (stmt *opStmt, err error) {
 	for {
 		var op opType
@@ -195,8 +195,11 @@ func (og *operationGenerator) randOp(
 		} else {
 			op = opType(og.params.ops.Int())
 		}
-		if !allowDML && isDMLOpType(op) {
-			continue
+		if numOpsInTxn != 1 {
+			// DML and legacy PK changes are only allowed in single-statement transactions.
+			if isDMLOpType(op) || (op == alterTableAlterPrimaryKey && !useDeclarativeSchemaChanger) {
+				continue
+			}
 		}
 
 		og.resetOpState(useDeclarativeSchemaChanger)
@@ -2785,6 +2788,8 @@ func (og *operationGenerator) commentOn(ctx context.Context, tx pgx.Tx) (*opStmt
 	SELECT 'INDEX ' || quote_ident(schema_name) || '.' || quote_ident(table_name) || '@' || quote_ident("index"->>'name') FROM indexes
 		UNION ALL
 	SELECT 'CONSTRAINT ' || quote_ident("constraint"->>'name') || ' ON ' || quote_ident(schema_name) || '.' || quote_ident(table_name) FROM constraints
+    -- Avoid temporary CHECK constraints created while adding NOT NULL columns.
+    WHERE "constraint"->>'name' NOT LIKE '%%auto_not_null'
 		%s`, onType))
 
 	commentables, err := Collect(ctx, og, tx, pgx.RowTo[string], q)

pkg/workload/schemachange/schemachange.go

@@ -456,12 +456,12 @@ func (w *schemaChangeWorker) runInTxn(
 ) error {
 	w.logger.startLog(w.id)
 	w.logger.writeLog("BEGIN")
-	opsNum := 1 + w.opGen.randIntn(w.maxOpsPerWorker)
-	if useDeclarativeSchemaChanger && opsNum > w.workload.declarativeSchemaMaxStmtsPerTxn {
-		opsNum = w.workload.declarativeSchemaMaxStmtsPerTxn
+	numOps := 1 + w.opGen.randIntn(w.maxOpsPerWorker)
+	if useDeclarativeSchemaChanger && numOps > w.workload.declarativeSchemaMaxStmtsPerTxn {
+		numOps = w.workload.declarativeSchemaMaxStmtsPerTxn
 	}
 
-	for i := 0; i < opsNum; i++ {
+	for i := 0; i < numOps; i++ {
 		// Terminating this loop early if there are expected commit errors prevents unexpected commit behavior from being
 		// hidden by subsequent operations. Consider the case where there are expected commit errors.
 		// It is possible that committing the transaction now will fail the workload because the error does not occur
@@ -474,8 +474,7 @@ func (w *schemaChangeWorker) runInTxn(
 			break
 		}
 
-		// Only allow DML for single-statement transactions.
-		op, err := w.opGen.randOp(ctx, tx, useDeclarativeSchemaChanger, opsNum == 1)
+		op, err := w.opGen.randOp(ctx, tx, useDeclarativeSchemaChanger, numOps)
 		if pgErr := new(pgconn.PgError); errors.As(err, &pgErr) &&
 			pgcode.MakeCode(pgErr.Code) == pgcode.SerializationFailure {
 			return errors.Mark(err, errRunInTxnRbkSentinel)