Bug 683644: Foreign keys aren't renamed correctly when DB tables are renamed

r=wicked a=LpSolit

6 files changed, 120 insertions, 29 deletions

diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 083a1c208..0c841632f 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -548,7 +548,7 @@ sub bz_setup_foreign_keys {
             # prior to 4.2, and also to handle problems caused
             # by enabling an extension pre-4.2, disabling it for
             # the 4.2 upgrade, and then re-enabling it later.
-            if (!$fk) {
+            unless ($fk && $fk->{created}) {
                 my $standard_def = 
                     $self->_bz_schema->get_column_abstract($table, $column);
                 if (exists $standard_def->{REFERENCES}) {
@@ -1058,6 +1058,18 @@ sub bz_rename_table {
     my $new = $self->bz_table_info($new_name);
     ThrowCodeError('db_rename_conflict', { old => $old_name,
                                            new => $new_name }) if $new;
+
+    # FKs will all have the wrong names unless we drop and then let them
+    # be re-created later. Under normal circumstances, checksetup.pl will
+    # automatically re-create these dropped FKs at the end of its DB upgrade
+    # run, so we don't need to re-create them in this method.
+    my @columns = $self->bz_table_columns($old_name);
+    foreach my $column (@columns) {
+        # these just return silently if there's no FK to drop
+        $self->bz_drop_fk($old_name, $column);
+        $self->bz_drop_related_fks($old_name, $column);
+    }
+
     my @sql = $self->_bz_real_schema->get_rename_table_sql($old_name, $new_name);
     print get_text('install_table_rename', 
                    { old => $old_name, new => $new_name }) . "\n"
diff --git a/Bugzilla/DB/Oracle.pm b/Bugzilla/DB/Oracle.pm
index 711b84141..2cbd19a82 100644
--- a/Bugzilla/DB/Oracle.pm
+++ b/Bugzilla/DB/Oracle.pm
@@ -648,6 +648,10 @@ sub bz_setup_database {
                 my $fk_name = $self->_bz_schema->_get_fk_name($table,
                                                               $column,
                                                               $references);
+                # bz_rename_table didn't rename the trigger correctly.
+                if ($table eq 'bug_tag' && $to_table eq 'tags') {
+                    $to_table = 'tag';
+                }
                 if ( $update =~ /CASCADE/i ){
                      my $trigger_name = uc($fk_name . "_UC");
                      my $exist_trigger = $self->selectcol_arrayref(
diff --git a/Bugzilla/DB/Pg.pm b/Bugzilla/DB/Pg.pm
index e59a638a4..b6be64011 100644
--- a/Bugzilla/DB/Pg.pm
+++ b/Bugzilla/DB/Pg.pm
@@ -282,14 +282,18 @@ END
     $self->bz_add_index('products', 'products_name_lower_idx',
         {FIELDS => ['LOWER(name)'], TYPE => 'UNIQUE'});
 
-    # bz_rename_column didn't correctly rename the sequence.
-    if ($self->bz_column_info('fielddefs', 'id')
-        && $self->bz_sequence_exists('fielddefs_fieldid_seq')) 
-    {
-        print "Fixing fielddefs_fieldid_seq sequence...\n";
-        $self->do("ALTER TABLE fielddefs_fieldid_seq RENAME TO fielddefs_id_seq");
-        $self->do("ALTER TABLE fielddefs ALTER COLUMN id
-                    SET DEFAULT NEXTVAL('fielddefs_id_seq')");
+    # bz_rename_column and bz_rename_table didn't correctly rename
+    # the sequence.
+    $self->_fix_bad_sequence('fielddefs', 'id', 'fielddefs_fieldid_seq', 'fielddefs_id_seq');
+    # If the 'tags' table still exists, then bz_rename_table()
+    # will fix the sequence for us.
+    if (!$self->bz_table_info('tags')) {
+        my $res = $self->_fix_bad_sequence('tag', 'id', 'tags_id_seq', 'tag_id_seq');
+        # If $res is true, then the sequence has been renamed, meaning that
+        # the primary key must be renamed too.
+        if ($res) {
+            $self->do('ALTER INDEX tags_pkey RENAME TO tag_pkey');
+        }
     }
 
     # Certain sequences got upgraded before we required Pg 8.3, and
@@ -320,6 +324,20 @@ END
     }
 }
 
+sub _fix_bad_sequence {
+    my ($self, $table, $column, $old_seq, $new_seq) = @_;
+    if ($self->bz_column_info($table, $column)
+        && $self->bz_sequence_exists($old_seq))
+    {
+        print "Fixing $old_seq sequence...\n";
+        $self->do("ALTER SEQUENCE $old_seq RENAME TO $new_seq");
+        $self->do("ALTER TABLE $table ALTER COLUMN $column
+                    SET DEFAULT NEXTVAL('$new_seq')");
+        return 1;
+    }
+    return 0;
+}
+
 # Renames things that differ only in case.
 sub _fix_case_differences {
     my ($table, $field) = @_;
diff --git a/Bugzilla/DB/Schema/Oracle.pm b/Bugzilla/DB/Schema/Oracle.pm
index 6011cecfc..cdc39409d 100644
--- a/Bugzilla/DB/Schema/Oracle.pm
+++ b/Bugzilla/DB/Schema/Oracle.pm
@@ -351,17 +351,10 @@ sub get_rename_column_ddl {
     my $def = $self->get_column_abstract($table, $old_name);
     if ($def->{TYPE} =~ /SERIAL/i) {
         # We have to rename the series also, and fix the default of the series.
-        push(@sql, "RENAME ${table}_${old_name}_SEQ TO 
-                      ${table}_${new_name}_seq");
-        my $serial_sql =
-                       "CREATE OR REPLACE TRIGGER ${table}_${new_name}_TR "
-                     . " BEFORE INSERT ON ${table} "
-                     . " FOR EACH ROW "
-                     . " BEGIN "
-                     . "   SELECT ${table}_${new_name}_SEQ.NEXTVAL "
-                     . "   INTO :NEW.${new_name} FROM DUAL; "
-                     . " END;";
-        push(@sql, $serial_sql);
+        my $old_seq = "${table}_${old_name}_SEQ";
+        my $new_seq = "${table}_${new_name}_SEQ";
+        push(@sql, "RENAME $old_seq TO $new_seq");
+        push(@sql, $self->_get_create_trigger_ddl($table, $new_name, $new_seq));
         push(@sql, "DROP TRIGGER ${table}_${old_name}_TR");
     }
     if ($def->{TYPE} =~ /varchar|text/i && $def->{NOTNULL} ) {
@@ -371,6 +364,35 @@ sub get_rename_column_ddl {
     return @sql;
 }
 
+sub get_rename_table_sql {
+    my ($self, $old_name, $new_name) = @_;
+    if (lc($old_name) eq lc($new_name)) {
+        # if the only change is a case change, return an empty list.
+        return ();
+    }
+
+    my @sql = ("ALTER TABLE $old_name RENAME TO $new_name");
+    my @columns = $self->get_table_columns($old_name);
+    foreach my $column (@columns) {
+        my $def = $self->get_column_abstract($old_name, $column);
+        if ($def->{TYPE} =~ /SERIAL/i) {
+            # If there's a SERIAL column on this table, we also need
+            # to rename the sequence.
+            my $old_seq = "${old_name}_${column}_SEQ";
+            my $new_seq = "${new_name}_${column}_SEQ";
+            push(@sql, "RENAME $old_seq TO $new_seq");
+            push(@sql, $self->_get_create_trigger_ddl($new_name, $column, $new_seq));
+            push(@sql, "DROP TRIGGER ${old_name}_${column}_TR");
+        }
+        if ($def->{TYPE} =~ /varchar|text/i && $def->{NOTNULL}) {
+            push(@sql, _get_notnull_trigger_ddl($new_name, $column));
+            push(@sql, "DROP TRIGGER ${$old_name}_${column}");
+        }
+    }
+
+    return @sql;
+}
+
 sub _get_notnull_trigger_ddl {
       my ($table, $column) = @_;
 
@@ -398,19 +420,24 @@ sub _get_create_seq_ddl {
                    . " NOMAXVALUE "
                    . " NOCYCLE "
                    . " NOCACHE";
-     my $serial_sql = "CREATE OR REPLACE TRIGGER ${table}_${column}_TR "
-                    . " BEFORE INSERT ON ${table} "
-                    . " FOR EACH ROW "
-                    . " BEGIN "
-                    . "   SELECT ${seq_name}.NEXTVAL "
-                    . "   INTO :NEW.${column} FROM DUAL; "
-                    . " END;";
     push (@ddl, $seq_sql);
-    push (@ddl, $serial_sql);
+    push(@ddl, $self->_get_create_trigger_ddl($table, $column, $seq_name));
 
     return @ddl;
 }
 
+sub _get_create_trigger_ddl {
+    my ($self, $table, $column, $seq_name) = @_;
+    my $serial_sql = "CREATE OR REPLACE TRIGGER ${table}_${column}_TR "
+                   . " BEFORE INSERT ON $table "
+                   . " FOR EACH ROW "
+                   . " BEGIN "
+                   . "   SELECT ${seq_name}.NEXTVAL "
+                   . "   INTO :NEW.$column FROM DUAL; "
+                   . " END;";
+    return $serial_sql;
+}
+
 sub get_set_serial_sql { 
     my ($self, $table, $column, $value) = @_; 
     my @sql;
diff --git a/Bugzilla/DB/Schema/Pg.pm b/Bugzilla/DB/Schema/Pg.pm
index 517837dcc..ef6e5671d 100644
--- a/Bugzilla/DB/Schema/Pg.pm
+++ b/Bugzilla/DB/Schema/Pg.pm
@@ -114,7 +114,30 @@ sub get_rename_table_sql {
         # is case-insensitive and will return an error about a duplicate name
         return ();
     }
-    return ("ALTER TABLE $old_name RENAME TO $new_name");
+
+    my @sql = ("ALTER TABLE $old_name RENAME TO $new_name");
+
+    # If there's a SERIAL column on this table, we also need to rename the
+    # sequence.
+    # If there is a PRIMARY KEY, we need to rename it too.
+    my @columns = $self->get_table_columns($old_name);
+    foreach my $column (@columns) {
+        my $def = $self->get_column_abstract($old_name, $column);
+        if ($def->{TYPE} =~ /SERIAL/i) {
+            my $old_seq = "${old_name}_${column}_seq";
+            my $new_seq = "${new_name}_${column}_seq";
+            push(@sql, "ALTER SEQUENCE $old_seq RENAME TO $new_seq");
+            push(@sql, "ALTER TABLE $new_name ALTER COLUMN $column
+                             SET DEFAULT NEXTVAL('$new_seq')");
+        }
+        if ($def->{PRIMARYKEY}) {
+            my $old_pk = "${old_name}_pkey";
+            my $new_pk = "${new_name}_pkey";
+            push(@sql, "ALTER INDEX $old_pk RENAME to $new_pk");
+        }
+    }
+
+    return @sql;
 }
 
 sub get_set_serial_sql {
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index b98b88077..adff55915 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -3607,6 +3607,13 @@ sub _rename_tags_to_tag {
         $dbh->bz_add_index('tag', 'tag_user_id_idx',
                            {FIELDS => [qw(user_id name)], TYPE => 'UNIQUE'});
     }
+    if (my $bug_tag_fk = $dbh->bz_fk_info('bug_tag', 'tag_id')) {
+        # bz_rename_table() didn't handle FKs correctly.
+        if ($bug_tag_fk->{TABLE} eq 'tags') {
+            $bug_tag_fk->{TABLE} = 'tag';
+            $dbh->bz_alter_fk('bug_tag', 'tag_id', $bug_tag_fk);
+        }
+    }
 }
 
 sub _on_delete_set_null_for_audit_log_userid {