schemachanger: remove subzone fallbacks

Fixes: cockroachdb#130900

Release note: None

pkg/ccl/logictestccl/testdata/logic_test/multi_region_zone_configs

@@ -1106,9 +1106,6 @@ CREATE TABLE tbl2 (
 statement ok
 SET override_multi_region_zone_config = true;
 
-statement ok
-SET use_declarative_schema_changer = off;
-
 statement ok
 ALTER INDEX tbl2@tbl2_i_idx CONFIGURE ZONE USING num_replicas=10;
 

pkg/ccl/testutilsccl/BUILD.bazel

@@ -17,6 +17,7 @@ go_library(
         "//pkg/sql",
         "//pkg/sql/execinfra",
         "//pkg/sql/sqltestutils",
+        "//pkg/testutils",
         "//pkg/testutils/serverutils",
         "//pkg/testutils/skip",
         "//pkg/util",

pkg/ccl/testutilsccl/alter_primary_key.go

@@ -18,7 +18,9 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltestutils"
+	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
+	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/stretchr/testify/require"
 )
@@ -115,9 +117,18 @@ USE t;
 			// Insert some rows so we can interrupt inspect state during backfill.
 			require.NoError(t, sqltestutils.BulkInsertIntoTable(sqlDB, maxValue))
 
-			runCheck = true
-			_, err := sqlDB.Exec(tc.AlterQuery)
-			require.NoError(t, err)
+			testutils.RunTrueAndFalse(t, "uses-declarative-for-alter-table",
+				func(t *testing.T, useDeclarativeSchemaChangerForAlter bool) {
+					if useDeclarativeSchemaChangerForAlter {
+						skip.WithIssue(t, 136846)
+					} else {
+						_, err := sqlDB.Exec("SET CLUSTER SETTING sql.schema.force_declarative_statements = '!ALTER TABLE';")
+						require.NoError(t, err)
+					}
+					runCheck = true
+					_, err := sqlDB.Exec(tc.AlterQuery)
+					require.NoError(t, err)
+				})
 		})
 	}
 

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

