Skip to content

Commit a5c44e1

Browse files
committed
schemachanger: remove deprecated SecondaryIndexPartial element
This element was deprecated in 64f20d8 and a migration was added so it's no longer referenced anywhere. We can safely remove all existing usages in the code, since that migration occurred in v23.1. Release note: None
1 parent 92aa0df commit a5c44e1

File tree

24 files changed

+140
-1066
lines changed

24 files changed

+140
-1066
lines changed

pkg/cli/testdata/declarative-rules/deprules

Lines changed: 23 additions & 143 deletions
Large diffs are not rendered by default.

pkg/sql/catalog/redact/redact.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,11 @@ func redactElement(element scpb.Element) error {
151151
e.PhysicalRepresentation = []byte("_")
152152
case *scpb.IndexPartitioning:
153153
redactPartitioning(&e.PartitioningDescriptor)
154-
case *scpb.SecondaryIndexPartial:
155-
return redactExpr(&e.Expression.Expr)
154+
case *scpb.SecondaryIndex:
155+
if e.EmbeddedExpr != nil {
156+
return redactExpr(&e.EmbeddedExpr.Expr)
157+
}
158+
return nil
156159
case *scpb.CheckConstraint:
157160
return redactExpr(&e.Expression.Expr)
158161
case *scpb.ColumnDefaultExpression:

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,16 +1016,6 @@ func shouldCreateUniqueIndexOnOldPrimaryKeyColumns(
10161016
}
10171017

10181018
func isIndexPartial(b BuildCtx, tableID catid.DescID, indexID catid.IndexID) (ret bool) {
1019-
scpb.ForEachSecondaryIndexPartial(b.QueryByID(tableID), func(
1020-
current scpb.Status, target scpb.TargetStatus, e *scpb.SecondaryIndexPartial,
1021-
) {
1022-
if e.IndexID == indexID {
1023-
ret = true
1024-
}
1025-
})
1026-
if ret {
1027-
return ret
1028-
}
10291019
scpb.ForEachSecondaryIndex(b.QueryByID(tableID), func(
10301020
current scpb.Status, target scpb.TargetStatus, e *scpb.SecondaryIndex,
10311021
) {

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -330,8 +330,6 @@ func walkColumnDependencies(
330330
sequenceDeps.Add(elt.SequenceID)
331331
case *scpb.SecondaryIndex:
332332
indexDeps.Add(elt.IndexID)
333-
case *scpb.SecondaryIndexPartial:
334-
indexDeps.Add(elt.IndexID)
335333
case *scpb.IndexColumn:
336334
indexDeps.Add(elt.IndexID)
337335
case *scpb.ForeignKeyConstraint:
@@ -434,10 +432,6 @@ func panicIfColReferencedInPredicate(
434432
if elt.EmbeddedExpr != nil && contains(elt.EmbeddedExpr.ReferencedColumnIDs, col.ColumnID) {
435433
violatingIndex = elt.IndexID
436434
}
437-
case *scpb.SecondaryIndexPartial:
438-
if contains(elt.ReferencedColumnIDs, col.ColumnID) {
439-
violatingIndex = elt.IndexID
440-
}
441435
case *scpb.UniqueWithoutIndexConstraint:
442436
if elt.Predicate != nil && contains(elt.Predicate.ReferencedColumnIDs, col.ColumnID) {
443437
violatingUWI = elt.ConstraintID

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -519,11 +519,11 @@ func isIndexUniqueAndCanServeFK(
519519
}
520520

521521
isPartial := false
522-
scpb.ForEachSecondaryIndexPartial(b.QueryByID(ie.TableID), func(
523-
current scpb.Status, target scpb.TargetStatus, sipe *scpb.SecondaryIndexPartial,
522+
scpb.ForEachSecondaryIndex(b.QueryByID(ie.TableID), func(
523+
current scpb.Status, target scpb.TargetStatus, sie *scpb.SecondaryIndex,
524524
) {
525-
if sipe.TableID == ie.TableID && sipe.IndexID == ie.IndexID {
526-
isPartial = true
525+
if sie.TableID == ie.TableID && sie.IndexID == ie.IndexID {
526+
isPartial = sie.EmbeddedExpr != nil
527527
}
528528
})
529529
if isPartial {

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

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ func dropCascadeDescriptor(b BuildCtx, id catid.DescID) {
239239
dropCascadeDescriptor(next, t.TableID)
240240
case *scpb.PolicyDeps:
241241
dropCascadeDescriptor(next, t.TableID)
242-
case *scpb.Column, *scpb.ColumnType, *scpb.SecondaryIndexPartial:
242+
case *scpb.Column, *scpb.ColumnType:
243243
// These only have type references.
244244
break
245245
case *scpb.Namespace, *scpb.Function, *scpb.SecondaryIndex, *scpb.PrimaryIndex,
@@ -545,7 +545,6 @@ type indexSpec struct {
545545
temporary *scpb.TemporaryIndex
546546

547547
name *scpb.IndexName
548-
partial *scpb.SecondaryIndexPartial
549548
partitioning *scpb.IndexPartitioning
550549
columns []*scpb.IndexColumn
551550
idxComment *scpb.IndexComment
@@ -602,9 +601,6 @@ func (s *indexSpec) apply(fn func(e scpb.Element)) {
602601
if s.name != nil {
603602
fn(s.name)
604603
}
605-
if s.partial != nil {
606-
fn(s.partial)
607-
}
608604
if s.partitioning != nil {
609605
fn(s.partitioning)
610606
}
@@ -642,9 +638,6 @@ func (s *indexSpec) clone() (c indexSpec) {
642638
if s.name != nil {
643639
c.name = protoutil.Clone(s.name).(*scpb.IndexName)
644640
}
645-
if s.partial != nil {
646-
c.partial = protoutil.Clone(s.partial).(*scpb.SecondaryIndexPartial)
647-
}
648641
if s.partitioning != nil {
649642
c.partitioning = protoutil.Clone(s.partitioning).(*scpb.IndexPartitioning)
650643
}
@@ -852,7 +845,6 @@ func makeIndexSpec(b BuildCtx, tableID catid.DescID, indexID catid.IndexID) (s i
852845
tableID, indexID, s.primary != nil, s.secondary != nil, s.temporary != nil))
853846
}
854847
_, _, s.name = scpb.FindIndexName(idxElts)
855-
_, _, s.partial = scpb.FindSecondaryIndexPartial(idxElts)
856848
_, _, s.partitioning = scpb.FindIndexPartitioning(idxElts)
857849
scpb.ForEachIndexColumn(idxElts, func(_ scpb.Status, _ scpb.TargetStatus, ic *scpb.IndexColumn) {
858850
s.columns = append(s.columns, ic)
@@ -904,9 +896,6 @@ func makeTempIndexSpec(src indexSpec) indexSpec {
904896
if newTempSpec.partitioning != nil {
905897
newTempSpec.partitioning.IndexID = tempID
906898
}
907-
if newTempSpec.partial != nil {
908-
newTempSpec.partial.IndexID = tempID
909-
}
910899
for _, ic := range newTempSpec.columns {
911900
ic.IndexID = tempID
912901
}
@@ -1008,9 +997,6 @@ func makeSwapIndexSpec(
1008997
if in.name != nil {
1009998
in.name.IndexID = inID
1010999
}
1011-
if in.partial != nil {
1012-
in.partial.IndexID = inID
1013-
}
10141000
if in.partitioning != nil {
10151001
in.partitioning.IndexID = inID
10161002
}

pkg/sql/schemachanger/scpb/element_collection_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,15 +98,15 @@ func TestElementCollection(t *testing.T) {
9898
t.Run("predicates", func(t *testing.T) {
9999
require.False(t, c.IsEmpty())
100100
require.False(t, c.FilterSchema().IsEmpty())
101-
require.True(t, c.FilterSecondaryIndexPartial().IsEmpty())
101+
require.True(t, c.FilterSecondaryIndex().IsEmpty())
102102

103103
require.False(t, c.IsSingleton())
104104
require.True(t, c.FilterSchema().IsSingleton())
105-
require.False(t, c.FilterSecondaryIndexPartial().IsSingleton())
105+
require.False(t, c.FilterSecondaryIndex().IsSingleton())
106106

107107
require.True(t, c.IsMany())
108108
require.False(t, c.FilterSchema().IsMany())
109-
require.False(t, c.FilterSecondaryIndexPartial().IsMany())
109+
require.False(t, c.FilterSecondaryIndex().IsMany())
110110
})
111111
t.Run("assertions", func(t *testing.T) {
112112
e, err := recoverFromPanic(c.MustGetZeroOrOneElement)
@@ -123,10 +123,10 @@ func TestElementCollection(t *testing.T) {
123123
require.NoError(t, err)
124124
require.NotNil(t, e)
125125

126-
e, err = recoverFromPanic(c.FilterSecondaryIndexPartial().MustGetZeroOrOneElement)
126+
e, err = recoverFromPanic(c.FilterSecondaryIndex().MustGetZeroOrOneElement)
127127
require.NoError(t, err)
128128
require.Nil(t, e)
129-
e, err = recoverFromPanic(c.FilterSecondaryIndexPartial().MustGetOneElement)
129+
e, err = recoverFromPanic(c.FilterSecondaryIndex().MustGetOneElement)
130130
require.Error(t, err)
131131
require.Nil(t, e)
132132
})

pkg/sql/schemachanger/scpb/elements.proto

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ message ElementProto {
115115
// Index elements.
116116
IndexName index_name = 40 [(gogoproto.moretags) = "parent:\"PrimaryIndex, SecondaryIndex\""];
117117
IndexPartitioning index_partitioning = 41 [(gogoproto.moretags) = "parent:\"PrimaryIndex, SecondaryIndex\""];
118-
SecondaryIndexPartial secondary_index_partial = 42 [(gogoproto.moretags) = "parent:\"SecondaryIndex\""];
119118
IndexComment index_comment = 43 [(gogoproto.moretags) = "parent:\"PrimaryIndex, SecondaryIndex\""];
120119
IndexColumn index_column = 44 [(gogoproto.moretags) = "parent:\"PrimaryIndex, SecondaryIndex, TemporaryIndex, Column\""];
121120
IndexData index_data = 45 [(gogoproto.customname) = "IndexData", (gogoproto.moretags) = "parent:\"PrimaryIndex, SecondaryIndex, TemporaryIndex\""];
@@ -180,6 +179,9 @@ message ElementProto {
180179

181180
// Next element group start id: 250
182181
}
182+
183+
// Reserved for the now-removed SecondaryIndexPartial element.
184+
reserved 42;
183185
}
184186

185187
// TypeT is a wrapper for a types.T which contains its user-defined type ID
@@ -358,13 +360,6 @@ message TemporaryIndex {
358360
Expression expr = 3 [(gogoproto.nullable) = true];
359361
}
360362

361-
message SecondaryIndexPartial {
362-
uint32 table_id = 1 [(gogoproto.customname) = "TableID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"];
363-
uint32 index_id = 2 [(gogoproto.customname) = "IndexID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.IndexID"];
364-
Expression embedded_expr = 3 [(gogoproto.nullable) = false, (gogoproto.embed) = true];
365-
reserved 10;
366-
}
367-
368363
// SchemaParent models the schema to parent database relationship.
369364
// Every schema has a parent, so there is a 1:1 relationship between
370365
// the Schema and the SchemaParent relationship. This is modeled as a separate

pkg/sql/schemachanger/scpb/elements_generated.go

Lines changed: 0 additions & 41 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/sql/schemachanger/scpb/migration.go

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
// HasDeprecatedElements returns if the target contains any element or fields
1717
// marked for deprecation.
1818
func HasDeprecatedElements(version clusterversion.ClusterVersion, target Target) bool {
19-
return target.GetSecondaryIndexPartial() != nil
19+
return false
2020
}
2121

2222
// migrateDeprecatedFields will check if any of the deprecated fields are being
@@ -82,16 +82,9 @@ func migrateDeprecatedFields(
8282
func migrateTargetElement(targets []Target, idx int) {
8383
targetToMigrate := targets[idx]
8484
switch t := targetToMigrate.Element().(type) {
85-
case *SecondaryIndexPartial:
86-
for _, target := range targets {
87-
if secondaryIndex := target.GetSecondaryIndex(); secondaryIndex != nil &&
88-
secondaryIndex.TableID == t.TableID &&
89-
secondaryIndex.IndexID == t.IndexID &&
90-
target.TargetStatus == targetToMigrate.TargetStatus {
91-
secondaryIndex.EmbeddedExpr = &t.Expression
92-
break
93-
}
94-
}
85+
default:
86+
// No-op case to defeat unused linter when there are no elements to migrate.
87+
_ = t.element
9588
}
9689
}
9790

0 commit comments

Comments
 (0)