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: panic: add column T[] unique #26483

Closed
maddyblue opened this issue Jun 6, 2018 · 4 comments
Closed

sql: panic: add column T[] unique #26483

maddyblue opened this issue Jun 6, 2018 · 4 comments
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@maddyblue
Copy link
Contributor

create table t(); alter table t ADD COLUMN c int[] UNIQUE;

panic while executing 1 statements: ALTER TABLE _ ADD COLUMN _ INT[] UNIQUE; caused by unimplemented: column c is of type ARRAY and thus is not indexable

Found with RSG.

@maddyblue maddyblue added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-bulkio-schema-changes labels Jun 6, 2018
@maddyblue
Copy link
Contributor Author

This panics 2.0.2 so we should fix that in master and backport.

@solongordon
Copy link
Contributor

Sentry issue: COCKROACHDB-GY

@bobvawter
Copy link
Contributor

checkColumnsValidForIndex depends on the new column being present in the TableDescriptor before being able to perform the check. This means that an AddColumnMutation followed by an AddIndexMutation winds up actually not performing the index preflight check since the column (with un-indexable type) isn't yet present.

@vivekmenezes I'll update the ADD COLUMN ... UNIQUE flow to ensure the pre-flight checks are run and then fix this more generally in the work you and I have discussed to make TableDescriptor changes operate as a queue of functions.

@knz
Copy link
Contributor

knz commented Jun 13, 2018

@bobvawter note there are multiple types besides arrays that are non-indexable. for example JSON. Perhaps make multiple tests?

craig bot pushed a commit that referenced this issue Jun 14, 2018
26684: sql: Fix panic adding unique, non-indexable column r=bobvawter a=bobvawter

This change updates the `checkColumnsFor...Index` functions to use
`allNonDropColumns()` to validate against any proposed mutations.  Otherwise, a
`ColumnMutation` that adds a non-indexable column followed by an
`IndexMutation` creating an index on that column would would be incorrectly
accepted, leading to a panic.

Fixes #26483

Release note (sql change): Return an error to the user instead of panicing when
trying to add a column with a unique constraint when that column's type is not
indexable.

26705: roachtest: use --sequential when generating store dumps r=benesch a=petermattis

This makes the mapping from cockroach node ID to roachprod node ID the
identify function.

Release note: None

Co-authored-by: Bob Vawter <[email protected]>
Co-authored-by: Peter Mattis <[email protected]>
@craig craig bot closed this as completed in #26684 Jun 14, 2018
craig bot pushed a commit that referenced this issue Jun 14, 2018
26728: backport 2.0: sql: Fix panic adding unique, non-indexable column r=bobvawter a=bobvawter

This change updates the `checkColumnsFor...Index` functions to use
`allNonDropColumns()` to validate against any proposed mutations.  Otherwise, a
`ColumnMutation` that adds a non-indexable column followed by an
`IndexMutation` creating an index on that column would would be incorrectly
accepted, leading to a panic.

Fixes #26483

Release note (sql change): Return an error to the user instead of panicing when
trying to add a column with a unique constraint when that column's type is not
indexable.

Co-authored-by: Bob Vawter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

No branches or pull requests

5 participants