sql: stop returning raw protobufs from descriptor accessors

[jordanlewis]

Currently, the interfaces defined for the top level descriptor types (catalog.{,Database,Schema,Table,Type}Descriptor) return raw protobufs from most of their methods. This is undesirable (at minimum) because the return values are easily mutated by accident, and because there's no opportunity to extend the raw protobufs with more advanced caching data structures.

Adding interfaces here instead of raw protos will also improve API uniformity and make our codebase more easily tested and understood.

---

The goal of this issue is to wrap the returned protobufs in an struct and accompanying interface that hides the underlying protobufs. Some of the leaf structs matter less - for example, UserPrivilege isn't important to wrap. It's more important to prevent the big ones from mutability: ColumnDescriptor, IndexDescriptor, and so on.

We'll be extending catalog.Descriptor and its sub-interfaces (DatabaseDescriptor, SchemaDescriptor, TableDescriptor, TypeDescriptor) so that their methods do not return things from the descpb package at all.

For example, we'll want to start returning a new catalog.IndexDescriptor interface instead of descpb.IndexDescriptor from TableDescriptor.GetPrimaryIndex().

Here is the list of interfaces we'll want to extract into sql/catalog, along with associated new structs inside of sql/catalog/tabledesc:

• ColumnFamilyDescriptor
• IndexDescriptor sql: stop returning raw protobufs for IndexDescriptor API getters #57465
• ColumnDescriptor sql: stop returning raw protobufs for ColumnDescriptor API getters #59805
• PrivilegeDescriptor
• Constraints - we need to figure out what to do here in more detail.

We'll need to figure out a pattern for iteration as well, for methods like GetPublicNonPrimaryIndexes(). We don't want to have to allocate a slice of n interfaces and structs for each of these items - for example, the ColumnDescriptors inside of an IndexDescriptor.

---

In addition to the new interfaces listed above we'll also want to extend and use the existing catalog interfaces:

• TableDescriptor sql: use catalog.TableDescriptor instead of tabledesc.Immutable #59486
• DatabaseDescriptor, SchemaDescriptor, TypeDescriptor sql: use catalog.*Descriptor instead of (dbdesc|schemadesc|typedesc).Immutable #62750
• Column, Index sql: use catalog.Column and catalog.Index outside of catalog #63755

---

We'll also want to unify the new interfaces with those in pkg/sql/opt/cat at some point:

• cat.Index -> catalog.Index
• cat.Column -> catalog.Column
• others as well most likely (TBD)

[blathers-crl]

Hi @jordanlewis, please add a C-ategory label to your issue. Check out the label system docs.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[postamar]

I believe we should close this issue for now. This doesn't mean there's nothing left to do re. virtualization of descriptor protos, in fact there's plenty to do. However:

1. At this point it the direction of future work has been set and there's not much to discuss, on, for instance, how to virtualize column families.
2. The original goal was to wrap everything in descpb. At this point this isn't particularly desirable (nothing wrong with descpb.ID for instance) and I'm quite happy with our choice of not virtualizing mutable descriptors: immutable table descriptors are used via a catalog.TableDescriptor interface but mutables are a concrete tabledesc.Mutable.
3. Our efforts on the new schema changer will probably drive this in un-anticipated directions anyway.

Let's leave things be as they are and revisit this after the next release.