Merge #153365 #153598

153365: sql/schemachanger: implement TRUNCATE in declarative schema changer r=fqazi a=fqazi

Previously, we only supported truncate in the legacy schema changer, which meant that automatic toggling o schema_locked was not supported. This meant certain ORM tests were failing attempting to truncate tables with automatic schema locked enabled.  This patch adds support for TRUNCATE in the declarative schema changer in majority of cases, as long as the index does not have references via index hints.

Fixes: #153241
Fixes: #153248
fixes #153498
fixes #153411
fixes #153343
fixes #153333
fixes #153319
fixes #153318

Release note: None

153598: server: call WaitFor5NodeReplication in TestCheckRestartSafe r=dhartunian a=dhartunian

This test should call the more specialized method to avoid timing out when we're waiting for 5 nodes to replicate.

Epic: None
Resolves: #153523
Release note: None

Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: David Hartunian <[email protected]>

Author: 3 people

pkg/bench/rttanalysis/testdata/benchmark_expectations

@@ -120,12 +120,12 @@ exp,benchmark
 1,SystemDatabaseQueries/select_system.users_with_schema_Name
 1,SystemDatabaseQueries/select_system.users_without_schema_Name
 1,TriggerResolution/insert_into_table_with_trigger
-15,Truncate/truncate_1_column_0_rows
-15,Truncate/truncate_1_column_1_row
-15,Truncate/truncate_1_column_2_rows
-15,Truncate/truncate_2_column_0_rows
-15,Truncate/truncate_2_column_1_rows
-15,Truncate/truncate_2_column_2_rows
+18,Truncate/truncate_1_column_0_rows
+17,Truncate/truncate_1_column_1_row
+17,Truncate/truncate_1_column_2_rows
+18,Truncate/truncate_2_column_0_rows
+17,Truncate/truncate_2_column_1_rows
+17,Truncate/truncate_2_column_2_rows
 0,UDFResolution/select_from_udf
 2,UseManyRoles/use_2_roles
 2,UseManyRoles/use_50_roles

pkg/ccl/logictestccl/testdata/logic_test/partitioning

@@ -1563,6 +1563,9 @@ PARTITION y OF INDEX test.public.bar@bar_pkey  ALTER PARTITION y OF INDEX test.p
                                                  constraints = '[]',
                                                  lease_preferences = '[]'
 
