Bug 285748: Cross-DB bz_alter_column (Part of Bug 285111)

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

4 files changed, 159 insertions, 9 deletions

diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 5256a5434..d84ce873d 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -357,6 +357,27 @@ sub bz_add_column {
     }
 }
 
+sub bz_alter_column {
+    my ($self, $table, $name, $new_def) = @_;
+
+    my $current_def = $self->bz_column_info($table, $name);
+
+    if (!$self->_bz_schema->columns_equal($current_def, $new_def)) {
+        my @statements = $self->_bz_real_schema->get_alter_column_ddl(
+            $table, $name, $new_def);
+        my $old_ddl = $self->_bz_schema->get_type_ddl($current_def);
+        my $new_ddl = $self->_bz_schema->get_type_ddl($new_def);
+        print "Updating column $name in table $table ...\n";
+        print "Old: $old_ddl\n";
+        print "New: $new_ddl\n";
+        foreach my $sql (@statements) {
+            $self->do($sql);
+        }
+        $self->_bz_real_schema->set_column($table, $name, $new_def);
+        $self->_bz_store_real_schema;
+    }
+}
+
 
 # XXX - Need to make this cross-db compatible
 # XXX - This shouldn't print stuff to stdout
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index d4a59ed78..525ab99f6 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -1359,6 +1359,7 @@ sub _get_create_index_ddl {
 #--------------------------------------------------------------------------
 
 sub get_add_column_ddl {
+
 =item C<get_alter_ddl($table, $column, \%definition)>
 
  Description: Generate SQL to add a column to a table.
@@ -1377,10 +1378,101 @@ sub get_add_column_ddl {
     return ($statement);
 }
 
+sub get_alter_column_ddl {
+
+=item C<get_alter_ddl($table, $column, \%definition)>
+
+ Description: Generate SQL to alter a column in a table.
+              The column that you are altering must exist,
+              and the table that it lives in must exist.
+ Params:      $table - The table containing the column.
+              $column - The name of the column being changed.
+              \%definition - The new definition for the column,
+                  in standard C<ABSTRACT_SCHEMA> format.
+ Returns:     An array of SQL statements.
+
+=cut
+
+    my ($self, $table, $column, $new_def) = @_;
+
+    my @statements;
+    my $old_def = $self->get_column_abstract($table, $column);
+    my $specific = $self->{db_specific};
+
+    my $typechange = 0;
+    # If the types have changed, we have to deal with that.
+    if (uc(trim($old_def->{TYPE})) ne uc(trim($new_def->{TYPE}))) {
+        $typechange = 1;
+        my $type = $new_def->{TYPE};
+        $type = $specific->{$type} if exists $specific->{$type};
+        # Make sure we can CAST from the old type to the new without an error.
+        push(@statements, "SELECT CAST($column AS $type) FROM $table LIMIT 1");
+        # Add a new temporary column of the new type
+        push(@statements, "ALTER TABLE $table ADD COLUMN ${column}_ALTERTEMP"
+                        . " $type");
+        # UPDATE the temp column to have the same values as the old column
+        push(@statements, "UPDATE $table SET ${column}_ALTERTEMP = " 
+                        . " CAST($column AS $type)");
+        # DROP the old column
+        push(@statements, "ALTER TABLE $table DROP COLUMN $column");
+        # And rename the temp column to be the new one.
+        push(@statements, "ALTER TABLE $table RENAME COLUMN "
+                        . " ${column}_ALTERTEMP TO $column");
+
+        # FIXME - And now, we have to regenerate any indexes that got
+        #         dropped, except for the PK index which will be handled
+        #         below.
+    }
+
+    my $default = $new_def->{DEFAULT};
+    my $default_old = $old_def->{DEFAULT};
+    # If we went from having a default to not having one
+    if (!defined $default && defined $default_old) {
+        push(@statements, "ALTER TABLE $table ALTER COLUMN $column"
+                        . " DROP DEFAULT");
+    }
+    # If we went from no default to a default, or we changed the default,
+    # or we have a default and we changed the data type of the field
+    elsif ( (defined $default && !defined $default_old) || 
+         ($default ne $default_old) ||
+         ($typechange && defined $new_def->{DEFAULT}) ) {
+        $default = $specific->{$default} if exists $specific->{$default};
+        push(@statements, "ALTER TABLE $table ALTER COLUMN $column "
+                         . " SET DEFAULT $default");
+    }
+
+    # 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}) ) {
+        push(@statements, "ALTER TABLE $table ALTER COLUMN $column"
+                          . " SET NOT NULL");
+    }
+    # If we went from NOT NULL to NULL
+    elsif ($old_def->{NOTNULL} && !$new_def->{NOTNULL}) {
+        push(@statements, "ALTER TABLE $table ALTER COLUMN $column"
+                        . " DROP NOT NULL");
+    }
+
+    # If we went from not being a PRIMARY KEY to being a PRIMARY KEY,
+    # or if we changed types and we are a PK.
+    if ( (!$old_def->{PRIMARYKEY} && $new_def->{PRIMARYKEY}) ||
+         ($typechange && $new_def->{PRIMARYKEY}) ) {
+        push(@statements, "ALTER TABLE $table ADD PRIMARY KEY ($column)");
+    }
+    # If we went from being a PK to not being a PK
+    elsif ( $old_def->{PRIMARYKEY} && !$new_def->{PRIMARYKEY} ) {
+        push(@statements, "ALTER TABLE $table DROP PRIMARY KEY");
+    }
+
+    return @statements;
+}
+
 sub get_column_abstract {
 
 =item C<get_column_abstract($table, $column)>
 
+
  Description: A column definition from the abstract internal schema.
               cross-database format.
  Params:      $table - The name of the table
@@ -1421,7 +1513,7 @@ sub get_index_abstract {
     # table doesn't exist.
     if (exists $self->{abstract_schema}->{$table}) {
         my %indexes = (@{ $self->{abstract_schema}{$table}{INDEXES} });
-        return $indexes{$index};
+        return dclone($indexes{$index});
     }
     return undef;
 }
@@ -1446,26 +1538,52 @@ sub set_column {
 
     my ($self, $table, $column, $new_def) = @_;
 
-    my $fields = \@{ $self->{schema}{$table}{FIELDS} };
     my $abstract_fields = \@{ $self->{abstract_schema}{$table}{FIELDS} };
 
-    my $field_position = lsearch($fields, $column) + 1;
+    my $field_position = lsearch($abstract_fields, $column) + 1;
     # If the column doesn't exist, then add it.
     if (!$field_position) {
-        push(@$fields, $column);
-        push(@$fields, $new_def);
         push(@$abstract_fields, $column);
         push(@$abstract_fields, $new_def);
     }
     # We're modifying an existing column.
     else {
-        splice(@$fields, $field_position, 1, $new_def);
         splice(@$abstract_fields, $field_position, 1, $new_def);
     }
 
+    $self->{schema} = dclone($self->{abstract_schema});
     $self->_adjust_schema();
 }
 
+sub columns_equal {
+
+=item C<columns_equal($col_one, $col_two)>
+
+ Description: Tells you if two columns have entirely identical definitions.
+              The TYPE field's value will be compared case-insensitive.
+              However, all other fields will be case-sensitive.
+ Params:      $col_one, $col_two - The columns to compare. Hash 
+                  references, in C<ABSTRACT_SCHEMA> format.
+ Returns:     C<1> if the columns are identical, C<0> if they are not.
+=cut
+
+    my $self = shift;
+    my $col_one = dclone(shift);
+    my $col_two = dclone(shift);
+
+    $col_one->{TYPE} = uc($col_one->{TYPE});
+    $col_two->{TYPE} = uc($col_two->{TYPE});
+
+    my @col_one_array = %$col_one;
+    my @col_two_array = %$col_two;
+
+    my ($removed, $added) = diff_arrays(\@col_one_array, \@col_two_array);
+
+    # If there are no differences between the arrays,
+    # then they are equal.
+    return !scalar(@$removed) && !scalar(@$added) ? 1 : 0;
+}
+
 
 =head1 SERIALIZATION/DESERIALIZATION
 
diff --git a/Bugzilla/DB/Schema/Mysql.pm b/Bugzilla/DB/Schema/Mysql.pm
index dff626097..fa6e2d5ba 100644
--- a/Bugzilla/DB/Schema/Mysql.pm
+++ b/Bugzilla/DB/Schema/Mysql.pm
@@ -96,5 +96,16 @@ sub _get_create_index_ddl {
     return($sql);
 
 } #eosub--_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 $new_ddl = $self->get_type_ddl($new_def);
+
+    return (("ALTER TABLE $table CHANGE COLUMN $column $column $new_ddl"));
+}
+
 1;
diff --git a/Bugzilla/DB/Schema/Pg.pm b/Bugzilla/DB/Schema/Pg.pm
index 3a45744bb..59072e79d 100644
--- a/Bugzilla/DB/Schema/Pg.pm
+++ b/Bugzilla/DB/Schema/Pg.pm
@@ -115,8 +115,8 @@ sub get_add_column_ddl {
     }
 
     if ($definition->{PRIMARYKEY}) {
-         push(@statements, "ALTER TABLE $table ALTER COLUMN $column "
-                         . " SET ADD PRIMARY KEY");
+         push(@statements, "ALTER TABLE $table ALTER COLUMN "
+                         . " ADD PRIMARY KEY ($column)");
     }
 
     return @statements;