Skip to content

clusterversion,sql: remove ClusterLocksVirtualTable#85938

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
celiala:remove-gates.ClusterLocksVirtualTable
Aug 19, 2022
Merged

clusterversion,sql: remove ClusterLocksVirtualTable#85938
craig[bot] merged 1 commit intocockroachdb:masterfrom
celiala:remove-gates.ClusterLocksVirtualTable

Conversation

@celiala
Copy link
Copy Markdown
Collaborator

@celiala celiala commented Aug 11, 2022 •
edited
Loading

This commit removes the 22.1 ClusterLocksVirtualTable version gate.

Cleanup was done following guidance from 21.2 cleanup:

For the most part, if the gates were just simple if !version.IsActive { return x } or something, I just removed the block, and even if it was a little more complicated, like args = [x]; if version { args = append(args, y) }; foo(args) I still tried to mostly inline it such that it looked natural (i.e. remove that append and make it args = [x, y]).

However for just a couple more complicated cases that were referring to <21.2 versions that needed to be replaced when those were deleted, I added a placeholder clusterversion.TODOPre21_2 alias for 21.2. Replacing those calls with this alias shouldn't change their behavior -- it was already always true, since the code today should never run in a <21.2 cluster -- but means we can delete those older versions in the meantime and then the owners of these bits can decide how to update them.

Partially addresses #80663

Release note: none
Release justification: remove old version gates.

@celiala celiala requested review from a team August 11, 2022 01:18
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Aug 11, 2022

This change is Reviewable

@celiala celiala requested a review from AlexTalks August 11, 2022 01:19
AlexTalks
AlexTalks approved these changes Aug 11, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@AlexTalks AlexTalks left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@celiala
Copy link
Copy Markdown
Collaborator Author

celiala commented Aug 11, 2022

TFTR!

FYI @AlexTalks that I'm going to remove TestAutoStatsTableSettingsDisallowedOnOldCluster -- please scream and shout if this should be kept!

Removing TestAutoStatsTableSettingsDisallowedOnOldCluster since:

  • it was failing (expected "feature not available yet" error but instead got nil)
  • it depended on both ClusterLocksVirtualTable and AutoStatsTableSettings -- both of which are 22.1 version gates

@celiala celiala force-pushed the remove-gates.ClusterLocksVirtualTable branch 2 times, most recently from 4501264 to a065663 Compare August 12, 2022 20:37
@celiala celiala mentioned this pull request Aug 15, 2022
Closed
2 tasks
@celiala celiala force-pushed the remove-gates.ClusterLocksVirtualTable branch 2 times, most recently from 40f30c5 to 7301baf Compare August 19, 2022 01:53
@celiala
Copy link
Copy Markdown
Collaborator Author

celiala commented Aug 19, 2022 •
edited
Loading

TestLogic_external_connection_privileges is failing all of #85937 , #85938 , #85939, so I'm going to assume that it's not related to changes in this commit.

off_test.runfiles/com_github_cockroachdb_cockroach/pkg/sql/logictest/testdata/logic_test/external_connection_privileges:24: SELECT * FROM system.privileges
        expected:
            root      /externalconn/foo  {ALL}         {}
            testuser  /externalconn/foo  {DROP,USAGE}  {}
        but found (query options: "") :
            testuser  /externalconn/foo  {DROP,USAGE}  {}
            root      /externalconn/foo  {ALL}         {}

This was referenced Aug 19, 2022
Merged
Merged
@celiala
clusterversion,sql: remove ClusterLocksVirtualTable
493f535 
Release note: None
Release justification: remove old version gates.
@celiala celiala force-pushed the remove-gates.ClusterLocksVirtualTable branch from 7301baf to 493f535 Compare August 19, 2022 06:09
@celiala
Copy link
Copy Markdown
Collaborator Author

celiala commented Aug 19, 2022

TFTR!

bors r=AlexTalks

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 19, 2022

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 19, 2022

Build succeeded:

@craig craig bot merged commit 9a1c3a9 into cockroachdb:master Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@AlexTalks AlexTalks AlexTalks approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@celiala @cockroach-teamcity @AlexTalks