-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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/physicalplan: make more tests compatible with secondary tenants #108764
Conversation
393d08d
to
39e70a3
Compare
0173582
to
371c1c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The general changes and the ones from sql_stats:
I will leave the kv, multiregion and plans changes to be reviewed by their respective teams
Reviewed 7 of 7 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @DarrylWong, @rachitgsrivastava, and @yuzefovich)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 7 of 7 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @DarrylWong, @knz, and @rachitgsrivastava)
pkg/server/testserver.go
line 1689 at r1 (raw file):
} // LocalityRef is part of the serverutils.StorageLayerInterface.
nit: do we still need this method? I don't see any renaming in the first commit, so it looks like it's actually no longer used. Also, it's now removed from the storage layer interface.
Release note: None
Release note: None
371c1c5
to
9c30b09
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @DarrylWong, @maryliag, @rachitgsrivastava, and @yuzefovich)
pkg/server/testserver.go
line 1689 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: do we still need this method? I don't see any renaming in the first commit, so it looks like it's actually no longer used. Also, it's now removed from the storage layer interface.
Good call. Removed.
TFYRs bors r=maryliag,yuzefovich |
Build succeeded: |
Informs #76378.
Links to #108763.
Epic: CRDB-18499