-
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
parser: implement ALTER TABLE ... RESET (...) #75429
Conversation
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.
lgtm! just a nit for comments
Reviewed 2 of 3 files at r1, 3 of 5 files at r3, 9 of 9 files at r4, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan)
pkg/sql/paramparse/paramobserver.go, line 85 at r4 (raw file):
onSet(evalCtx *tree.EvalContext, key string, datum tree.Datum) error onReset(evalCtx *tree.EvalContext, key string) error runPostChecks() error
nit: could use comments on all of the interface methods
done! bors r=rafiss |
Canceled. |
oops, pushed the wrong branch bors r=rafiss |
Build failed (retrying...): |
This is a minor refactor of the paramparse package to prepare for implementing the ALTER TABLE ... RESET (...) syntax. Actions * Rename the receivers from `a` to `po`. * Make methods on `ParamObserver` interface private. * Rename `Apply` to `Set. * Make the table parameters a map. Release note: None
conflict ... bors r- |
Canceled. |
Also includes pieces here which will pave the way for implementing the TTL syntax. Release note (sql change): Implemented the `ALTER TABLE ... RESET (...)` syntax. This is currently a no-op operation.
Release note: None
bors r=rafiss |
Build succeeded: |
Only last two commits for review (rest is #75262).
See individual commits for details.