@@ -44,9 +44,6 @@ func alterTableAddColumn(
 	b BuildCtx, tn *tree.TableName, tbl *scpb.Table, stmt tree.Statement, t *tree.AlterTableAddColumn,
 ) {
 	d := t.ColumnDef
-	// We don't support handling zone config related properties for tables, so
-	// throw an unsupported error.
-	fallBackIfSubZoneConfigExists(b, t, tbl.TableID)
 	fallBackIfRegionalByRowTable(b, t, tbl.TableID)
 
 	// Check column non-existence.

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

@@ -72,7 +72,6 @@ func alterPrimaryKey(
 	fallBackIfShardedIndexExists(b, t, tbl.TableID)
 	fallBackIfPartitionedIndexExists(b, t, tbl.TableID)
 	fallBackIfRegionalByRowTable(b, t.n, tbl.TableID)
-	fallBackIfSubZoneConfigExists(b, t.n, tbl.TableID)
 
 	inflatedChain := getInflatedPrimaryIndexChain(b, tbl.TableID)
 	if !haveSameIndexColsByKind(b, tbl.TableID, inflatedChain.oldSpec.primary.IndexID,

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

@@ -32,7 +32,6 @@ func alterTableDropColumn(
 	stmt tree.Statement,
 	n *tree.AlterTableDropColumn,
 ) {
-	fallBackIfSubZoneConfigExists(b, n, tbl.TableID)
 	fallBackIfRegionalByRowTable(b, n, tbl.TableID)
 	checkSafeUpdatesForDropColumn(b)
 	checkRegionalByRowColumnConflict(b, tbl, n)

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

@@ -148,12 +148,6 @@ func astToZoneConfigObject(b BuildCtx, n *tree.SetZoneConfig) (zoneConfigObject,
 		return nil, scerrors.NotImplementedErrorf(n, "referencing an index without a table "+
 			"prefix is not supported in the DSC")
 	}
-
-	if !zs.TargetsTable() {
-		return nil, scerrors.NotImplementedErrorf(n, "zone configurations on system ranges "+
-			"are not supported in the DSC")
-	}
-
 	// If this is an ALTER ALL PARTITIONS statement, fallback to the legacy schema
 	// changer.
 	if zs.TargetsPartition() && zs.StarIndex {

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

@@ -88,14 +88,15 @@ func maybeDropIndex(
 				"use DROP CONSTRAINT ... PRIMARY KEY followed by ADD CONSTRAINT ... PRIMARY KEY in a transaction",
 		))
 	}
-	// TODO (Xiang): Check if requires CCL binary for eventual zone config removal.
 	_, _, sie := scpb.FindSecondaryIndex(toBeDroppedIndexElms)
 	if sie == nil {
 		panic(errors.AssertionFailedf("programming error: cannot find secondary index element."))
 	}
-	// We don't support handling zone config related properties for tables, so
-	// throw an unsupported error.
-	fallBackIfSubZoneConfigExists(b, nil, sie.TableID)
+	// TODO(136320): If a region is being added or dropped on this database
+	// concurrently, we have to block dropping an index for RBR tables. The logic
+	// for detecting a concurrent ADD/DROP region is not yet in the DSC; falling
+	// back for now.
+	fallBackIfRegionalByRowTable(b, nil, sie.TableID)
 	panicIfSchemaChangeIsDisallowed(b.QueryByID(sie.TableID), n)
 	// Cannot drop the index if not CASCADE and a unique constraint depends on it.
 	if n.DropBehavior != tree.DropCascade && sie.IsUnique && !sie.IsCreatedExplicitly {

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

@@ -862,24 +862,6 @@ func makeSwapIndexSpec(
 	return in, temp
 }
 
-// fallBackIfSubZoneConfigExists determines if the table has a subzone
-// config. Normally this logic is used to limit index related operations,
-// since dropping indexes will need to remove entries of sub zones from
-// the zone config.
-func fallBackIfSubZoneConfigExists(b BuildCtx, n tree.NodeFormatter, id catid.DescID) {
-	{
-		tableElts := b.QueryByID(id)
-		if _, _, elem := scpb.FindIndexZoneConfig(tableElts); elem != nil {
-			panic(scerrors.NotImplementedErrorf(n,
-				"sub zone configs are not supported"))
-		}
-		if _, _, elem := scpb.FindPartitionZoneConfig(tableElts); elem != nil {
-			panic(scerrors.NotImplementedErrorf(n,
-				"sub zone configs are not supported"))
-		}
-	}
-}
-
 // ExtractColumnIDsInExpr extracts column IDs used in expr. It's similar to
 // schemaexpr.ExtractColumnIDs but this function can also extract columns
 // added in the same transaction (e.g. for `ADD COLUMN j INT CHECK (j > 0);`,

pkg/sql/schemachanger/scbuild/testdata/zone_cfg

@@ -0,0 +1,153 @@
+setup
+SET experimental_enable_unique_without_index_constraints = true;
+CREATE TABLE defaultdb.foo (
+    i INT8 PRIMARY KEY,
+    -- j gets added by the test
+    k INT8 CHECK (k > 10),
+    l INT8 NOT NULL UNIQUE,
+    m INT8 REFERENCES foo (l),
+    n UUID,
+    UNIQUE WITHOUT INDEX (n),
+    o INT -- this column can be dropped
+);
+ALTER TABLE defaultdb.foo CONFIGURE ZONE USING gc.ttlseconds=10;
+CREATE INDEX idx ON defaultdb.foo(k, l);
+CREATE TABLE defaultdb.foo_index_zone_cfg (
+    i INT8 PRIMARY KEY,
+    -- j gets added by the test
+    k INT8 CHECK (k > 10),
+    l INT8 NOT NULL UNIQUE,
+    m INT8 REFERENCES foo (l),
+    n UUID,
+    UNIQUE WITHOUT INDEX (n),
+    o INT -- this column can be dropped
+);
+CREATE INDEX idx ON defaultdb.foo_index_zone_cfg(k, l);
+ALTER INDEX defaultdb.foo_index_zone_cfg@idx CONFIGURE ZONE USING gc.ttlseconds=10;
+----
+
+build
+ALTER TABLE defaultdb.foo_index_zone_cfg ADD COLUMN j INT
+----
+- [[IndexData:{DescID: 105, IndexID: 1}, PUBLIC], PUBLIC]
+  {indexId: 1, tableId: 105}
+- [[IndexData:{DescID: 105, IndexID: 2}, PUBLIC], PUBLIC]
+  {indexId: 2, tableId: 105}
+- [[IndexData:{DescID: 105, IndexID: 3}, PUBLIC], PUBLIC]
+  {indexId: 3, tableId: 105}
+- [[TableData:{DescID: 105, ReferencedDescID: 100}, PUBLIC], PUBLIC]
+  {databaseId: 100, tableId: 105}
+- [[Column:{DescID: 105, ColumnID: 7}, PUBLIC], ABSENT]
+  {columnId: 7, tableId: 105}
+- [[ColumnName:{DescID: 105, Name: j, ColumnID: 7}, PUBLIC], ABSENT]
+  {columnId: 7, name: j, tableId: 105}
+- [[ColumnType:{DescID: 105, ColumnFamilyID: 0, ColumnID: 7, TypeName: INT8}, PUBLIC], ABSENT]
+  {columnId: 7, elementCreationMetadata: {in231OrLater: true, in243OrLater: true}, isNullable: true, tableId: 105, type: {family: IntFamily, oid: 20, width: 64}, typeName: INT8}
+- [[IndexColumn:{DescID: 105, ColumnID: 7, IndexID: 1}, PUBLIC], ABSENT]
+  {columnId: 7, indexId: 1, kind: STORED, ordinalInKind: 5, tableId: 105}
+
+unimplemented
+ALTER TABLE defaultdb.foo_index_zone_cfg ADD PRIMARY KEY (i, n)
+----
+
+build
+ALTER TABLE defaultdb.foo_index_zone_cfg DROP COLUMN k
+----
+- [[Column:{DescID: 105, ColumnID: 2}, ABSENT], PUBLIC]
+  {columnId: 2, tableId: 105}
+- [[ColumnName:{DescID: 105, Name: k, ColumnID: 2}, ABSENT], PUBLIC]
+  {columnId: 2, name: k, tableId: 105}
+- [[ColumnType:{DescID: 105, ColumnFamilyID: 0, ColumnID: 2, TypeName: INT8}, ABSENT], PUBLIC]
+  {columnId: 2, elementCreationMetadata: {in231OrLater: true, in243OrLater: true}, isNullable: true, tableId: 105, type: {family: IntFamily, oid: 20, width: 64}, typeName: INT8}
+- [[IndexColumn:{DescID: 105, ColumnID: 1, IndexID: 1}, ABSENT], PUBLIC]
+  {columnId: 1, indexId: 1, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 2, IndexID: 1}, ABSENT], PUBLIC]
+  {columnId: 2, indexId: 1, kind: STORED, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 3, IndexID: 1}, ABSENT], PUBLIC]
+  {columnId: 3, indexId: 1, kind: STORED, ordinalInKind: 1, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 4, IndexID: 1}, ABSENT], PUBLIC]
+  {columnId: 4, indexId: 1, kind: STORED, ordinalInKind: 2, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 5, IndexID: 1}, ABSENT], PUBLIC]
+  {columnId: 5, indexId: 1, kind: STORED, ordinalInKind: 3, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 6, IndexID: 1}, ABSENT], PUBLIC]
+  {columnId: 6, indexId: 1, kind: STORED, ordinalInKind: 4, tableId: 105}
+- [[PrimaryIndex:{DescID: 105, IndexID: 1, ConstraintID: 2}, ABSENT], PUBLIC]
+  {constraintId: 2, indexId: 1, isUnique: true, tableId: 105}
+- [[IndexName:{DescID: 105, Name: foo_index_zone_cfg_pkey, IndexID: 1}, ABSENT], PUBLIC]
+  {indexId: 1, name: foo_index_zone_cfg_pkey, tableId: 105}
+- [[IndexData:{DescID: 105, IndexID: 1}, ABSENT], PUBLIC]
+  {indexId: 1, tableId: 105}
+- [[IndexData:{DescID: 105, IndexID: 2}, PUBLIC], PUBLIC]
+  {indexId: 2, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 2, IndexID: 3}, ABSENT], PUBLIC]
+  {columnId: 2, indexId: 3, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 3, IndexID: 3}, ABSENT], PUBLIC]
+  {columnId: 3, indexId: 3, ordinalInKind: 1, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 1, IndexID: 3}, ABSENT], PUBLIC]
+  {columnId: 1, indexId: 3, kind: KEY_SUFFIX, tableId: 105}
+- [[SecondaryIndex:{DescID: 105, IndexID: 3, ConstraintID: 0, RecreateSourceIndexID: 0}, ABSENT], PUBLIC]
+  {indexId: 3, isCreatedExplicitly: true, tableId: 105}
+- [[IndexName:{DescID: 105, Name: idx, IndexID: 3}, ABSENT], PUBLIC]
+  {indexId: 3, name: idx, tableId: 105}
+- [[IndexData:{DescID: 105, IndexID: 3}, ABSENT], PUBLIC]
+  {indexId: 3, tableId: 105}
+- [[CheckConstraint:{DescID: 105, IndexID: 0, ConstraintID: 4, ReferencedColumnIDs: [2]}, ABSENT], PUBLIC]
+  {columnIds: [2], constraintId: 4, expr: 'k > 10:::INT8', referencedColumnIds: [2], tableId: 105}
+- [[IndexZoneConfig:{DescID: 105, IndexID: 3, SeqNum: 0}, ABSENT], PUBLIC]
+  {indexId: 3, oldIdxRef: -1, subzone: {config: {gc: {ttlSeconds: 10}, inheritedConstraints: true, inheritedLeasePreferences: true}, indexId: 3}, subzoneSpans: [{key: iw==}], tableId: 105}
+- [[TableData:{DescID: 105, ReferencedDescID: 100}, PUBLIC], PUBLIC]
+  {databaseId: 100, tableId: 105}
+- [[PrimaryIndex:{DescID: 105, IndexID: 5, ConstraintID: 6, TemporaryIndexID: 6, SourceIndexID: 1}, PUBLIC], ABSENT]
+  {constraintId: 6, indexId: 5, isUnique: true, sourceIndexId: 1, tableId: 105, temporaryIndexId: 6}
+- [[IndexName:{DescID: 105, Name: foo_index_zone_cfg_pkey, IndexID: 5}, PUBLIC], ABSENT]
+  {indexId: 5, name: foo_index_zone_cfg_pkey, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 1, IndexID: 5}, PUBLIC], ABSENT]
+  {columnId: 1, indexId: 5, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 3, IndexID: 5}, PUBLIC], ABSENT]
+  {columnId: 3, indexId: 5, kind: STORED, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 4, IndexID: 5}, PUBLIC], ABSENT]
+  {columnId: 4, indexId: 5, kind: STORED, ordinalInKind: 1, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 5, IndexID: 5}, PUBLIC], ABSENT]
+  {columnId: 5, indexId: 5, kind: STORED, ordinalInKind: 2, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 6, IndexID: 5}, PUBLIC], ABSENT]
+  {columnId: 6, indexId: 5, kind: STORED, ordinalInKind: 3, tableId: 105}
+- [[IndexData:{DescID: 105, IndexID: 5}, PUBLIC], ABSENT]
+  {indexId: 5, tableId: 105}
+- [[TemporaryIndex:{DescID: 105, IndexID: 6, ConstraintID: 7, SourceIndexID: 1}, TRANSIENT_ABSENT], ABSENT]
+  {constraintId: 7, indexId: 6, isUnique: true, sourceIndexId: 1, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 1, IndexID: 6}, TRANSIENT_ABSENT], ABSENT]
+  {columnId: 1, indexId: 6, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 3, IndexID: 6}, TRANSIENT_ABSENT], ABSENT]
+  {columnId: 3, indexId: 6, kind: STORED, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 4, IndexID: 6}, TRANSIENT_ABSENT], ABSENT]
+  {columnId: 4, indexId: 6, kind: STORED, ordinalInKind: 1, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 5, IndexID: 6}, TRANSIENT_ABSENT], ABSENT]
+  {columnId: 5, indexId: 6, kind: STORED, ordinalInKind: 2, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 6, IndexID: 6}, TRANSIENT_ABSENT], ABSENT]
+  {columnId: 6, indexId: 6, kind: STORED, ordinalInKind: 3, tableId: 105}
+- [[IndexData:{DescID: 105, IndexID: 6}, TRANSIENT_ABSENT], ABSENT]
+  {indexId: 6, tableId: 105}
+
+build
+DROP INDEX defaultdb.foo_index_zone_cfg@idx
+----
+- [[IndexData:{DescID: 105, IndexID: 1}, PUBLIC], PUBLIC]
+  {indexId: 1, tableId: 105}
+- [[IndexData:{DescID: 105, IndexID: 2}, PUBLIC], PUBLIC]
+  {indexId: 2, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 2, IndexID: 3}, ABSENT], PUBLIC]
+  {columnId: 2, indexId: 3, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 3, IndexID: 3}, ABSENT], PUBLIC]
+  {columnId: 3, indexId: 3, ordinalInKind: 1, tableId: 105}
+- [[IndexColumn:{DescID: 105, ColumnID: 1, IndexID: 3}, ABSENT], PUBLIC]
+  {columnId: 1, indexId: 3, kind: KEY_SUFFIX, tableId: 105}
+- [[SecondaryIndex:{DescID: 105, IndexID: 3, ConstraintID: 0, RecreateSourceIndexID: 0}, ABSENT], PUBLIC]
+  {indexId: 3, isCreatedExplicitly: true, tableId: 105}
+- [[IndexName:{DescID: 105, Name: idx, IndexID: 3}, ABSENT], PUBLIC]
+  {indexId: 3, name: idx, tableId: 105}
+- [[IndexData:{DescID: 105, IndexID: 3}, ABSENT], PUBLIC]
+  {indexId: 3, tableId: 105}
+- [[IndexZoneConfig:{DescID: 105, IndexID: 3, SeqNum: 0}, ABSENT], PUBLIC]
+  {indexId: 3, oldIdxRef: -1, subzone: {config: {gc: {ttlSeconds: 10}, inheritedConstraints: true, inheritedLeasePreferences: true}, indexId: 3}, subzoneSpans: [{key: iw==}], tableId: 105}
+- [[TableData:{DescID: 105, ReferencedDescID: 100}, PUBLIC], PUBLIC]
+  {databaseId: 100, tableId: 105}

