Skip to content

Commit

Permalink
sql: remove deprecated ForeachColumn-methods from TableDescriptor
Browse files Browse the repository at this point in the history
Previously, these methods would be called on a table descriptor interface
to apply a function on *descpb.ColumnDescriptor for some subset of columns.

This patch removes these calls, along with the method definitions, in favour
of new methods which use the catalog.Column interface type instead.

Fixes cockroachdb#59805.

Release note: None
  • Loading branch information
Marius Posta committed Feb 5, 2021
1 parent a4ac4d1 commit c5c0035
Show file tree
Hide file tree
Showing 10 changed files with 165 additions and 192 deletions.
4 changes: 1 addition & 3 deletions pkg/sql/catalog/descriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,7 @@ type TableDescriptor interface {
FindColumnWithID(id descpb.ColumnID) (Column, error)
FindColumnWithName(name tree.Name) (Column, error)

GetPublicColumns() []descpb.ColumnDescriptor // deprecated
ForeachPublicColumn(f func(col *descpb.ColumnDescriptor) error) error // deprecated
ForeachNonDropColumn(f func(col *descpb.ColumnDescriptor) error) error // deprecated
GetPublicColumns() []descpb.ColumnDescriptor // deprecated
NamesForColumnIDs(ids descpb.ColumnIDs) ([]string, error)
FindColumnByName(name tree.Name) (*descpb.ColumnDescriptor, bool, error) // deprecated
FindActiveColumnByID(id descpb.ColumnID) (*descpb.ColumnDescriptor, error) // deprecated
Expand Down
18 changes: 9 additions & 9 deletions pkg/sql/catalog/schemaexpr/computed_column.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,33 +137,33 @@ func (v *ComputedColumnValidator) Validate(
// computed columns being added reference the given column.
// TODO(mgartner): Add unit tests for ValidateNoDependents.
func (v *ComputedColumnValidator) ValidateNoDependents(col *descpb.ColumnDescriptor) error {
checkComputed := func(c *descpb.ColumnDescriptor) error {
for _, c := range v.desc.NonDropColumnsNew() {
if !c.IsComputed() {
return nil
continue
}

expr, err := parser.ParseExpr(*c.ComputeExpr)
expr, err := parser.ParseExpr(c.GetComputeExpr())
if err != nil {
// At this point, we should be able to parse the computed expression.
return errors.WithAssertionFailure(err)
}

return iterColDescriptors(v.desc, expr, func(colVar *descpb.ColumnDescriptor) error {
err = iterColDescriptors(v.desc, expr, func(colVar *descpb.ColumnDescriptor) error {
if colVar.ID == col.ID {
return pgerror.Newf(
pgcode.InvalidColumnReference,
"column %q is referenced by computed column %q",
col.Name,
c.Name,
c.GetName(),
)
}
return nil
})
if err != nil {
return err
}
}

return v.desc.ForeachNonDropColumn(func(col *descpb.ColumnDescriptor) error {
return checkComputed(col)
})
return nil
}

// MakeComputedExprs returns a slice of the computed expressions for the
Expand Down
29 changes: 0 additions & 29 deletions pkg/sql/catalog/tabledesc/structured.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,35 +364,6 @@ func (desc *wrapper) AllActiveAndInactiveForeignKeys() []*descpb.ForeignKeyConst
return fks
}

// ForeachPublicColumn runs a function on all public columns.
func (desc *wrapper) ForeachPublicColumn(f func(column *descpb.ColumnDescriptor) error) error {
for i := range desc.Columns {
if err := f(&desc.Columns[i]); err != nil {
return err
}
}
return nil
}

// ForeachNonDropColumn runs a function on all public columns and columns
// currently being added.
func (desc *wrapper) ForeachNonDropColumn(f func(column *descpb.ColumnDescriptor) error) error {
if err := desc.ForeachPublicColumn(f); err != nil {
return err
}

for i := range desc.Mutations {
mut := &desc.Mutations[i]
mutCol := mut.GetColumn()
if mut.Direction == descpb.DescriptorMutation_ADD && mutCol != nil {
if err := f(mutCol); err != nil {
return err
}
}
}
return nil
}

// ForeachDependedOnBy runs a function on all indexes, including those being
// added in the mutations.
func (desc *wrapper) ForeachDependedOnBy(
Expand Down
11 changes: 6 additions & 5 deletions pkg/sql/crdb_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -2346,12 +2346,13 @@ CREATE TABLE crdb_internal.backward_dependencies (
}

// Record sequence dependencies.
return table.ForeachPublicColumn(func(col *descpb.ColumnDescriptor) error {
for _, sequenceID := range col.UsesSequenceIds {
for _, col := range table.PublicColumnsNew() {
for i := 0; i < col.NumUsesSequences(); i++ {
sequenceID := col.GetUsesSequenceID(i)
if err := addRow(
tableID, tableName,
tree.DNull,
tree.NewDInt(tree.DInt(col.ID)),
tree.NewDInt(tree.DInt(col.GetID())),
tree.NewDInt(tree.DInt(sequenceID)),
sequenceDep,
tree.DNull,
Expand All @@ -2361,8 +2362,8 @@ CREATE TABLE crdb_internal.backward_dependencies (
return err
}
}
return nil
})
}
return nil
})
},
}
Expand Down
9 changes: 4 additions & 5 deletions pkg/sql/indexbackfiller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,12 +361,11 @@ INSERT INTO foo VALUES (1), (10), (100);
colIdxMap := table.ColumnIdxMap()
var valsNeeded util.FastIntSet
if idx.Primary() {
_ = table.ForeachPublicColumn(func(column *descpb.ColumnDescriptor) error {
if !column.Virtual {
valsNeeded.Add(colIdxMap.GetDefault(column.ID))
for _, column := range table.PublicColumnsNew() {
if !column.IsVirtual() {
valsNeeded.Add(colIdxMap.GetDefault(column.GetID()))
}
return nil
})
}
} else {
_ = idx.ForEachColumnID(func(id descpb.ColumnID) error {
valsNeeded.Add(colIdxMap.GetDefault(id))
Expand Down
Loading

0 comments on commit c5c0035

Please sign in to comment.