+# The declarative schema changer will correctly maintain the
+# old zone configs until the index is GCed. The legacy one instantly
+# removes the zone configs, which can lead to unexpected behavior.
 query T rowsort
 WITH subzone_spans AS (
     SELECT json_array_elements(crdb_internal.pb_to_json('cockroach.config.zonepb.ZoneConfig', config) -> 'subzoneSpans') ->> 'key' AS key
@@ -1572,6 +1575,8 @@ WITH subzone_spans AS (
 SELECT crdb_internal.pretty_key(decode(key, 'base64'), 0) AS pretty_key
 FROM subzone_spans;
 ----
+/1/1
+/1/2
 /2/1
 /2/2
 

pkg/ccl/schemachangerccl/sctestbackupccl/backup_base_generated_test.go

@@ -444,7 +444,11 @@ func updateAllReplicaCounts(
 ) {
 	_, err := testCluster.ServerConn(0).Exec("ALTER RANGE default CONFIGURE ZONE USING num_replicas=$1", desiredVoters)
 	require.NoError(t, err)
-	require.NoError(t, testCluster.WaitForFullReplication())
+	if desiredVoters == 5 {
+		require.NoError(t, testCluster.WaitFor5NodeReplication())
+	} else {
+		require.NoError(t, testCluster.WaitForFullReplication())
+	}
 
 	ts0 := testCluster.Server(0)
 	stores := ts0.GetStores().(storeVisitor)

pkg/server/api_v2_test.go

@@ -338,7 +338,7 @@ func TestStoreProcedureCallStatementMetrics(t *testing.T) {
 	)
 
 	_, err := db.Exec(`		
-		CREATE TABLE tbl (id SERIAL PRIMARY KEY, t text UNIQUE) WITH (schema_locked=false);
+		CREATE TABLE tbl (id SERIAL PRIMARY KEY, t text UNIQUE);
 		INSERT INTO tbl (t) VALUES ('d');`)
 	require.NoError(t, err)
 
@@ -376,11 +376,9 @@ func TestStoreProcedureCallStatementMetrics(t *testing.T) {
 
 	// This is to show that the metrics is globally aggregated across all calls of
 	// store procedures.
-	_, err = db.Exec(`
-			TRUNCATE tbl;
-			CALL inserttbl();
-	`)
-
+	_, err = db.Exec(`TRUNCATE tbl;`)
+	require.NoError(t, err)
+	_, err = db.Exec(`CALL inserttbl();`)
 	require.NoError(t, err)
 
 	expectedStartedInsertCount += 5
@@ -569,10 +567,9 @@ func TestUDFStatementMetrics(t *testing.T) {
 
 	// This is to show that the metrics is globally aggregated across all calls of
 	// store procedures.
-	_, err = db.Exec(`
-			TRUNCATE tbl;
-			SELECT inserttbl();
-	`)
+	_, err = db.Exec(`TRUNCATE tbl;`)
+	require.NoError(t, err)
+	_, err = db.Exec(`SELECT inserttbl();`)
 	require.NoError(t, err)
 
 	expectedStartedInsertCount += 5

pkg/server/application_api/metrics_test.go

@@ -196,12 +196,19 @@ func (is *indexSplitAndScatter) getSplitPointsWithStats(
 
 // MaybeSplitIndexSpans implements the scexec.IndexSpanSplitter interface.
 func (is *indexSplitAndScatter) MaybeSplitIndexSpans(
-	ctx context.Context, table catalog.TableDescriptor, indexToBackfill catalog.Index,
+	ctx context.Context,
+	table catalog.TableDescriptor,
+	indexToBackfill catalog.Index,
+	copyIndexSource catalog.Index,
 ) error {
-	// We will always pre-split index spans if there is partitioning.
-	err := is.MaybeSplitIndexSpansForPartitioning(ctx, table, indexToBackfill)
-	if err != nil {
-		return err
+	// If we are asked to copy a source indexes splits, then there is
+	// no need split along partitioning.
+	if copyIndexSource == nil {
+		// We will always pre-split index spans if there is partitioning.
+		err := is.MaybeSplitIndexSpansForPartitioning(ctx, table, indexToBackfill)
+		if err != nil {
+			return err
+		}
 	}
 
 	const backfillSplitExpiration = time.Hour
@@ -210,7 +217,11 @@ func (is *indexSplitAndScatter) MaybeSplitIndexSpans(
 	nNodes := is.nodeDescs.GetNodeDescriptorCount()
 	nSplits := preservedSplitsMultiple * nNodes
 	var copySplitsFromIndexID descpb.IndexID
-	for _, idx := range table.ActiveIndexes() {
+	indexesToConsider := table.ActiveIndexes()
+	if copyIndexSource != nil {
+		indexesToConsider = []catalog.Index{copyIndexSource}
+	}
+	for _, idx := range indexesToConsider {
 		if idx.GetID() != indexToBackfill.GetID() &&
 			idx.CollectKeyColumnIDs().Equals(indexToBackfill.CollectKeyColumnIDs()) {
 			copySplitsFromIndexID = idx.GetID()

pkg/sql/index_split_scatter.go

@@ -245,15 +245,16 @@ ORDER BY "timestamp", info
 # Truncate a table
 ##################
 
-
+onlyif config local-mixed-25.3
 statement ok
 ALTER TABLE a SET (schema_locked=false)
 
 statement ok
 TRUNCATE TABLE a
 
+onlyif config local-mixed-25.3
 statement ok
-ALTER TABLE a RESET (schema_locked)
+ALTER TABLE a SET (schema_locked=true)
 
 query IT
 SELECT "reportingID", info::JSONB - 'Timestamp' - 'DescriptorID' - 'TxnReadTimestamp'

pkg/sql/logictest/testdata/logic_test/event_log

@@ -753,9 +753,23 @@ orders      orders_pkey          PRIMARY KEY      PRIMARY KEY (id ASC, shipment
 orders      orders_product_fkey  FOREIGN KEY      FOREIGN KEY (product) REFERENCES products(sku) ON DELETE RESTRICT ON UPDATE RESTRICT  true
 orders      valid_customer       FOREIGN KEY      FOREIGN KEY (customer) REFERENCES customers(id)                                       true
 
+skipif config local-legacy-schema-changer
+skipif config local-mixed-25.2
+skipif config local-mixed-25.3
+statement error pq: "products_upc_key" is referenced by foreign key from table "delivery"
+DROP INDEX products@products_upc_key
+
+skipif config local-legacy-schema-changer
+skipif config local-mixed-25.2
+skipif config local-mixed-25.3
+statement error pq: "products_upc_key" is referenced by foreign key from table "delivery"
+DROP INDEX products@products_upc_key RESTRICT
+
+onlyif config local-legacy-schema-changer
 statement error pq: index "products_upc_key" is in use as unique constraint
 DROP INDEX products@products_upc_key
 
+onlyif config local-legacy-schema-changer
 statement error pq: index "products_upc_key" is in use as unique constraint
 DROP INDEX products@products_upc_key RESTRICT
 

pkg/sql/logictest/testdata/logic_test/fk

@@ -38,12 +38,14 @@ SELECT * FROM kview
 5 6
 7 8
 
+onlyif config local-mixed-25.3
 statement ok
 ALTER TABLE kv SET (schema_locked=false)
 
 statement ok
 TRUNCATE TABLE kv
 
+onlyif config local-mixed-25.3
 statement ok
 ALTER TABLE kv RESET (schema_locked)
 
@@ -67,12 +69,14 @@ CREATE TABLE selfref (
   Z INT REFERENCES selfref (y)
 )
 
+onlyif config local-mixed-25.3
 statement ok
 ALTER TABLE selfref SET (schema_locked=false)
 
 statement ok
 TRUNCATE table selfref
 
+onlyif config local-mixed-25.3
 statement ok
 ALTER TABLE selfref RESET (schema_locked)
 
@@ -95,12 +99,14 @@ CREATE TABLE bar (
   FAMILY "primary" (id, description)
 );
 
+onlyif config local-mixed-25.3
 statement ok
 ALTER TABLE bar SET (schema_locked=false)
 
 statement ok
 TRUNCATE bar
 
+onlyif config local-mixed-25.3
 statement ok
 ALTER TABLE bar RESET (schema_locked)
 
@@ -138,6 +144,7 @@ SELECT get_min_t0();
 statement ok
 ALTER TABLE t0 SET (schema_locked=false)
 
+# Fallback to legacy schema changer due to references.
 statement ok
 TRUNCATE TABLE t0;
 
@@ -212,12 +219,14 @@ COMMENT ON COLUMN t.x IS '''hi''); DROP TABLE t;';
 COMMENT ON COLUMN t.z IS 'comm"en"t2';
 COMMENT ON INDEX t@i2 IS 'comm''ent3';
 
+onlyif config local-mixed-25.3
 statement ok
 ALTER TABLE t SET (schema_locked=false)
 
 statement ok
 TRUNCATE t
 
+onlyif config local-mixed-25.3
 statement ok
 ALTER TABLE t RESET (schema_locked)