Bug 285722: Updates From 2.18- to 2.20+ will not work

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=Tomas.Kopal, a=justdave

4 files changed, 71 insertions, 307 deletions

diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index f1747b39d..b6025791f 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -361,24 +361,29 @@ sub bz_add_column {
 }
 
 sub bz_alter_column {
-    my ($self, $table, $name, $new_def) = @_;
+    my ($self, $table, $name, $new_def, $set_nulls_to) = @_;
 
     my $current_def = $self->bz_column_info($table, $name);
 
     if (!$self->_bz_schema->columns_equal($current_def, $new_def)) {
-        # You can't change a column to be NOT NULL if you have no DEFAULT,
-        # if there are any NULL values in that column.
-        if ($new_def->{NOTNULL} && !exists $new_def->{DEFAULT}) {
+        # You can't change a column to be NOT NULL if you have no DEFAULT
+        # and no value for $set_nulls_to, if there are any NULL values 
+        # in that column.
+        if ($new_def->{NOTNULL} && 
+            !exists $new_def->{DEFAULT} && !defined $set_nulls_to)
+        {
             # Check for NULLs
             my $any_nulls = $self->selectrow_array(
                 "SELECT 1 FROM $table WHERE $name IS NULL");
             if ($any_nulls) {
                 die "You cannot alter the ${table}.${name} column to be"
-                    . " NOT NULL without\nspecifying a default, because"
-                    . " there are NULL values currently in it.";
+                    . " NOT NULL without\nspecifying a default or"
+                    . " something for \$set_nulls_to, because"
+                    . " there are\nNULL values currently in it.";
             }
         }
-        $self->bz_alter_column_raw($table, $name, $new_def, $current_def);
+        $self->bz_alter_column_raw($table, $name, $new_def, $current_def,
+                                   $set_nulls_to);
         $self->_bz_real_schema->set_column($table, $name, $new_def);
         $self->_bz_store_real_schema;
     }
@@ -402,12 +407,15 @@ sub bz_alter_column {
 #              $current_def - (optional) The current definition of the
 #                             column. Will be used in the output message,
 #                             if given.
+#              $set_nulls_to - The same as the param of the same name
+#                              from bz_alter_column.
 # Returns:     nothing
 #
 sub bz_alter_column_raw {
-    my ($self, $table, $name, $new_def, $current_def) = @_;
+    my ($self, $table, $name, $new_def, $current_def, $set_nulls_to) = @_;
     my @statements = $self->_bz_real_schema->get_alter_column_ddl(
-        $table, $name, $new_def);
+        $table, $name, $new_def,
+        defined $set_nulls_to ? $self->quote($set_nulls_to) : undef);
     my $new_ddl = $self->_bz_schema->get_type_ddl($new_def);
     print "Updating column $name in table $table ...\n";
     if (defined $current_def) {
@@ -418,27 +426,13 @@ sub bz_alter_column_raw {
     $self->do($_) foreach (@statements);
 }
 
-
-# XXX - Need to make this cross-db compatible
-# XXX - This shouldn't print stuff to stdout
-sub bz_add_field ($$$) {
-    my ($self, $table, $field, $definition) = @_;
-
-    my $ref = $self->bz_get_field_def($table, $field);
-    return if $ref; # already added?
-
-    print "Adding new field $field to table $table ...\n";
-    $self->do("ALTER TABLE $table
-              ADD COLUMN $field $definition");
-}
-
 sub bz_add_index {
     my ($self, $table, $name, $definition) = @_;
 
     my $index_exists = $self->bz_index_info($table, $name);
 
     if (!$index_exists) {
-        $self->_bz_add_index_raw($table, $name, $definition);
+        $self->bz_add_index_raw($table, $name, $definition);
         $self->_bz_real_schema->set_index($table, $name, $definition);
         $self->_bz_store_real_schema;
     }
@@ -502,31 +496,6 @@ sub _bz_add_table_raw {
     $self->do($_) foreach (@statements);
 }
 
-# XXX - Need to make this cross-db compatible
-# XXX - This shouldn't print stuff to stdout
-sub bz_change_field_type ($$$) {
-    my ($self, $table, $field, $newtype) = @_;
-
-    my $ref = $self->bz_get_field_def($table, $field);
-
-    my $oldtype = $ref->[1];
-    if (! $ref->[2]) {
-        $oldtype .= qq{ not null};
-    }
-    if ($ref->[4]) {
-        $oldtype .= qq{ default "$ref->[4]"};
-    }
-
-    if ($oldtype ne $newtype) {
-        print "Updating field type $field in table $table ...\n";
-        print "old: $oldtype\n";
-        print "new: $newtype\n";
-        $self->do("ALTER TABLE $table
-                  CHANGE $field
-                  $field $newtype");
-    }
-}
-
 sub bz_drop_column {
     my ($self, $table, $column) = @_;
 
@@ -544,26 +513,13 @@ sub bz_drop_column {
     }
 }
 
-# XXX - Need to make this cross-db compatible
-# XXX - This shouldn't print stuff to stdout
-sub bz_drop_field ($$) {
-    my ($self, $table, $field) = @_;
-
-    my $ref = $self->bz_get_field_def($table, $field);
-    return unless $ref; # already dropped?
-
-    print "Deleting unused field $field from table $table ...\n";
-    $self->do("ALTER TABLE $table
-              DROP COLUMN $field");
-}
-
 sub bz_drop_index {
     my ($self, $table, $name) = @_;
 
     my $index_exists = $self->bz_index_info($table, $name);
 
     if ($index_exists) {
-        $self->_bz_drop_index_raw($table, $name);
+        $self->bz_drop_index_raw($table, $name);
         $self->_bz_real_schema->delete_index($table, $name);
         $self->_bz_store_real_schema;        
     }
@@ -607,37 +563,6 @@ sub bz_drop_table {
     }
 }
 
-
-# XXX - Needs to be made cross-db compatible
-sub bz_drop_table_indexes ($) {
-    my ($self, $table) = @_;
-    my %seen;
-
-    # get the list of indexes
-    my $sth = $self->prepare("SHOW INDEX FROM $table");
-    $sth->execute;
-
-    # drop each index
-    while ( my $ref = $sth->fetchrow_arrayref) {
-
-      # note that some indexes are described by multiple rows in the
-      # index table, so we may have already dropped the index described
-      # in the current row.
-      next if exists $seen{$$ref[2]};
-
-      if ($$ref[2] eq 'PRIMARY') {
-          # The syntax for dropping a PRIMARY KEY is different
-          # from the normal DROP INDEX syntax.
-          $self->do("ALTER TABLE $table DROP PRIMARY KEY");
-      }
-      else {
-          $self->do("ALTER TABLE $table DROP INDEX $$ref[2]");
-      }
-      $seen{$$ref[2]} = 1;
-
-    }
-}
-
 sub bz_rename_column {
     my ($self, $table, $old_name, $new_name) = @_;
 
@@ -660,24 +585,6 @@ sub bz_rename_column {
     }
 }
 
-# XXX - Needs to be made cross-db compatible
-sub bz_rename_field ($$$) {
-    my ($self, $table, $field, $newname) = @_;
-
-    my $ref = $self->bz_get_field_def($table, $field);
-    return unless $ref; # already renamed?
-
-    if ($$ref[1] ne $newname) {
-        print "Updating field $field in table $table ...\n";
-        my $type = $$ref[1];
-        $type .= " NOT NULL" if !$$ref[2];
-        $type .= " auto_increment" if $$ref[5] =~ /auto_increment/;
-        $self->do("ALTER TABLE $table
-                  CHANGE $field
-                  $newname $type");
-    }
-}
-
 #####################################################################
 # Schema Information Methods
 #####################################################################
@@ -726,71 +633,11 @@ sub bz_table_info {
 }
 
 
-# XXX - Needs to be made cross-db compatible.
-sub bz_get_field_def ($$) {
-    my ($self, $table, $field) = @_;
-
-    if ($self->bz_table_exists($table)) {
-
-        my $sth = $self->prepare("SHOW COLUMNS FROM $table");
-        $sth->execute;
-
-        while (my $ref = $sth->fetchrow_arrayref) {
-            next if $$ref[0] ne $field;
-            return $ref;
-        }
-    }
-    return undef;
-}
-
-# XXX - Needs to be made cross-db compatible
-sub bz_get_index_count ($) {
-    my ($self, $table) = @_;
-
-    my $sth = $self->prepare("SHOW INDEX FROM $table");
-    $sth->execute;
-
-    if ( $sth->rows == -1 ) {
-      die ("Unexpected response while counting indexes in $table:" .
-           " \$sth->rows == -1");
-    }
-
-    return ($sth->rows);
-}
-
-# XXX - Needs to be made cross-db compatible.
-sub bz_get_index_def ($$) {
-    my ($self, $table, $field) = @_;
-    my $sth = $self->prepare("SHOW INDEX FROM $table");
-    $sth->execute;
-
-    while (my $ref = $sth->fetchrow_arrayref) {
-        next if $$ref[4] ne $field;
-        return $ref;
-   }
-}
-
-# XXX - Should be updated to use _bz_real_schema when we have that,
-#       if we ever need it.
 sub bz_table_columns {
     my ($self, $table) = @_;
     return $self->_bz_schema->get_table_columns($table);
 }
 
-# XXX - Needs to be made cross-db compatible
-sub bz_table_exists ($) {
-   my ($self, $table) = @_;
-   my $exists = 0;
-   my $sth = $self->prepare("SHOW TABLES");
-   $sth->execute;
-   while (my ($dbtable) = $sth->fetchrow_array ) {
-      if ($dbtable eq $table) {
-         $exists = 1;
-      }
-   }
-   return $exists;
-}
-
 #####################################################################
 # Protected "Real Database" Schema Information Methods
 #####################################################################
@@ -1046,26 +893,16 @@ Bugzilla::DB - Database access routines, using L<DBI>
   $dbh->bz_add_table($name);
   $dbh->bz_drop_index($table, $name);
   $dbh->bz_drop_table($name);
-  $dbh->bz_alter_column($table, $name, \%new_def);
+  $dbh->bz_alter_column($table, $name, \%new_def, $set_nulls_to);
   $dbh->bz_drop_column($table, $column);
   $dbh->bz_rename_column($table, $old_name, $new_name);
 
-  # Schema Modification (DEPRECATED)
-  $dbh->bz_add_field($table, $column, $definition);
-  $dbh->bz_change_field_type($table, $column, $newtype);
-  $dbh->bz_drop_field($table, $column);
-  $dbh->bz_drop_table_indexes($table);
-  $dbh->bz_rename_field($table, $column, $newname);
-
   # Schema Information
   my $column = $dbh->bz_column_info($table, $column);
   my $index  = $dbh->bz_index_info($table, $index);
 
-  # Schema Information (DEPRECATED)
+  # General Information
   my @fields    = $dbh->bz_get_field_defs();
-  my @fieldinfo = $dbh->bz_get_field_def($table, $column);
-  my @indexinfo = $dbh->bz_get_index_def($table, $field);
-  my $exists    = $dbh->bz_table_exists($table);
 
 =head1 DESCRIPTION
 
@@ -1412,7 +1249,7 @@ C<Bugzilla::DB::Schema::ABSTRACT_SCHEMA>.
  Params:      $name - The name of the table to drop.
  Returns:     nothing
 
-=item C<bz_alter_column($table, $name, \%new_def)>
+=item C<bz_alter_column($table, $name, \%new_def, $set_nulls_to)>
 
  Description: Changes the data type of a column in a table. Prints out
               the changes being made to stdout. If the new type is the
@@ -1422,6 +1259,11 @@ C<Bugzilla::DB::Schema::ABSTRACT_SCHEMA>.
               $name    = the name of the column you want to change
               $new_def = An abstract column definition for the new 
                          data type of the columm
+              $set_nulls_to = (Optional) If you are changing the column
+                              to be NOT NULL, you probably also want to
+                              set any existing NULL columns to a particular
+                              value. Specify that value here. 
+                              NOTE: The value should not already be SQL-quoted.
  Returns:     nothing
 
 =item C<bz_drop_column($table, $column)>
@@ -1449,61 +1291,6 @@ C<Bugzilla::DB::Schema::ABSTRACT_SCHEMA>.
 =back
 
 
-=head2 Deprecated Schema Modification Methods
-
-These methods modify the current Bugzilla schema, for MySQL only. 
-Do not use them in new code.
-
-=over 4
-
-=item C<bz_add_field>
-
- Description: Adds a new column to a table in the database. Prints out
-              a brief statement that it did so, to stdout.
- Params:      $table = the table where the column is being added
-              $column = the name of the new column
-              $definition = SQL for defining the new column
- Returns:     none
-
-=item C<bz_change_field_type>
-
- Description: Changes the data type of a column in a table. Prints out 
-              the changes being made to stdout. If the new type is the 
-              same as the old type, the function returns without changing
-              anything.
- Params:      $table = the table where the column is
-              $column = the name of the column you want to change
-              $newtype = the new data type of the column, in SQL format.
- Returns:     none
-
-=item C<bz_drop_field>
-
- Description: Removes a column from a database table. If the column 
-              doesn't exist, we return without doing anything. If we do
-              anything, we print a short message to stdout about the change.
- Params:      $table = The table where the column is
-              $column = The name of the column you want to drop
- Returns:     none
-
-=item C<bz_drop_table_indexes>
-
- Description: Drops all indexes on a given table.
- Params:      $table = the table on which you wish to remove all indexes
- Returns:     none
-
-=item C<bz_rename_field>
-
- Description: Renames a column in a database table. If the column 
-              doesn't exist, or if the new name is the same as the 
-              old name, we return without changing anything.
- Params:      $table = the table where the column is that you want to rename
-              $column = the name of the column you want to rename
-              $newname = the new name of the column
- Returns:     none
-
-=back
-
-
 =head2 Schema Information Methods
 
 These methods return information about the current Bugzilla database
@@ -1551,53 +1338,6 @@ MySQL only.
  Params:      none
  Returns:     List of all the "bug" fields
 
-=item C<bz_get_field_def>
-
- Description: Returns information about a column in a table in the database.
- Params:      $table = name of table containing the column (scalar)
-              $column = column you want info about (scalar)
- Returns:     An reference to an array containing information about the
-              field, with the following information in each array place:
-              0 = column name
-              1 = column type
-              2 = 'YES' if the column can be NULL, empty string otherwise
-              3 = The type of key, either 'MUL', 'UNI', 'PRI, or ''.
-              4 = The default value
-              5 = An "extra" column, per MySQL docs. Don't use it.
-              If the column does not exist, the function returns undef.
-
-=item C<bz_get_index_count>
-
- Description: Returns the number of indexes on a certain table.
- Params:      $table = the table that you want to count indexes on
- Returns:     The number of indexes on the table.
-
-=item C<bz_get_index_def($table, $field)>
-
- Description: Returns information about an index on a table in the database.
- Params:      $table = name of table containing the index (scalar)
-              $field = name of a field that the index is on (scalar)
- Returns:     A reference to an array containing information about the
-              index, with the following information in each array place:
-              0 = name of the table that the index is on
-              1 = 0 if unique, 1 if not unique
-              2 = name of the index
-              3 = seq_in_index (either 1 or 0)
-              4 = Name of ONE column that the index is on
-              5 = 'Collation' of the index. Usually 'A'.
-              6 = Cardinality. Either a number or undef.
-              7 = sub_part. Usually undef. Sometimes 1.
-              8 = "packed". Usually undef.
-              9 = comments. Usually an empty string. Sometimes 'FULLTEXT'.
-              If the index does not exist, the function returns undef.
-
-=item C<bz_table_exists>
-
- Description: Returns whether or not the specified table exists in the DB.
- Params:      $table = the name of the table you're checking the existence
-                       of (scalar)
- Returns:     A true value if the table exists, a false value otherwise.
-
 =head2 Transaction Methods
 
 These methods deal with the starting and stopping of transactions 
diff --git a/Bugzilla/DB/Mysql.pm b/Bugzilla/DB/Mysql.pm
index 921b27195..08d72e4d3 100644
--- a/Bugzilla/DB/Mysql.pm
+++ b/Bugzilla/DB/Mysql.pm
@@ -359,7 +359,7 @@ sub bz_setup_database {
                     # Fix the name to fit in with the new naming scheme.
                     my $new_name = $table . "_" .
                                    $index->{FIELDS}->[0] . "_idx";
-                    print "Renaming index $column to to $new_name...\n";
+                    print "Renaming index $column to $new_name...\n";
                     # Unfortunately, MySQL has no way to rename an index. :-(
                     # So we have to drop and recreate the indexes.
                     $self->bz_drop_index_raw($table, $column, "silent");
@@ -370,6 +370,11 @@ sub bz_setup_database {
         } # foreach table
     } # if old-name indexes
 
+
+    # And now we create the tables and the Schema object.
+    $self->SUPER::bz_setup_database();
+
+
     # The old timestamp fields need to be adjusted here instead of in
     # checksetup. Otherwise the UPDATE statements inside of bz_add_column
     # will cause accidental timestamp updates.
@@ -378,8 +383,8 @@ sub bz_setup_database {
     # 2002-08-14 - bbaetz@student.usyd.edu.au - bug 153578
     # attachments creation time needs to be a datetime, not a timestamp
     my $attach_creation = 
-        $self->bz_get_field_def("attachments", "creation_ts");
-    if ($attach_creation && $attach_creation->[1] =~ /^timestamp/) {
+        $self->bz_column_info("attachments", "creation_ts");
+    if ($attach_creation && $attach_creation->{TYPE} =~ /^TIMESTAMP/i) {
         print "Fixing creation time on attachments...\n";
 
         my $sth = $self->prepare("SELECT COUNT(attach_id) FROM attachments");
@@ -424,26 +429,26 @@ sub bz_setup_database {
         }
         print "Done - converted $i attachments\n";
 
-        $self->bz_change_field_type("attachments", "creation_ts", 
-            'datetime NOT NULL');
+        $self->bz_alter_column("attachments", "creation_ts", 
+                               {TYPE => 'DATETIME', NOTNULL => 1});
     }
 
     # 2004-08-29 - Tomas.Kopal@altap.cz, bug 257303
     # Change logincookies.lastused type from timestamp to datetime
-    my $login_lastused = $self->bz_get_field_def("logincookies", "lastused");
-    if ($login_lastused && $login_lastused->[1] =~ /^timestamp/) {
-        $self->bz_change_field_type('logincookies', 'lastused', 
-                                    'DATETIME NOT NULL');
+    my $login_lastused = $self->bz_column_info("logincookies", "lastused");
+    if ($login_lastused && $login_lastused->{TYPE} =~ /^TIMESTAMP/i) {
+        $self->bz_alter_column('logincookies', 'lastused', 
+                               { TYPE => 'DATETIME',  NOTNULL => 1});
     }
 
     # 2005-01-17 - Tomas.Kopal@altap.cz, bug 257315
     # Change bugs.delta_ts type from timestamp to datetime 
-    my $bugs_deltats = $self->bz_get_field_def("bugs", "delta_ts");
-    if ($bugs_deltats && $bugs_deltats->[1] =~ /^timestamp/) {
-        $self->bz_change_field_type('bugs', 'delta_ts', 'DATETIME NOT NULL');
+    my $bugs_deltats = $self->bz_column_info("bugs", "delta_ts");
+    if ($bugs_deltats && $bugs_deltats->{TYPE} =~ /^TIMESTAMP/i) {
+        $self->bz_alter_column('bugs', 'delta_ts', 
+                               {TYPE => 'DATETIME', NOTNULL => 1});
     }
 
-    $self->SUPER::bz_setup_database();
 }
 
 
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index e0b49d859..a58c20ade 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -1434,11 +1434,16 @@ sub get_alter_column_ddl {
               $column - The name of the column being changed.
               \%definition - The new definition for the column,
                   in standard C<ABSTRACT_SCHEMA> format.
+              $set_nulls_to - A value to set NULL values to, if
+                  your new definition is NOT NULL and contains
+                  no DEFAULT, and when there is a possibility
+                  that the column could contain NULLs. $set_nulls_to
+                  should be already SQL-quoted if necessary.
  Returns:     An array of SQL statements.
 
 =cut
 
-    my ($self, $table, $column, $new_def) = @_;
+    my ($self, $table, $column, $new_def, $set_nulls_to) = @_;
 
     my @statements;
     my $old_def = $self->get_column_abstract($table, $column);
@@ -1489,10 +1494,17 @@ sub get_alter_column_ddl {
     # If we went from NULL to NOT NULL
     # OR if we changed the type and we are NOT NULL
     if ( (!$old_def->{NOTNULL} && $new_def->{NOTNULL}) ||
-         ($typechange && $new_def->{NOTNULL}) ) {
-        if (exists $new_def->{DEFAULT}) {
-            # Handle any fields that were NULL before, if we have a default.
-            push(@statements, "UPDATE $table SET $column = $default"
+         ($typechange && $new_def->{NOTNULL}) ) 
+    {
+        my $setdefault;
+        # Handle any fields that were NULL before, if we have a default,
+        $setdefault = $new_def->{DEFAULT} if exists $new_def->{DEFAULT};
+        # But if we have a set_nulls_to, that overrides the DEFAULT 
+        # (although nobody would usually specify both a default and 
+        # a set_nulls_to.)
+        $setdefault = $set_nulls_to if defined $set_nulls_to;
+        if (defined $setdefault) {
+            push(@statements, "UPDATE $table SET $column = $setdefault"
                             . "  WHERE $column IS NULL");
         }
         push(@statements, "ALTER TABLE $table ALTER COLUMN $column"
diff --git a/Bugzilla/DB/Schema/Mysql.pm b/Bugzilla/DB/Schema/Mysql.pm
index 1ea1d285a..21274f575 100644
--- a/Bugzilla/DB/Schema/Mysql.pm
+++ b/Bugzilla/DB/Schema/Mysql.pm
@@ -151,9 +151,14 @@ sub _get_create_index_ddl {
 
 # MySQL has a simpler ALTER TABLE syntax than ANSI.
 sub get_alter_column_ddl {
-    my ($self, $table, $column, $new_def) = @_;
+    my ($self, $table, $column, $new_def, $set_nulls_to) = @_;
     my $new_ddl = $self->get_type_ddl($new_def);
-    return (("ALTER TABLE $table CHANGE COLUMN $column $column $new_ddl"));
+    my @statements;
+    push(@statements, "UPDATE $table SET $column = $set_nulls_to
+                        WHERE $column IS NULL") if defined $set_nulls_to;
+    push(@statements, "ALTER TABLE $table CHANGE COLUMN 
+                       $column $column $new_ddl");
+    return @statements;
 }
 
 sub get_drop_index_ddl {
@@ -281,6 +286,8 @@ sub column_info_to_column {
 sub get_rename_column_ddl {
     my ($self, $table, $old_name, $new_name) = @_;
     my $def = $self->get_type_ddl($self->get_column($table, $old_name));
+    # MySQL doesn't like having the PRIMARY KEY statement in a rename.
+    $def =~ s/PRIMARY KEY//i;
     return ("ALTER TABLE $table CHANGE COLUMN $old_name $new_name $def");
 }