Last Comment Bug 715870 - [Oracle] Related sequences and triggers must be removed when dropping a table
: [Oracle] Related sequences and triggers must be removed when dropping a table
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Database (show other bugs)
: 4.2
: All All
: -- normal (vote)
: Bugzilla 4.2
Assigned To: Frédéric Buclin
: default-qa
Mentors:
http://groups.google.com/group/mozill...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-06 06:08 PST by Frédéric Buclin
Modified: 2012-01-24 09:43 PST (History)
2 users (show)
LpSolit: approval+
LpSolit: approval4.2+
LpSolit: blocking4.2+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch, v1 (1.25 KB, patch)
2012-01-06 06:08 PST, Frédéric Buclin
mkanat: review+
Details | Diff | Splinter Review
patch, v2 (1.04 KB, patch)
2012-01-16 16:00 PST, Frédéric Buclin
mkanat: review+
Details | Diff | Splinter Review

Description Frédéric Buclin 2012-01-06 06:08:02 PST
Created attachment 586399 [details] [diff] [review]
patch, v1

When a table is created, several sequences and triggers are created. But bz_drop_table() doesn't remove them, meaning that you cannot recreate the table because it will complain that the sequence already exists. This is the reason why a user got this exact problem in the support mailing-list when upgrading from 4.1.3 to 4.2rc1. We remove the newly created "tag" table, and then rename "tags" to "tag". But bz_drop_table("tag") didn't remove all its related stuff correctly, and so bz_rename_table("tags", "tag") cannot run correctly because stuff is already there.

This patch also fixes a typo in get_rename_table_sql() introduced by bug 683644.
Comment 1 Frédéric Buclin 2012-01-06 06:59:01 PST
In the URL field, the original description of the problem.
Comment 2 Max Kanat-Alexander 2012-01-07 00:11:48 PST
Comment on attachment 586399 [details] [diff] [review]
patch, v1

Review of attachment 586399 [details] [diff] [review]:
-----------------------------------------------------------------

If this bug has corrupted existing databases, there will also have to be some code in DB::Oracle::bz_setup_database to fix existing installs.
Comment 3 Frédéric Buclin 2012-01-07 04:52:27 PST
(In reply to Max Kanat-Alexander from comment #2)
> If this bug has corrupted existing databases, there will also have to be
> some code in DB::Oracle::bz_setup_database to fix existing installs.

I would have to investigate even more, but I would like to keep this bug specific to fix bz_drop_table() correctly, and keep for a separate bug the problem you mention, if it's still present. AFAIK, this only affects upgrades from 4.1.x to 4.2rc1, not from a stable release to 4.2rc1. Do you agree with this proposal?
Comment 4 Frédéric Buclin 2012-01-11 14:59:42 PST
Comment on attachment 586399 [details] [diff] [review]
patch, v1

wicked doesn't seem to be around these days.
Comment 5 Max Kanat-Alexander 2012-01-13 22:32:13 PST
Comment on attachment 586399 [details] [diff] [review]
patch, v1

Review of attachment 586399 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great! I'm assuming you tested and those trigger names are right. (I know that Oracle does this hashing thing for identifiers--I'm not sure if it applies here?)

::: Bugzilla/DB/Schema/Oracle.pm
@@ +410,5 @@
> +        if ($def->{TYPE} =~ /varchar|text/i && $def->{NOTNULL}) {
> +            push(@sql, "DROP TRIGGER ${name}_${column}");
> +        }
> +    }
> +    push(@sql, "DROP TABLE $name");

Maybe get this by calling $self->SUPER::get_drop_table_ddl?
Comment 6 Frédéric Buclin 2012-01-14 03:49:47 PST
(In reply to Max Kanat-Alexander from comment #5)
> Looks great! I'm assuming you tested and those trigger names are right.

Yes, those names are right. I checked them one by one.


> Maybe get this by calling $self->SUPER::get_drop_table_ddl?

Sure, I will do this change.


Thanks for the review!
Comment 7 Frédéric Buclin 2012-01-16 16:00:40 PST
Created attachment 589060 [details] [diff] [review]
patch, v2

I realized that my previous patch didn't correctly remove all indexes attached to tables, and so the deletion could sometimes fail. To remove all indexes correctly, I have to write "DROP TABLE $name CASCADE CONSTRAINTS PURGE" instead of "DROP TABLE $name" only. CASCADE CONSTRAINTS also removes all triggers itself, so we do not need to do it ourselves anymore. But it doesn't remove sequences at all, which is why we still have to do it ourselves. This makes the patch shorter and cleaner.

I tested, and everything works very cleanly now.
Comment 8 Max Kanat-Alexander 2012-01-24 01:36:27 PST
Comment on attachment 589060 [details] [diff] [review]
patch, v2

Review of attachment 589060 [details] [diff] [review]:
-----------------------------------------------------------------

Beautiful! Very nice. :-)
Comment 9 Frédéric Buclin 2012-01-24 09:43:23 PST
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/DB/Schema/Oracle.pm
Committed revision 8091.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified Bugzilla/DB/Schema/Oracle.pm
Committed revision 8012.

Note You need to log in before you can comment on or make changes to this bug.