sql: remove deprecated GetPublicColumns method from TableDescriptor

Previously, this method would be called on a table descriptor interface
to return a []descpb.ColumnDescriptor value containing all public
columns.

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

Fixes cockroachdb#59805.

Release note: None

pkg/ccl/backupccl/backup_test.go

@@ -5216,13 +5216,13 @@ func TestBackupRestoreSequenceOwnership(t *testing.T) {
  require.Equal(t, tableDesc.GetID(), seqDesc.GetSequenceOpts().SequenceOwner.OwnerTableID,
  "unexpected table is sequence owner after restore",
  )
- require.Equal(t, tableDesc.GetPublicColumns()[0].ID, seqDesc.GetSequenceOpts().SequenceOwner.OwnerColumnID,
+ require.Equal(t, tableDesc.PublicColumnsNew()[0].GetID(), seqDesc.GetSequenceOpts().SequenceOwner.OwnerColumnID,
  "unexpected column is sequence owner after restore",
  )
- require.Equal(t, 1, len(tableDesc.GetPublicColumns()[0].OwnsSequenceIds),
+ require.Equal(t, 1, tableDesc.PublicColumnsNew()[0].NumOwnsSequences(),
  "unexpected number of sequences owned by d.t after restore",
  )
- require.Equal(t, seqDesc.GetID(), tableDesc.GetPublicColumns()[0].OwnsSequenceIds[0],
+ require.Equal(t, seqDesc.GetID(), tableDesc.PublicColumnsNew()[0].GetOwnsSequenceID(0),
  "unexpected ID of sequence owned by table d.t after restore",
  )
  })
@@ -5269,7 +5269,7 @@ func TestBackupRestoreSequenceOwnership(t *testing.T) {
 
  tableDesc := catalogkv.TestingGetTableDescriptor(kvDB, keys.SystemSQLCodec, "d", "t")
 
- require.Equal(t, 0, len(tableDesc.GetPublicColumns()[0].OwnsSequenceIds),
+ require.Equal(t, 0, tableDesc.PublicColumnsNew()[0].NumOwnsSequences(),
  "expected restored table to own 0 sequences",
  )
 
@@ -5300,13 +5300,13 @@ func TestBackupRestoreSequenceOwnership(t *testing.T) {
  require.Equal(t, tableDesc.GetID(), seqDesc.GetSequenceOpts().SequenceOwner.OwnerTableID,
  "unexpected table is sequence owner after restore",
  )
- require.Equal(t, tableDesc.GetPublicColumns()[0].ID, seqDesc.GetSequenceOpts().SequenceOwner.OwnerColumnID,
+ require.Equal(t, tableDesc.PublicColumnsNew()[0].GetID(), seqDesc.GetSequenceOpts().SequenceOwner.OwnerColumnID,
  "unexpected column is sequence owner after restore",
  )
- require.Equal(t, 1, len(tableDesc.GetPublicColumns()[0].OwnsSequenceIds),
+ require.Equal(t, 1, tableDesc.PublicColumnsNew()[0].NumOwnsSequences(),
  "unexpected number of sequences owned by d.t after restore",
  )
- require.Equal(t, seqDesc.GetID(), tableDesc.GetPublicColumns()[0].OwnsSequenceIds[0],
+ require.Equal(t, seqDesc.GetID(), tableDesc.PublicColumnsNew()[0].GetOwnsSequenceID(0),
  "unexpected ID of sequence owned by table d.t after restore",
  )
  })
@@ -5345,7 +5345,7 @@ func TestBackupRestoreSequenceOwnership(t *testing.T) {
  newDB.Exec(t, `RESTORE DATABASE d2 FROM $1 WITH skip_missing_sequence_owners`, backupLocD2D3)
 
  tableDesc := catalogkv.TestingGetTableDescriptor(kvDB, keys.SystemSQLCodec, "d2", "t")
- require.Equal(t, 0, len(tableDesc.GetPublicColumns()[0].OwnsSequenceIds),
+ require.Equal(t, 0, tableDesc.PublicColumnsNew()[0].NumOwnsSequences(),
  "expected restored table to own no sequences.",
  )
 
@@ -5365,12 +5365,12 @@ func TestBackupRestoreSequenceOwnership(t *testing.T) {
  require.Equal(t, td.GetID(), sd.GetSequenceOpts().SequenceOwner.OwnerTableID,
  "unexpected table owner for sequence seq2 after restore",
  )
- require.Equal(t, td.GetPublicColumns()[0].ID, sd.GetSequenceOpts().SequenceOwner.OwnerColumnID,
+ require.Equal(t, td.PublicColumnsNew()[0].GetID(), sd.GetSequenceOpts().SequenceOwner.OwnerColumnID,
  "unexpected column owner for sequence seq2 after restore")
- require.Equal(t, 1, len(td.GetPublicColumns()[0].OwnsSequenceIds),
+ require.Equal(t, 1, td.PublicColumnsNew()[0].NumOwnsSequences(),
  "unexpected number of sequences owned by d3.t after restore",
  )
- require.Equal(t, sd.GetID(), td.GetPublicColumns()[0].OwnsSequenceIds[0],
+ require.Equal(t, sd.GetID(), td.PublicColumnsNew()[0].GetOwnsSequenceID(0),
  "unexpected ID of sequences owned by d3.t",
  )
  })
@@ -5394,13 +5394,13 @@ func TestBackupRestoreSequenceOwnership(t *testing.T) {
  require.Equal(t, tableDesc.GetID(), seqDesc.GetSequenceOpts().SequenceOwner.OwnerTableID,
  "unexpected table is sequence owner after restore",
  )
- require.Equal(t, tableDesc.GetPublicColumns()[0].ID, seqDesc.GetSequenceOpts().SequenceOwner.OwnerColumnID,
+ require.Equal(t, tableDesc.PublicColumnsNew()[0].GetID(), seqDesc.GetSequenceOpts().SequenceOwner.OwnerColumnID,
  "unexpected column is sequence owner after restore",
  )
- require.Equal(t, 1, len(tableDesc.GetPublicColumns()[0].OwnsSequenceIds),
+ require.Equal(t, 1, tableDesc.PublicColumnsNew()[0].NumOwnsSequences(),
  "unexpected number of sequences owned by d.t after restore",
  )
- require.Equal(t, seqDesc.GetID(), tableDesc.GetPublicColumns()[0].OwnsSequenceIds[0],
+ require.Equal(t, seqDesc.GetID(), tableDesc.PublicColumnsNew()[0].GetOwnsSequenceID(0),
  "unexpected ID of sequence owned by table d.t after restore",
  )
 
@@ -5419,20 +5419,20 @@ func TestBackupRestoreSequenceOwnership(t *testing.T) {
  "unexpected table is sequence owner of d3.seq2 after restore",
  )
 
- require.Equal(t, td.GetPublicColumns()[0].ID, sd.GetSequenceOpts().SequenceOwner.OwnerColumnID,
+ require.Equal(t, td.PublicColumnsNew()[0].GetID(), sd.GetSequenceOpts().SequenceOwner.OwnerColumnID,
  "unexpected column is sequence owner of d2.seq after restore",
  )
- require.Equal(t, td.GetPublicColumns()[0].ID, sdSeq2.GetSequenceOpts().SequenceOwner.OwnerColumnID,
+ require.Equal(t, td.PublicColumnsNew()[0].GetID(), sdSeq2.GetSequenceOpts().SequenceOwner.OwnerColumnID,
  "unexpected column is sequence owner of d3.seq2 after restore",
  )
 
- require.Equal(t, 2, len(td.GetPublicColumns()[0].OwnsSequenceIds),
+ require.Equal(t, 2, td.PublicColumnsNew()[0].NumOwnsSequences(),
  "unexpected number of sequences owned by d3.t after restore",
  )
- require.Equal(t, sd.GetID(), td.GetPublicColumns()[0].OwnsSequenceIds[0],
+ require.Equal(t, sd.GetID(), td.PublicColumnsNew()[0].GetOwnsSequenceID(0),
  "unexpected ID of sequence owned by table d3.t after restore",
  )
- require.Equal(t, sdSeq2.GetID(), td.GetPublicColumns()[0].OwnsSequenceIds[1],
+ require.Equal(t, sdSeq2.GetID(), td.PublicColumnsNew()[0].GetOwnsSequenceID(1),
  "unexpected ID of sequence owned by table d3.t after restore",
  )
  })

pkg/ccl/changefeedccl/encoder.go

@@ -173,16 +173,15 @@ func (e *jsonEncoder) EncodeValue(_ context.Context, row encodeRow) ([]byte, err
 
  var after map[string]interface{}
  if !row.deleted {
- columns := row.tableDesc.GetPublicColumns()
+ columns := row.tableDesc.PublicColumnsNew()
  after = make(map[string]interface{}, len(columns))
- for i := range columns {
- col := &columns[i]
+ for i, col := range columns {
  datum := row.datums[i]
- if err := datum.EnsureDecoded(col.Type, &e.alloc); err != nil {
+ if err := datum.EnsureDecoded(col.GetType(), &e.alloc); err != nil {
  return nil, err
  }
  var err error
- after[col.Name], err = tree.AsJSON(datum.Datum, time.UTC)
+ after[col.GetName()], err = tree.AsJSON(datum.Datum, time.UTC)
  if err != nil {
  return nil, err
  }
@@ -191,16 +190,15 @@ func (e *jsonEncoder) EncodeValue(_ context.Context, row encodeRow) ([]byte, err
 
  var before map[string]interface{}
  if row.prevDatums != nil && !row.prevDeleted {
- columns := row.prevTableDesc.GetPublicColumns()
+ columns := row.prevTableDesc.PublicColumnsNew()
  before = make(map[string]interface{}, len(columns))
- for i := range columns {
- col := &columns[i]
+ for i, col := range columns {
  datum := row.prevDatums[i]
- if err := datum.EnsureDecoded(col.Type, &e.alloc); err != nil {
+ if err := datum.EnsureDecoded(col.GetType(), &e.alloc); err != nil {
  return nil, err
  }
  var err error
- before[col.Name], err = tree.AsJSON(datum.Datum, time.UTC)
+ before[col.GetName()], err = tree.AsJSON(datum.Datum, time.UTC)
  if err != nil {
  return nil, err
  }

pkg/ccl/changefeedccl/rowfetcher_cache.go

@@ -154,12 +154,24 @@ func (c *rowFetcherCache) RowFetcherForTableDesc(
  // TODO(dan): Allow for decoding a subset of the columns.
  var colIdxMap catalog.TableColMap
  var valNeededForCol util.FastIntSet
- for colIdx := range tableDesc.GetPublicColumns() {
- colIdxMap.Set(tableDesc.GetPublicColumns()[colIdx].ID, colIdx)
- valNeededForCol.Add(colIdx)
+ for _, col := range tableDesc.PublicColumnsNew() {
+ colIdxMap.Set(col.GetID(), col.Ordinal())
+ valNeededForCol.Add(col.Ordinal())
  }
 
  var rf row.Fetcher
+ rfArgs := row.FetcherTableArgs{
+ Spans: tableDesc.AllIndexSpans(c.codec),
+ Desc: tableDesc,
+ Index: tableDesc.GetPrimaryIndex().IndexDesc(),
+ ColIdxMap: colIdxMap,
+ IsSecondaryIndex: false,
+ Cols: make([]descpb.ColumnDescriptor, len(tableDesc.PublicColumnsNew())),
+ ValNeededForCol: valNeededForCol,
+ }
+ for i, col := range tableDesc.PublicColumnsNew() {
+ rfArgs.Cols[i] = *col.ColumnDesc()
+ }
  if err := rf.Init(
  context.TODO(),
  c.codec,
@@ -169,15 +181,7 @@ func (c *rowFetcherCache) RowFetcherForTableDesc(
  false, /* isCheck */
  &c.a,
  nil, /* memMonitor */
- row.FetcherTableArgs{
- Spans: tableDesc.AllIndexSpans(c.codec),
- Desc: tableDesc,
- Index: tableDesc.GetPrimaryIndex().IndexDesc(),
- ColIdxMap: colIdxMap,
- IsSecondaryIndex: false,
- Cols: tableDesc.GetPublicColumns(),
- ValNeededForCol: valNeededForCol,
- },
+ rfArgs,
  ); err != nil {
  return nil, err
  }

pkg/ccl/changefeedccl/schemafeed/table_event_filter.go

@@ -108,12 +108,12 @@ func dropColumnMutationExists(desc catalog.TableDescriptor) bool {
 func newColumnBackfillComplete(e TableEvent) (res bool) {
  // TODO(ajwerner): What is the case where the before has a backfill mutation
  // and the After doesn't? What about other queued mutations?
- return len(e.Before.GetPublicColumns()) < len(e.After.GetPublicColumns()) &&
+ return len(e.Before.PublicColumnsNew()) < len(e.After.PublicColumnsNew()) &&
  e.Before.HasColumnBackfillMutation() && !e.After.HasColumnBackfillMutation()
 }
 
 func newColumnNoBackfill(e TableEvent) (res bool) {
- return len(e.Before.GetPublicColumns()) < len(e.After.GetPublicColumns()) &&
+ return len(e.Before.PublicColumnsNew()) < len(e.After.PublicColumnsNew()) &&
  !e.Before.HasColumnBackfillMutation()
 }
 

pkg/server/BUILD.bazel

@@ -92,6 +92,7 @@ go_library(
  "//pkg/settings",
  "//pkg/settings/cluster",
  "//pkg/sql",
+ "//pkg/sql/catalog",
  "//pkg/sql/catalog/bootstrap",
  "//pkg/sql/catalog/catconstants",
  "//pkg/sql/catalog/colinfo",
@@ -111,7 +112,6 @@ go_library(
  "//pkg/sql/physicalplan",
  "//pkg/sql/querycache",
  "//pkg/sql/roleoption",
- "//pkg/sql/row",
  "//pkg/sql/rowenc",
  "//pkg/sql/sem/tree",
  "//pkg/sql/sessiondata",

pkg/server/settingsworker.go

@@ -48,7 +48,7 @@ func processSystemConfigKVs(
  var k, v, t string
  // First we need to decode the setting name field from the index key.
  {
- types := []*types.T{tbl.GetPublicColumns()[0].Type}
+ types := []*types.T{tbl.PublicColumnsNew()[0].GetType()}
  nameRow := make([]rowenc.EncDatum, 1)
  _, matches, _, err := rowenc.DecodeIndexKey(codec, tbl, tbl.GetPrimaryIndex().IndexDesc(), types, nameRow, nil, kv.Key)
  if err != nil {
@@ -83,16 +83,16 @@ func processSystemConfigKVs(
  colID := lastColID + descpb.ColumnID(colIDDiff)
  lastColID = colID
  if idx, ok := colIdxMap.Get(colID); ok {
- res, bytes, err = rowenc.DecodeTableValue(a, tbl.GetPublicColumns()[idx].Type, bytes)
+ res, bytes, err = rowenc.DecodeTableValue(a, tbl.PublicColumnsNew()[idx].GetType(), bytes)
  if err != nil {
  return err
  }
  switch colID {
- case tbl.GetPublicColumns()[1].ID: // value
+ case tbl.PublicColumnsNew()[1].GetID(): // value
  v = string(tree.MustBeDString(res))
- case tbl.GetPublicColumns()[3].ID: // valueType
+ case tbl.PublicColumnsNew()[3].GetID(): // valueType
  t = string(tree.MustBeDString(res))
- case tbl.GetPublicColumns()[2].ID: // lastUpdated
+ case tbl.PublicColumnsNew()[2].GetID(): // lastUpdated
  // TODO(dt): we could decode just the len and then seek `bytes` past
  // it, without allocating/decoding the unused timestamp.
  default: