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.

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: make partition names index-scoped, not table-scoped #20880

Closed
benesch opened this issue Dec 19, 2017 · 4 comments
Closed

sql: make partition names index-scoped, not table-scoped #20880

benesch opened this issue Dec 19, 2017 · 4 comments
Assignees
Labels
A-partitioning C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.

Comments

@benesch
Copy link
Contributor

benesch commented Dec 19, 2017

Users are likely to be confused by the fact that partition names cannot be reused across indexes of the same table.

> CREATE TABLE t ... PARTITION BY (a) (
  PARTITION p1 VALUES IN (...)
)

> CREATE INDEX ON t (b) PARTITION BY (
  PARTITION p1 VALUES IN (...)
)
error name already in use

This was somewhat solved with a better error message, which tells you which other index is already using the name.

Still, the only reason that partition names are table scoped is because there's no good way to refer to an index-scoped name from the CLI. Right now,

./cockroach zone set a.b.c

unambiguously means database "a", table "b", and partition "c". If partitions become index-scoped, you'll be required to instead say:

./cockroach zone set a.b@index.c

This raises an annoying question: is a.b@primary.c different from a.b.c? Should a.b@index.c inherit from a.b.c?

Some of these questions will be solved when we ditch the zone CLI for the SQL zone config interface, so holding off until that interface stabilizes.

@benesch benesch added A-partitioning S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. labels Dec 19, 2017
@benesch benesch added this to the Later milestone Dec 19, 2017
@knz knz added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Apr 27, 2018
@petermattis petermattis removed this from the Later milestone Oct 5, 2018
@rohany
Copy link
Contributor

rohany commented Jul 24, 2019

Since we have a way of manipulating partitions now in SQL, are there other things that depend on partitions having duplicate names across different indexes?

@solongordon
Copy link
Contributor

I noticed that the ALTER PARTITION ... OF TABLE currently allows you to modify secondary index partitions as well. We'll probably need to remove that functionality and require users to use ALTER PARTITION ... OF INDEX instead.

Example:

root@:26257/defaultdb> CREATE TABLE t (
    x INT PRIMARY KEY,
    y INT,
    INDEX t_y_idx (y ASC) PARTITION BY LIST (y) (
        PARTITION a_idx VALUES IN (1)
    )
) PARTITION BY LIST (x) (
    PARTITION a VALUES IN (1)
);
CREATE TABLE

Time: 3.806ms

root@:26257/defaultdb> ALTER PARTITION a_idx OF TABLE t CONFIGURE ZONE USING constraints='[+region=us-east1]';
CONFIGURE ZONE 1

Time: 5.536ms

Once partition names aren't unique, users will instead need to run

root@:26257/defaultdb> ALTER PARTITION a_idx OF INDEX t@t_y_idx CONFIGURE ZONE USING constraints='[+region=us-east1]';
CONFIGURE ZONE 1

Time: 5.549ms

@solongordon
Copy link
Contributor

I also expect we'll want to remove the deprecated cockroach zone commands to avoid dealing with the ambiguities discussed in the original issue.

solongordon added a commit to solongordon/cockroach that referenced this issue Jul 30, 2019
I removed the "cockroach zone" commands, which have been deprecated
since 19.1. This will facilitate some of the zone usability improvements
we are making for 19.2. I left the commands there but they now return an
error with the equivalent SQL command.

Refers cockroachdb#20880

Release note (cli change): Removed the deprecated "cockroach zone" CLI
commands. Please use the equivalent "SHOW ZONE" and "CONFIGURE ZONE"
commands in a SQL client.
@solongordon solongordon self-assigned this Jul 30, 2019
solongordon added a commit to solongordon/cockroach that referenced this issue Jul 31, 2019
I removed the "cockroach zone" commands, which have been deprecated
since 19.1. This will facilitate some of the zone usability improvements
we are making for 19.2. I left the commands there but they now return an
error with the equivalent SQL command.

Refers cockroachdb#20880

Release note (cli change): Removed the deprecated "cockroach zone" CLI
commands. Please use the equivalent "SHOW ZONE" and "CONFIGURE ZONE"
commands in a SQL client.
solongordon added a commit to solongordon/cockroach that referenced this issue Jul 31, 2019
I removed the "cockroach zone" commands, which have been deprecated
since 19.1. This will facilitate some of the zone usability improvements
we are making for 19.2. I left the commands there but they now return an
error with the equivalent SQL command.

Refers cockroachdb#20880

Release note (cli change): Removed the deprecated "cockroach zone" CLI
commands. Please use the equivalent "SHOW ZONE" and "CONFIGURE ZONE"
commands in a SQL client.
craig bot pushed a commit that referenced this issue Jul 31, 2019
39177: cli: remove deprecated "zone" commands r=solongordon a=solongordon

I removed the "cockroach zone" commands, which have been deprecated
since 19.1. This will facilitate some of the zone usability improvements
we are making for 19.2. I left the commands there but they now return an
error with the equivalent SQL command.

Refers #20880

Release note (cli change): Removed the deprecated "cockroach zone" CLI
commands. Please use the equivalent "SHOW ZONE" and "CONFIGURE ZONE"
commands in a SQL client.

Co-authored-by: Solon Gordon <solon@cockroachlabs.com>
@solongordon
Copy link
Contributor

Fixed by #39332.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-partitioning C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.
Projects
None yet
Development

No branches or pull requests

5 participants