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.

Sign up for GitHub

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

Jump to bottom

kv: add placeholders to ALTER RANGE RELOCATE #73315

Closed
lunevalex opened this issue Nov 30, 2021 · 4 comments · Fixed by #73807
Closed

kv: add placeholders to ALTER RANGE RELOCATE #73315

lunevalex opened this issue Nov 30, 2021 · 4 comments · Fixed by #73807
Labels
A-kv Anything in KV that doesn't belong in a more specific category. A-kv-distribution Relating to rebalancing and leasing. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team

Comments

@lunevalex
Copy link
Collaborator

#72305 introduced a new ALTER RANGE RELOCATE syntax to move leases and replicas around. The syntax does not support placeholder values for the range/store ids. This issue is to track to extending the syntax to support placeholders.

cc: @knz
@lunevalex lunevalex added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-kv-distribution Relating to rebalancing and leasing. A-kv Anything in KV that doesn't belong in a more specific category. labels Nov 30, 2021
@blathers-crl blathers-crl bot added the T-kv KV Team label Nov 30, 2021
@knz
Copy link
Contributor

knz commented Dec 13, 2021

This could be an issue for the SQL xp team right?

@lunevalex
Copy link
Collaborator Author

It could be although I suspect, they will find a hard time prioritizing this, since it's mainly to be used by KV.

@knz
Copy link
Contributor

knz commented Dec 13, 2021

I see. But then it's also "on hold" for KV now :)

@knz
Copy link
Contributor

knz commented Dec 14, 2021

I'll try to do a thing about it

This was referenced Dec 14, 2021
Merged
Merged
Merged
craig bot pushed a commit that referenced this issue Dec 14, 2021
@knz
Merge #73802 #73803
7bf398f 
73802: sql: use 'NONVOTERS' as keyword for 'RELOCATE', not 'NON_VOTERS' r=rafiss,RaduBerinde a=knz

Informs #73315.
Fixes #63219.

The idiom for positional keywords in SQL is to either use words
separated by spaces (e.g. NOT NULL), or to concatenate the words
together (ISERROR, NOLOGIN, LINESTRING).

Release note (sql change): In the experimental RELOCATE syntax
forms, the positional keyword that indicates that the statement
should move non-voter replicas is now spelled `NONVOTERS`, instead
of `NON_VOTERS` previously.

Release note (sql change): The inline help for the ALTER
statements now mentions the RELOCATE syntax.

73803: sql: use a 3-valued type instead of 2 booleans for relocate statements r=RaduBerinde a=knz

Informs #73315. 
First 3 commits from #73802 (reviewers, focus on the last commit)

By using a 3-valued types, we greatly simplify the code to print out
statement types, and also clarify intent in all the conditionals that
depend on the relocation mode.

Co-authored-by: Raphael 'kena' Poss <[email protected]>
@craig craig bot closed this as completed in f6f60c6 Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-kv Anything in KV that doesn't belong in a more specific category. A-kv-distribution Relating to rebalancing and leasing. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: support scalar expressions (& placeholders) for alter range ... relocate knz/cockroach
2 participants
@knz @lunevalex