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: DROP DATABASE CASCADE doesn't work with sequence owners #50712

Closed
thoszhang opened this issue Jun 26, 2020 · 0 comments · Fixed by #50744
Closed

sql: DROP DATABASE CASCADE doesn't work with sequence owners #50712

thoszhang opened this issue Jun 26, 2020 · 0 comments · Fixed by #50744
Assignees
@arulajmani
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@thoszhang
Copy link
Contributor

Related to #50649.

[email protected]:52333/defaultdb> create database db;
CREATE DATABASE
Time: 2.824ms
[email protected]:52333/defaultdb> create table db.a (a int primary key);
CREATE TABLE
Time: 3.013ms
[email protected]:52333/defaultdb> create sequence db.foo owned by db.a.a;
CREATE SEQUENCE
Time: 27.509ms
[email protected]:52333/defaultdb> drop database db cascade;
ERROR: table "a" is being dropped

From @arulajmani:

So I dug into this a bit, and looks like there’s two issues going on here, 1 of which is because of [#50649].
Depending on what is dropped first (the sequence or the table) we run into two separate issues.

  • if the sequence is dropped before the table, then the table tries to drop the sequence again because it owns it. This results in “table foo is being dropped”. (initiateDropTable.go line 341)
  • if the table is dropped before the sequence, then the sequence tries to remove itself from the owner table column’s descriptor. This initiates a schema change, but we don’t allow schema changes on dropped tables, so we get an error. (I introduced this in my fix)
@thoszhang thoszhang added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jun 26, 2020
@thoszhang thoszhang changed the title sql: DROP DATABASE CASCADE bug doesn't work with sequence owners sql: DROP DATABASE CASCADE doesn't work with sequence owners Jun 26, 2020
@arulajmani arulajmani mentioned this issue Jun 29, 2020
Merged
craig bot pushed a commit that referenced this issue Jul 6, 2020
@giorgosp @arulajmani
@petermattis @tbg
Merge #45831 #50369 #50744 #50961 #51000
0b6e118 
45831: sql: add a comment that JSON behaviour is similar to JSONB r=jordanlewis a=giorgosp

Closes #44465
Release note : None

50369: sql: fix NewUniquenessConstraintViolationError reporting r=spaskob a=spaskob

Release note (bug fix):
If NewUniquenessConstraintViolationError cannot initialize a row fetcher
it will only report this error to the client without wrapping it with
information about the actual constraint violation. This is confusing.

See #46276.

Old error:
`ERROR: column-id "2" does not exist`

New Error:
```
ERROR: duplicate key value (b)=(couldn't fetch value: column-id "2"
does not exist) violates unique constraint "t_secondary"
```

50744: sql: fix drop database - sequence ownership bug r=solongordon a=arulajmani

Previously, `DROP DATABASE CASCADE` would not work if the database
contained a sequence owned by a table in the database. This would
happen because of two separate reasons:
- If the sequence was dropped before the table, the table would try to
"double drop" the sequence as it owned it; this would result in an
error.
- If the table was dropped before the sequence, the sequence would try
to remove the ownership dependency from the table descriptor, which had
already been dropped; this would also result in an error.

This PR addresses both these issues separately. Sequences are no longer
double dropped when dropping tables. Additionally, no attempt is made
to remove the ownership dependency from the table descriptor if the
table descriptor has already been dropped.

Fixes #50712

Release note (bug fix): `DROP DATABASE CASCADE` now works as expected
even when the database has a sequence with an owner in it.

50961: cmd/roachtest: fix --zones flag to work on AWS r=petermattis a=petermattis

Previously the `--zones` flag only worked on GCE and Azure and was
silently ignored on AWS. Noticed in passing while trying to run a
roachtest on a different AWS zone.

Release note: None

51000: acceptance: skip TestDockerCLI/test_demo_partitioning r=knz a=tbg

See: #50970

Release note: None

Co-authored-by: George Papadrosou <[email protected]>
Co-authored-by: Spas Bojanov <[email protected]>
Co-authored-by: arulajmani <[email protected]>
Co-authored-by: Peter Mattis <[email protected]>
Co-authored-by: Tobias Schottdorf <[email protected]>
@craig craig bot closed this as completed in 0ba9148 Jul 6, 2020
@arulajmani arulajmani mentioned this issue Jul 8, 2020
Closed
@jordanlewis jordanlewis mentioned this issue Jul 9, 2020
Closed
arulajmani added a commit to arulajmani/cockroach that referenced this issue Jul 21, 2020
@arulajmani
sql: fix drop database - sequence ownership bug
24e9d84 
Previously, `DROP DATABASE CASCADE` would not work if the database
contained a sequence owned by a table in the database. This would
happen because of two separate reasons:
- If the sequence was dropped before the table, the table would try to
"double drop" the sequence as it owned it; this would result in an
error.
- If the table was dropped before the sequence, the sequence would try
to remove the ownership dependency from the table descriptor, which had
already been dropped; this would also result in an error.

This PR addresses both these issues separately. Sequences are no longer
double dropped when dropping tables. Additionally, no attempt is made
to remove the ownership dependency from the table descriptor if the
table descriptor has already been dropped.

Fixes cockroachdb#50712

Release note (bug fix): `DROP DATABASE CASCADE` now works as expected
even when the database has a sequence with an owner in it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@arulajmani arulajmani

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: fix drop database - sequence ownership bug arulajmani/cockroach
2 participants
@thoszhang @arulajmani