-
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
schemachanger: prep work ALTER DATABASE .. CONFIGURE ZONE in DSC #120362
schemachanger: prep work ALTER DATABASE .. CONFIGURE ZONE in DSC #120362
Conversation
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
0d4c606
to
c0a39bc
Compare
This some of Xiang's prep work for CONFIGURE ZONE (the parts that can be merged ahead of time to ease the review process) |
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.
One minor nit
Reviewed 4 of 4 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @annrpom)
pkg/sql/catalog/zone/zones.go
line 37 at r2 (raw file):
Field config.Field RequiredType *types.T // Setter is used to set `Field` in the `zone config` to `datum`.
Think this needs to be in a different commit
This commit is a mechnical change that moves the gloval variable `errNoZoneConfigApplies` from package `sql` to package `sqlerrors`, as it will be used by the DSC when we support zone config in it. Informs: cockroachdb#117574 Release note: None
This commit cleaned up a few functions using the preferred .FilterXxx() API to retrieve element from an element set. No functional change is introduced. Informs: cockroachdb#117574 Release note: None
c0a39bc
to
1d110f0
Compare
had this sitting in anticipation for when db zone configs was wrapping up (personal preference) -- just rebased and addressed nit. will merge after checks are run |
TFTR! ('-')7 bors r=fqazi |
sqlerrors: move gloval var errNoZoneConfigApplies into sqlerrors package
This commit is a mechnical change that moves the gloval variable
errNoZoneConfigApplies
from packagesql
to packagesqlerrors
, asit will be used by the DSC when we support zone config in it.
Informs: #117574
Release note: None
scbuildstmt: Rerewrite a few function with better APIs
This commit cleaned up a few functions using the preferred .FilterXxx()
API to retrieve element from an element set. No functional change is
introduced.
Informs: #117574
Release note: None