pkg/sql/schemachanger/scplan/internal/opgen/op_funcs.go

@@ -192,14 +192,55 @@ func checkIfDescriptorIsWithoutData(id descpb.ID, md *opGenContext) bool {
 	return !doesDescriptorHaveData
 }
 
-// checkIfDescriptorHasGCDependents will determine if a descriptor has data
-// dependencies it still needs to GC. This allows us to determine when we can
-// need to skip certain operations like deleting a zone config.
-func checkIfDescriptorHasGCDependents(_ descpb.ID, md *opGenContext) bool {
-	for _, t := range md.Targets {
-		switch t.Element().(type) {
-		case *scpb.IndexData, *scpb.TableData:
-			return true
+// checkIfZoneConfigHasGCDependents will determine if a table/database
+// descriptor has data dependencies it still needs to GC. This allows us to
+// determine when we need to skip certain operations like deleting a zone
+// config.
+func checkIfZoneConfigHasGCDependents(elem scpb.ZoneConfigElement, md *opGenContext) bool {
+	isValidTableData := func(td *scpb.TableData) bool {
+		switch e := elem.(type) {
+		case *scpb.DatabaseZoneConfig:
+			return e.DatabaseID == td.DatabaseID
+		case *scpb.TableZoneConfig:
+			return e.TableID == td.TableID
+		default:
+			panic(errors.AssertionFailedf(
+				"element type %T not allowed for checkIfZoneConfigHasGCDependents", e))
+		}
+	}
+	for idx, t := range md.Targets {
+		switch e := t.Element().(type) {
+		case *scpb.TableData:
+			// Filter out any elements we do not want to consider.
+			if !isValidTableData(e) {
+				continue
+			}
+			// Check if this descriptor has any data marked for an absent state.
+			if t.TargetStatus == scpb.Status_ABSENT &&
+				md.Initial[idx] == scpb.Status_PUBLIC {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// checkIfIndexHasGCDependents is like checkIfZoneConfigHasGCDependents, but
+// for indexes. We also ensure that we filter out irrelevant indexes here.
+func checkIfIndexHasGCDependents(tableID descpb.ID, md *opGenContext) bool {
+	for idx, t := range md.Targets {
+		switch e := t.Element().(type) {
+		case *scpb.IndexData:
+			// Validate this is the table ID we are
+			// looking for.
+			if e.TableID != tableID {
+				continue
+			}
+			// Check if this descriptor has any data marked for an absent state.
+			if t.TargetStatus == scpb.Status_ABSENT &&
+				md.Initial[idx] == scpb.Status_PUBLIC {
+				return true
+			}
 		}
 	}
 	return false