diff --git a/pkg/ccl/logictestccl/testdata/logic_test/multi_region_zone_configs b/pkg/ccl/logictestccl/testdata/logic_test/multi_region_zone_configs index 372a9787a91f..3a46c1eba759 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/multi_region_zone_configs +++ b/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; diff --git a/pkg/ccl/testutilsccl/BUILD.bazel b/pkg/ccl/testutilsccl/BUILD.bazel index c15eaf1ba979..1764726e27b4 100644 --- a/pkg/ccl/testutilsccl/BUILD.bazel +++ b/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", diff --git a/pkg/ccl/testutilsccl/alter_primary_key.go b/pkg/ccl/testutilsccl/alter_primary_key.go index 08d59b10ebdb..771683d54382 100644 --- a/pkg/ccl/testutilsccl/alter_primary_key.go +++ b/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) + }) }) } diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_column.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_column.go index 5a7a8037f9d7..7241d817f4ea 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_column.go +++ b/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. diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_alter_primary_key.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_alter_primary_key.go index 7a9fdcae4766..8e64542b1389 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_alter_primary_key.go +++ b/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, diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_drop_column.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_drop_column.go index 509355f8dac6..7f8482d54dcd 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_drop_column.go +++ b/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) diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/configure_zone.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/configure_zone.go index 6502a1ef4ade..fc533bbd64af 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/configure_zone.go +++ b/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 { diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_index.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_index.go index 01ab1ea80ebd..f829156e1ad9 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_index.go +++ b/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 { diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/helpers.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/helpers.go index 6b3753eefd39..8a24ba27de3f 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/helpers.go +++ b/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);`, diff --git a/pkg/sql/schemachanger/scbuild/testdata/unimplemented_zone_cfg b/pkg/sql/schemachanger/scbuild/testdata/unimplemented_zone_cfg deleted file mode 100644 index d3e17269787e..000000000000 --- a/pkg/sql/schemachanger/scbuild/testdata/unimplemented_zone_cfg +++ /dev/null @@ -1,43 +0,0 @@ -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; ----- - -unimplemented -ALTER TABLE defaultdb.foo_index_zone_cfg ADD COLUMN j INT ----- - -unimplemented -ALTER TABLE defaultdb.foo_index_zone_cfg ADD PRIMARY KEY (i, n) ----- - -unimplemented -ALTER TABLE defaultdb.foo_index_zone_cfg DROP COLUMN k ----- - -unimplemented -DROP INDEX defaultdb.foo_index_zone_cfg@idx ----- diff --git a/pkg/sql/schemachanger/scbuild/testdata/zone_cfg b/pkg/sql/schemachanger/scbuild/testdata/zone_cfg new file mode 100644 index 000000000000..b94391945953 --- /dev/null +++ b/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} diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/op_funcs.go b/pkg/sql/schemachanger/scplan/internal/opgen/op_funcs.go index 4b02f9b25cc8..34914fcd502d 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/op_funcs.go +++ b/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 diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database_zone_config.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database_zone_config.go index d2b7d5d61800..74900e6d7444 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database_zone_config.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database_zone_config.go @@ -27,10 +27,9 @@ func init() { scpb.Status_PUBLIC, to(scpb.Status_ABSENT, emit(func(this *scpb.DatabaseZoneConfig, md *opGenContext) *scop.DiscardZoneConfig { - // If we are dropping a database with table dependencies, let the GC - // job take care of dropping this zone config -- as its table - // dependencies need this zone config. - if checkIfDescriptorHasGCDependents(this.DatabaseID, md) { + // If this belongs to a drop instead of a CONFIGURE ZONE DISCARD, let + // the GC job take care of dropping the zone config. + if checkIfZoneConfigHasGCDependents(this, md) { return nil } return &scop.DiscardZoneConfig{ diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_zone_config.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_zone_config.go index 369a6824c29a..d05e1f19a850 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_zone_config.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_zone_config.go @@ -28,7 +28,12 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_ABSENT, - emit(func(this *scpb.IndexZoneConfig) *scop.DiscardSubzoneConfig { + emit(func(this *scpb.IndexZoneConfig, md *opGenContext) *scop.DiscardSubzoneConfig { + // If this belongs to a drop instead of a CONFIGURE ZONE DISCARD, let + // the GC job take care of dropping the zone config. + if checkIfIndexHasGCDependents(this.TableID, md) { + return nil + } return &scop.DiscardSubzoneConfig{ TableID: this.TableID, Subzone: this.Subzone, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_partition_zone_config.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_partition_zone_config.go index 9139abf35c53..e2e2e61ee474 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_partition_zone_config.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_partition_zone_config.go @@ -28,7 +28,12 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_ABSENT, - emit(func(this *scpb.PartitionZoneConfig) *scop.DiscardSubzoneConfig { + emit(func(this *scpb.PartitionZoneConfig, md *opGenContext) *scop.DiscardSubzoneConfig { + // If this belongs to a drop instead of a CONFIGURE ZONE DISCARD, let + // the GC job take care of dropping the zone config. + if checkIfIndexHasGCDependents(this.TableID, md) { + return nil + } return &scop.DiscardSubzoneConfig{ TableID: this.TableID, Subzone: this.Subzone, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_zone_config.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_zone_config.go index bef346f6cef3..fa810e488978 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_zone_config.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_zone_config.go @@ -27,6 +27,11 @@ func init() { scpb.Status_PUBLIC, to(scpb.Status_ABSENT, emit(func(this *scpb.TableZoneConfig, md *opGenContext) *scop.DiscardTableZoneConfig { + // If this belongs to a drop instead of a CONFIGURE ZONE DISCARD, let + // the GC job take care of dropping the zone config. + if checkIfZoneConfigHasGCDependents(this, md) { + return nil + } return &scop.DiscardTableZoneConfig{ TableID: this.TableID, ZoneConfig: *this.ZoneConfig,