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.

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

sql: replace deprecated descs.Collection method calls #93570

Merged
merged 3 commits into from
Dec 14, 2022

Conversation

postamar
Copy link

@postamar postamar commented Dec 14, 2022

descs: remove GetObjectNamesAndIDs method

This commit replaces usages of that method with calls to
GetAllObjectsInSchema.

Informs #64089.

Release note: None


sql: amortize schema descriptor lookups in vtable generation

This gets rid of some deprecated descs.Collection method calls.

Informs #64089.

Release note: None


scbuild,scdeps: replace deprecated descs.Collection method calls

Informs #64089.

Release note: None

Marius Posta added 3 commits December 13, 2022 23:28
This gets rid of some deprecated descs.Collection method calls.

Informs cockroachdb#64089.

Release note: None
This commit replaces usages of that method with calls to
GetAllObjectsInSchema.

Informs cockroachdb#64089.

Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@postamar postamar marked this pull request as ready for review December 14, 2022 12:06
@postamar postamar requested review from a team and ajwerner December 14, 2022 12:06
@postamar
Copy link
Author

@ajwerner this is the PR I alluded to in #93543 which undoes the rttanalysis regression.

Copy link
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm cool with this change. More generally I think functions have muddled our terminology in a bad way.

@@ -69,18 +70,27 @@ type schemaResolver struct {
// GetObjectNamesAndIDs implements the resolver.SchemaResolver interface.
func (sr *schemaResolver) GetObjectNamesAndIDs(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we've got a terminology problem these days: is a function an object?

By my old definition, it is, but it isn't here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, but it's also a special kind of object in that unlike other objects we don't expect functions to have a namespace entry.

I completely agree with you in general, btw. Untangling GetObjectNamesAndIDs is going to be its own unit of work focused on the resolver interface, and it's something I want to do this week.

@postamar
Copy link
Author

Thanks for the review!

bors r+

@craig
Copy link
Contributor

craig bot commented Dec 14, 2022

Build succeeded:

@craig craig bot merged commit f25af03 into cockroachdb:master Dec 14, 2022
@postamar postamar deleted the remove-deprecated-get-all branch December 14, 2022 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants