Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sql: stop returning raw protobufs for ColumnDescriptor API getters #59805

Closed
postamar opened this issue Feb 4, 2021 · 0 comments · Fixed by #59789
Closed

sql: stop returning raw protobufs for ColumnDescriptor API getters #59805

postamar opened this issue Feb 4, 2021 · 0 comments · Fixed by #59789
Assignees
@postamar
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@postamar
Copy link
Contributor

postamar commented Feb 4, 2021

This is a child of #56306, see parent issue for details.
@postamar postamar added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Feb 4, 2021
@postamar postamar self-assigned this Feb 4, 2021
@postamar postamar mentioned this issue Feb 4, 2021
Closed
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 4, 2021
sql/catalog: add Column interface and TableDescriptor methods
50373fa 
Previously, the catalog.TableDescriptor interface and its implementing
types would liberally return descpb.ColumnDescriptor values, pointers and
slices. In an effort to stop manipulating such protos directly, this
patch introduces a catalog.Column interface to encapsulate it. In order
to enventually propagate this change throughout the code base, this
patch marks some existing catalog.TableDescriptor methods as deprecated
and introduces new ones to eventually replace them.

This work is very similar to that which I already performed for cockroachdb#57465.

Fixes cockroachdb#59805.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 4, 2021
sql: remove deprecated ForeachColumn-methods from TableDescriptor
f9fcf6d 
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
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 4, 2021
sql: remove deprecated FindColumn-methods from TableDescriptor
ff84b9b 
Previously, this method would be called on a table descriptor interface to
return a target *descpb.ColumDescriptor.

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

Fixes cockroachdb#59805.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 4, 2021
sql: remove deprecated column slice methods in TableDescriptor
bf1d1ad 
Previously, these methods would be called on a table descriptor interface
to obtain []descpb.ColumnDescriptor values.

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

GetPublicColumns() is treated in a separate commit owing to its large
number of call sites.

Fixes cockroachdb#59805.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 4, 2021
sql: remove deprecated ColumnIdx-methods from TableDescriptor
1a8b5dd 
Previously, the ColumnIdxMap* methods and the GetColumnAtIdx method would be
called on a table descriptor interface to return catalog.TableColMap and
corresponding descpb.ColumnDescriptor values, respectively.

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

Fixes cockroachdb#59805.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 4, 2021
sql: remove deprecated GetPublicColumns method from TableDescriptor
52705f2 
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
@postamar postamar mentioned this issue Feb 4, 2021
Merged
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 4, 2021
sql: rename Column slice methods in TableDescriptor
41bcc40 
This patch renames the catalog.Column-slice-returning methods which I
recently added to catalog.TableDescriptor. I gave them provisional names
to differentiate them from the existing and very similar deprecated methods
that they replaced. Now that all deprecated methods have been removed we
can give them their definitive names.

Fixes cockroachdb#59805.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 5, 2021
sql: remove deprecated ForeachColumn-methods from TableDescriptor
c5c0035 
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
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 5, 2021
sql: remove deprecated FindColumn-methods from TableDescriptor
f3ffee8 
Previously, this method would be called on a table descriptor interface to
return a target *descpb.ColumDescriptor.

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

Fixes cockroachdb#59805.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 5, 2021
sql: remove deprecated column slice methods in TableDescriptor
473f9df 
Previously, these methods would be called on a table descriptor interface
to obtain []descpb.ColumnDescriptor values.

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

GetPublicColumns() is treated in a separate commit owing to its large
number of call sites.

Fixes cockroachdb#59805.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 5, 2021
sql: remove deprecated ColumnIdx-methods from TableDescriptor
365dde6 
Previously, the ColumnIdxMap* methods and the GetColumnAtIdx method would be
called on a table descriptor interface to return catalog.TableColMap and
corresponding descpb.ColumnDescriptor values, respectively.

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

Fixes cockroachdb#59805.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 5, 2021
sql: remove deprecated GetPublicColumns method from TableDescriptor
38f055a 
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
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 5, 2021
sql: rename Column slice methods in TableDescriptor
655a28b 
This patch renames the catalog.Column-slice-returning methods which I
recently added to catalog.TableDescriptor. I gave them provisional names
to differentiate them from the existing and very similar deprecated methods
that they replaced. Now that all deprecated methods have been removed we
can give them their definitive names.

Fixes cockroachdb#59805.

Release note: None
craig bot pushed a commit that referenced this issue Feb 5, 2021
Merge #59789
4516f63 
59789: sql: add catalog.Column interface, replace catalog.TableDescriptor methods for columns r=postamar a=postamar

In an effort to not directly use `descpb.ColumnDescriptor` protos everywhere, this multi-commit PR:
1. introduces a new `catalog.Column` interface to encapsulate them, and a bunch of new methods in `catalog.TableDescriptor` to replace those which have `descpb.ColumnDescriptor` in their signatures, this is done in the first commit;
2. replaces calls to the old methods with calls to the new methods throughout the codebase in the subsequent commits.

This breakdown into multiple commits is done to ease review, most of the substantial changes are in the first commit, the others follow through with the change and so are quite noisy.

Fixes #59805. 

See also the parent issue #56306, as well as the related issue #57465 which tracks the same work I've already done for `descpb.IndexDescriptor`. In this PR I reused many of the same patterns I introduced in the corresponding PR #58678.

Subsequent work should consist in propagating this `descpb.ColumnDescriptor -> catalog.Column` change further throughout the codebase.

Co-authored-by: Marius Posta <marius@cockroachlabs.com>
@craig craig bot closed this as completed in a4ac4d1 Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@postamar postamar

Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
1 participant
@postamar