The default bug view has changed. See this FAQ.

[Oracle] Related sequences and triggers must be removed when dropping a table

RESOLVED FIXED in Bugzilla 4.2

Status

()

Bugzilla
Database
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Frédéric Buclin, Assigned: Frédéric Buclin)

Tracking

Bugzilla 4.2
Bug Flags:
approval +
approval4.2 +
blocking4.2 +

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

1.04 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
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.
Attachment #586399 - Flags: review?(wicked)
Flags: blocking4.2+
(Assignee)

Comment 1

5 years ago
In the URL field, the original description of the problem.

Comment 2

5 years ago
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.
(Assignee)

Comment 3

5 years ago
(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?
(Assignee)

Comment 4

5 years ago
Comment on attachment 586399 [details] [diff] [review]
patch, v1

wicked doesn't seem to be around these days.
Attachment #586399 - Flags: review?(mkanat)

Comment 5

5 years ago
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?
Attachment #586399 - Flags: review?(wicked)
Attachment #586399 - Flags: review?(mkanat)
Attachment #586399 - Flags: review+
(Assignee)

Comment 6

5 years ago
(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!
Flags: approval4.2+
Flags: approval+
(Assignee)

Comment 7

5 years ago
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.
Attachment #586399 - Attachment is obsolete: true
Attachment #589060 - Flags: review?(mkanat)

Comment 8

5 years ago
Comment on attachment 589060 [details] [diff] [review]
patch, v2

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

Beautiful! Very nice. :-)
Attachment #589060 - Flags: review?(mkanat) → review+
(Assignee)

Comment 9

5 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.