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

sql: improve backward compatibility for ALTER PARTITION ... OF TABLE #40425

Closed
solongordon opened this issue Sep 3, 2019 · 0 comments · Fixed by #40650
Closed

sql: improve backward compatibility for ALTER PARTITION ... OF TABLE #40425

solongordon opened this issue Sep 3, 2019 · 0 comments · Fixed by #40650
Assignees
@solongordon
Labels
A-partitioning S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.

Comments

@solongordon
Copy link
Contributor

In 19.1, the ALTER PARTITION p OF TABLE command will look for a partition named p across all of the indexes of a table. When we made partition names index-scoped in #20880, we changed this behavior so that ALTER PARTITION ... OF TABLE only works on the primary index of the table. To alter an index partition you need to instead run ALTER PARTITION p OF INDEX tbl@idx.

This makes a kind of sense because there might be multiple partitions named p across indexes. However, we have seen it break backward compatibility in a few cases like #40416, and I'm worried that customers could run into the same issue after they upgrade if they have scripts that use partitioning. I think it might be best if we bring back the old behavior and then only error out if we encounter multiple partitions with the same name.

cc @awoods187, does this make sense to you?
@solongordon solongordon added S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. A-partitioning labels Sep 3, 2019
@solongordon solongordon self-assigned this Sep 3, 2019
@solongordon solongordon mentioned this issue Sep 10, 2019
Merged
craig bot pushed a commit that referenced this issue Sep 11, 2019
@solongordon
Merge #40650
fc9eb68 
40650: sql: backward compat for ALTER PARTITION OF TABLE r=solongordon a=solongordon

When we made partition names index-scoped, we removed the ability to
specify secondary index partitions in the ALTER PARTITION ... OF TABLE
command. However I'm concerned this could break some backward
compatibility. I restored that behavior but added logic to throw an
error if there are multiple partitions with the same name.

Fixes #40425

Release note: None

Co-authored-by: Solon Gordon <solon@cockroachlabs.com>
@craig craig bot closed this as completed in 65c5e37 Sep 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@solongordon solongordon

Labels
A-partitioning S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: backward compat for ALTER PARTITION OF TABLE solongordon/cockroach
1 participant
@solongordon