sql: fix drop database - sequence ownership bug #50744
Conversation
|
This change is |
|
For some reason, the tests I added aren't moving past the |
|
Note, use the |
arulajmani
force-pushed
the
sequences-50712
branch
from
9dce950 to
e7cf2ce
Compare
I haven't looked closely at these changes yet, but this sounds potentially bad. Would you mind digging further into what's getting stuck? I wonder if it's related to the jobs. |
There was a problem hiding this comment.
Agree that we should dig into what's going wrong with that test config.
The changes look reasonable to me though I don't have a deep understanding of the queueJob parameter. It would be great for @lucy-zhang to sign off on that change, i.e. that queueJob is now being set to false when dropping sequences during a DROP DATABASE.
Could you also add analogous tests for DROP DATABASE without CASCADE? Looks like this is not covered right now.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @solongordon)
|
Couldn't find anything that immediately jumped out w.r.t the 3node-tenant config from the stack traces. Looks like the problem is not related to sequences -- it fails on a db containing a single, regular table as well. I've opened #50840 |
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.
arulajmani
force-pushed
the
sequences-50712
branch
from
e7cf2ce to
0ba9148
Compare
There was a problem hiding this comment.
Added a test for DROP DATABASE without CASCADE as well. Should be RFAL.
Note on the queueJob -- as part of my change, I plumbed it down so that dropping a sequence owned by a table respects the queueJob of the table that owned it. It doesn't have an affect on the bug.
Reviewable status:
There was a problem hiding this comment.
Reviewed 4 of 5 files at r1.
Reviewable status:
|
bors r=solongordon |
Build succeeded |
Previously,
DROP DATABASE CASCADEwould not work if the databasecontained a sequence owned by a table in the database. This would
happen because of two separate reasons:
"double drop" the sequence as it owned it; this would result in an
error.
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 CASCADEnow works as expectedeven when the database has a sequence with an owner in it.