Changes + Reverts for 0.11000, see Changes file for info
[dbsrgits/SQL-Translator.git] / lib / SQL / Translator / Diff.pm
index 2638e83..91b8c1a 100644 (file)
@@ -13,8 +13,8 @@ use base 'Class::Accessor::Fast';
 # Input/option accessors
 __PACKAGE__->mk_accessors(qw/
   ignore_index_names ignore_constraint_names ignore_view_sql
-  ignore_proc_sql output_db source_schema source_db target_schema target_db
-  case_insensitive no_batch_alters ignore_missing_methods
+  ignore_proc_sql output_db source_schema target_schema 
+  case_insensitive no_batch_alters ignore_missing_methods producer_options
 /);
 
 my @diff_arrays = qw/
@@ -32,6 +32,7 @@ my @diff_hash_keys = qw/
   fields_to_rename
   fields_to_drop
   table_options
+  table_renamed_from
 /;
 
 __PACKAGE__->mk_accessors(@diff_arrays, 'table_diff_hash');
@@ -44,15 +45,14 @@ sub schema_diff {
     ## _db is the name of the producer/db it came out of/into
     ## results are formatted to the source preferences
 
-    my ($source_schema, $source_db, $target_schema, $target_db, $options) = @_;
+    my ($source_schema, $source_db, $target_schema, $output_db, $options) = @_;
     $options ||= {};
 
     my $obj = SQL::Translator::Diff->new( {
       %$options,
       source_schema => $source_schema,
-      source_db     => $source_db,
       target_schema => $target_schema,
-      target_db     => $target_db
+      output_db     => $output_db
     } );
 
     $obj->compute_differences->produce_diff_sql;
@@ -63,6 +63,7 @@ sub new {
   $values->{$_} ||= [] foreach @diff_arrays;
   $values->{table_diff_hash} = {};
 
+  $values->{producer_options} ||= {};
   $values->{output_db} ||= $values->{source_db};
   return $class->SUPER::new($values);
 }
@@ -73,11 +74,38 @@ sub compute_differences {
     my $target_schema = $self->target_schema;
     my $source_schema = $self->source_schema;
 
+    my $producer_class = "SQL::Translator::Producer::@{[$self->output_db]}";
+    eval "require $producer_class";
+    die $@ if $@;
+
+    if (my $preprocess = $producer_class->can('preprocess_schema')) {
+      $preprocess->($source_schema);
+      $preprocess->($target_schema);
+    }
+
+    my %src_tables_checked = ();
     my @tar_tables = sort { $a->name cmp $b->name } $target_schema->get_tables;
     ## do original/source tables exist in target?
     for my $tar_table ( @tar_tables ) {
       my $tar_table_name = $tar_table->name;
-      my $src_table      = $source_schema->get_table( $tar_table_name, $self->case_insensitive );
+
+      my $src_table;
+
+      $self->table_diff_hash->{$tar_table_name} = {
+        map {$_ => [] } @diff_hash_keys
+      };
+
+      if (my $old_name = $tar_table->extra('renamed_from')) {
+        $src_table = $source_schema->get_table( $old_name, $self->case_insensitive );
+        if ($src_table) {
+          $self->table_diff_hash->{$tar_table_name}{table_renamed_from} = [ [$src_table, $tar_table] ];
+        } else {
+          delete $tar_table->extra->{renamed_from};
+          warn qq#Renamed table can't find old table "$old_name" for renamed table\n#;
+        }
+      } else {
+        $src_table = $source_schema->get_table( $tar_table_name, $self->case_insensitive );
+      }
 
       unless ( $src_table ) {
         ## table is new
@@ -86,9 +114,10 @@ sub compute_differences {
         next;
       }
 
-      $self->table_diff_hash->{$tar_table_name} = {
-        map {$_ => [] } @diff_hash_keys
-      };
+      my $src_table_name = $src_table->name;
+      $src_table_name = lc $src_table_name if $self->case_insensitive;
+      $src_tables_checked{$src_table_name} = 1;
+
 
       $self->diff_table_options($src_table, $tar_table);
 
@@ -102,16 +131,11 @@ sub compute_differences {
 
     for my $src_table ( $source_schema->get_tables ) {
       my $src_table_name = $src_table->name;
-      my $tar_table      = $target_schema->get_table( $src_table_name, $self->case_insensitive );
 
-      unless ( $tar_table ) {
-        $self->table_diff_hash->{$src_table_name} = {
-          map {$_ => [] } @diff_hash_keys
-        };
+      $src_table_name = lc $src_table_name if $self->case_insensitive;
 
-        push @{ $self->tables_to_drop}, $src_table;
-        next;
-      }
+      push @{ $self->tables_to_drop}, $src_table
+        unless $src_tables_checked{$src_table_name};
     }
 
     return $self;
@@ -139,7 +163,8 @@ sub produce_diff_sql {
       fields_to_alter       => 'alter_field',
       fields_to_rename      => 'rename_field',
       fields_to_drop        => 'drop_field',
-      table_options         => 'alter_table'
+      table_options         => 'alter_table',
+      table_renamed_from    => 'rename_table',
     );
     my @diffs;
   
@@ -155,11 +180,13 @@ sub produce_diff_sql {
           { map {
               $func_map{$_} => $self->table_diff_hash->{$table}{$_}
             } keys %func_map 
-          }
+          }, 
+          $self->producer_options
         );
       }
     } else {
 
+      # If we have any table renames we need to do those first;
       my %flattened_diffs;
       foreach my $table ( sort keys %{$self->table_diff_hash} ) {
         my $table_diff = $self->table_diff_hash->{$table};
@@ -169,16 +196,20 @@ sub produce_diff_sql {
       }
 
       push @diffs, map( {
-          if (@{$flattened_diffs{$_}}) {
+          if (@{ $flattened_diffs{$_} || [] }) {
             my $meth = $producer_class->can($_);
             
-            $meth ? map { my $sql = $meth->(ref $_ eq 'ARRAY' ? @$_ : $_); $sql ?  ("$sql;") : () } @{ $flattened_diffs{$_} }
+            $meth ? map { 
+                    my $sql = $meth->( (ref $_ eq 'ARRAY' ? @$_ : $_), $self->producer_options );
+                    $sql ?  ("$sql") : (); 
+                  } @{ $flattened_diffs{$_} }
                   : $self->ignore_missing_methods
                   ? "-- $producer_class cant $_"
                   : die "$producer_class cant $_";
           } else { () }
 
-        } qw/alter_drop_constraint
+        } qw/rename_table
+             alter_drop_constraint
              alter_drop_index
              drop_field
              add_field
@@ -195,39 +226,44 @@ sub produce_diff_sql {
         add_drop_table => 0,
         no_comments => 1,
         # TODO: sort out options
-        quote_table_names => 0,
-        quote_field_names => 0,
+        %{ $self->producer_options }
       );
+      $translator->producer_args->{no_transaction} = 1;
       my $schema = $translator->schema;
 
       $schema->add_table($_) for @tables;
 
       unshift @diffs, 
         # Remove begin/commit here, since we wrap everything in one.
-        grep { $_ !~ /^(?:COMMIT|BEGIN(?: TRANSACTION)?);/ } $producer_class->can('produce')->($translator);
+        grep { $_ !~ /^(?:COMMIT|START(?: TRANSACTION)?|BEGIN(?: TRANSACTION)?)/ } $producer_class->can('produce')->($translator);
     }
 
     if (my @tables_to_drop = @{ $self->{tables_to_drop} || []} ) {
       my $meth = $producer_class->can('drop_table');
       
-      push @diffs, $meth ? map( { $meth->($_) } @tables_to_drop )
+      push @diffs, $meth ? ( map { $meth->($_, $self->producer_options) } @tables_to_drop)
                          : $self->ignore_missing_methods
                          ? "-- $producer_class cant drop_table"
                          : die "$producer_class cant drop_table";
     }
 
     if (@diffs) {
-      unshift @diffs, "BEGIN TRANSACTION;\n";
-      push    @diffs, "\nCOMMIT;\n";
+      unshift @diffs, "BEGIN";
+      push    @diffs, "\nCOMMIT";
     } else {
-      @diffs = ("-- No differences found\n\n");
+      @diffs = ("-- No differences found");
     }
 
     if ( @diffs ) {
-      if ( $self->target_db !~ /^(?:MySQL|SQLite)$/ ) {
-        unshift(@diffs, "-- Target database @{[$self->target_db]} is untested/unsupported!!!");
+      if ( $self->output_db !~ /^(?:MySQL|SQLite|PostgreSQL)$/ ) {
+        unshift(@diffs, "-- Output database @{[$self->output_db]} is untested/unsupported!!!");
       }
-      return join( "\n", "-- Convert schema '$src_name' to '$tar_name':\n", @diffs);
+
+      my @return = 
+        map { $_ ? ( $_ =~ /;$/xms ? $_ : "$_;\n\n" ) : "\n" }
+        ("-- Convert schema '$src_name' to '$tar_name':", @diffs);
+
+      return wantarray ? @return : join('', @return);
     }
     return undef;
 
@@ -266,6 +302,10 @@ sub diff_table_constraints {
   CONSTRAINT_CREATE:
   for my $c_tar ( $tar_table->get_constraints ) {
     for my $c_src ( $src_table->get_constraints ) {
+
+      # This is a bit of a hack - needed for renaming tables to work
+      local $c_src->{table} = $tar_table;
+
       if ( $c_tar->equals($c_src, $self->case_insensitive, $self->ignore_constraint_names) ) {
         $checked_constraints{$c_src} = 1;
         next CONSTRAINT_CREATE;
@@ -277,6 +317,10 @@ sub diff_table_constraints {
 
   CONSTRAINT_DROP:
   for my $c_src ( $src_table->get_constraints ) {
+
+    # This is a bit of a hack - needed for renaming tables to work
+    local $c_src->{table} = $tar_table;
+
     next if !$self->ignore_constraint_names && $checked_constraints{$c_src};
     for my $c_tar ( $tar_table->get_constraints ) {
       next CONSTRAINT_DROP if $c_src->equals($c_tar, $self->case_insensitive, $self->ignore_constraint_names);
@@ -298,10 +342,14 @@ sub diff_table_fields {
 
     if (my $old_name = $tar_table_field->extra->{renamed_from}) {
       my $src_table_field = $src_table->get_field( $old_name, $self->case_insensitive );
-      die qq#Renamed cant find "@{[$src_table->name]}.$old_name" for renamed column\n# unless $src_table_field;
-      push @{$self->table_diff_hash->{$tar_table}{fields_to_rename} }, [ $src_table_field, $tar_table_field ];
-      $renamed_source_fields{$old_name} = 1;
-      next;
+      unless ($src_table_field) {
+        warn qq#Renamed column can't find old column "@{[$src_table->name]}.$old_name" for renamed column\n#;
+        delete $tar_table_field->extra->{renamed_from};
+      } else {
+        push @{$self->table_diff_hash->{$tar_table}{fields_to_rename} }, [ $src_table_field, $tar_table_field ];
+        $renamed_source_fields{$old_name} = 1;
+        next;
+      }
     }
 
     my $src_table_field = $src_table->get_field( $f_tar_name, $self->case_insensitive );
@@ -343,46 +391,19 @@ sub diff_table_fields {
 sub diff_table_options {
   my ($self, $src_table, $tar_table) = @_;
 
+  my $cmp = sub {
+    my ($a_name, undef, $b_name, undef) = ( %$a, %$b );
+    $a_name cmp $b_name;
+  };
+  # Need to sort the options so we dont get supruious diffs.
+  my (@src_opts, @tar_opts);
+  @src_opts = sort $cmp $src_table->options;
+  @tar_opts = sort $cmp $tar_table->options;
 
-  # Go through our options
-  my $options_different = 0;
-  my %checkedOptions;
-
-  OPTION:
-  for my $tar_table_option_ref ( $tar_table->options ) {
-    my($key_tar, $value_tar) = %{$tar_table_option_ref};
-    for my $src_table_option_ref ( $src_table->options ) {
-      my($key_src, $value_src) = %{$src_table_option_ref};
-      if ( $key_tar eq $key_src ) {
-        if ( defined $value_tar != defined $value_src ) {
-          $options_different = 1;
-          last OPTION;
-        }
-        if ( defined $value_tar && $value_tar ne $value_src ) {
-          $options_different = 1;
-          last OPTION;
-        }
-        $checkedOptions{$key_tar} = 1;
-        next OPTION;
-      }
-    }
-    $options_different = 1;
-    last OPTION;
-  }
-
-  # Go through the other table's options
-  unless ( $options_different ) {
-    for my $src_table_option_ref ( $src_table->options ) {
-      my($key, $value) = %{$src_table_option_ref};
-      next if $checkedOptions{$key};
-      $options_different = 1;
-      last;
-    }
-  }
 
   # If there's a difference, just re-set all the options
   push @{ $self->table_diff_hash->{$tar_table}{table_options} }, $tar_table
-    if ( $options_different );
+    unless $src_table->_compare_objects( \@src_opts, \@tar_opts );
 }
 
 1;
@@ -475,9 +496,9 @@ thrown.
 
 =item * C<drop_table($table)>
 
-=item * C<batch_alter_table($table, $hash)> (optional)
+=item * C<rename_table($old_table, $new_table)> (optional)
 
-=back
+=item * C<batch_alter_table($table, $hash)> (optional)
 
 If the producer supports C<batch_alter_table>, it will be called with the 
 table to alter and a hash, the keys of which will be the method names listed
@@ -492,11 +513,31 @@ I.e. the hash might look something like the following:
    alter_field => [ [$old_field, $new_field] ]
  }
 
+
+=item * C<preprocess_schema($class, $schema)> (optional)
+
+C<preprocess_schema> is called by the Diff code to allow the producer to
+normalize any data it needs to first. For example, the MySQL producer uses
+this method to ensure that FK contraint names are unique.
+
+Basicaly any changes that need to be made to produce the SQL file for the
+schema should be done here, so that a diff between a parsed SQL file and (say)
+a parsed DBIx::Class::Schema object will be sane.
+
+(As an aside, DBIx::Class, for instance, uses the presence of a 
+C<preprocess_schema> function on the producer to know that it can diff between
+the previous SQL file and its own internal representation. Without this method
+on th producer it will diff the two SQL files which is slower, but known to 
+work better on old-style producers.)
+
+=back
+
+
 =head1 AUTHOR
 
 Original Author(s) unknown.
 
-Refactor and more comprehensive tests by Ash Berlin C<< ash@cpan.org >>.
+Refactor/re-write and more comprehensive tests by Ash Berlin C<< ash@cpan.org >>.
 
 Redevelopment sponsored by Takkle Inc.