From: Ash Berlin <ash_github@firemirror.com>
Date: Thu, 20 Dec 2007 16:54:56 +0000 (+0000)
Subject: Fix some more normalization problems
X-Git-Tag: v0.11008~348^2~11
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=9ab59f87bc19583a9e07ba4747a9a7c5dbb58878;p=dbsrgits%2FSQL-Translator.git

Fix some more normalization problems
---

diff --git a/lib/SQL/Translator/Diff.pm b/lib/SQL/Translator/Diff.pm
index 2638e83..ac49b57 100644
--- a/lib/SQL/Translator/Diff.pm
+++ b/lib/SQL/Translator/Diff.pm
@@ -73,10 +73,21 @@ 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')) {
+      $producer_class->$preprocess($source_schema);
+      $DB::single = 1;
+      $producer_class->$preprocess($target_schema);
+    }
+
     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;
+      $DB::single = 1 if $tar_table_name eq 'admin_contest';
       my $src_table      = $source_schema->get_table( $tar_table_name, $self->case_insensitive );
 
       unless ( $src_table ) {
@@ -296,6 +307,7 @@ sub diff_table_fields {
   for my $tar_table_field ( $tar_table->get_fields ) {
     my $f_tar_name      = $tar_table_field->name;
 
+    $DB::single = 1 if $f_tar_name eq 'invite_type';
     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;
@@ -344,45 +356,9 @@ sub diff_table_options {
   my ($self, $src_table, $tar_table) = @_;
 
 
-  # 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( scalar $src_table->options, scalar $tar_table->options );
 }
 
 1;
diff --git a/lib/SQL/Translator/Parser/MySQL.pm b/lib/SQL/Translator/Parser/MySQL.pm
index 410fe23..fda11d7 100644
--- a/lib/SQL/Translator/Parser/MySQL.pm
+++ b/lib/SQL/Translator/Parser/MySQL.pm
@@ -145,49 +145,6 @@ use DBI qw(:sql_types);
 use base qw(Exporter);
 
 our %type_mapping = (
-  int => SQL_INTEGER,
-  integer => SQL_INTEGER,
-
-  tinyint => SQL_TINYINT,
-  smallint => SQL_SMALLINT,
-  mediumint,
-  bigint,
-
-  float => SQL_FLOAT, # Precision 0..23
-  double => SQL_DOUBLE, # Precision 24..53
-  "double precision" => SQL_DOUBLE,
-  real => SQL_DOUBLE,
-
-  # all these are the same.
-  decimal => SQL_DECIMAL,
-  numeric => SQL_NUMERIC,
-  dec => SQL_DECIMAL,
-  # fixed: does this exist
-
-  bit => SQL_BIT
-
-  date => SQL_DATE,
-  datetime => SQL_DATETIME,
-  timestamp => SQL_TIMESTAMP,
-  time => SQL_TIME,
-  year
-
-
-  char => SQL_CHAR,
-  varchar => SQL_VARCHAR,
-  binary => SQL_BINARY,
-  varbinary => SQL_VARBINARY,
-  tinyblob => SQL_BLOB,
-  tinytext => 
-  blob => SQL_BLOB,
-  text => SQL_LONGVARCHAR
-  mediumblob => SQL_BLOB,
-  mediumtext => SQL_LONGVARCHAR
-  longblob => SQL_BLOB
-  longtext => SQL_LONGVARCHAR
-
-  enum
-  set
 );
 
 @EXPORT_OK = qw(parse);
@@ -852,15 +809,6 @@ sub parse {
                 ) or die $table->error;
             }
 
-            if ( $field->data_type =~ /(set|enum)/i && !$field->size ) {
-                my %extra = $field->extra;
-                my $longest = 0;
-                for my $len ( map { length } @{ $extra{'list'} || [] } ) {
-                    $longest = $len if $len > $longest;
-                }
-                $field->size( $longest ) if $longest;
-            }
-
             for my $cdata ( @{ $fdata->{'constraints'} } ) {
                 next unless $cdata->{'type'} eq 'foreign_key';
                 $cdata->{'fields'} ||= [ $field->name ];
@@ -976,7 +924,17 @@ sub normalize_field {
         $changed = $size != 4_294_967_295;
         $size = 4_294_967_295;
     }
-    $DB::single = 1 if $field->name eq 'employee_id';
+    if ( $field->data_type =~ /(set|enum)/i && !$field->size ) {
+        my %extra = $field->extra;
+        my $longest = 0;
+        for my $len ( map { length } @{ $extra{'list'} || [] } ) {
+            $longest = $len if $len > $longest;
+        }
+        $changed = 1;
+        $size = $longest if $longest;
+    }
+
+
     if ($changed) {
       # We only want to clone the field, not *everything*
       { local $field->{table} = undef;
@@ -985,7 +943,7 @@ sub normalize_field {
       }
       $field->size($size);
       $field->data_type($type);
-      $field->sql_data_type( $type_mapping{lc $type} || SQL_UNKNOWN_TYPE );
+      $field->sql_data_type( $type_mapping{lc $type} ) if exists $type_mapping{lc $type};
       $field->extra->{list} = $list if @$list;
     }
 }
diff --git a/lib/SQL/Translator/Producer/MySQL.pm b/lib/SQL/Translator/Producer/MySQL.pm
index 945829f..f3f53a4 100644
--- a/lib/SQL/Translator/Producer/MySQL.pm
+++ b/lib/SQL/Translator/Producer/MySQL.pm
@@ -124,6 +124,68 @@ my %translate  = (
     'datetime'     => 'datetime',
 );
 
+
+sub preprocess_schema {
+    my ($class, $schema) = @_;
+
+    # extra->{mysql_table_type} used to be the type. It belongs in options, so
+    # move it if we find it. Return Engine type if found in extra or options
+    my $mysql_table_type_to_options = sub {
+      my ($table) = @_;
+
+      my $extra = $table->extra;
+
+      my $extra_type = delete $extra->{mysql_table_type};
+
+      # Now just to find if there is already an Engine or Type option...
+      # and lets normalize it to ENGINE since:
+      #
+      # The ENGINE table option specifies the storage engine for the table. 
+      # TYPE is a synonym, but ENGINE is the preferred option name.
+      #
+
+      # We have to use the hash directly here since otherwise there is no way 
+      # to remove options.
+      my $options = ( $table->{options} ||= []);
+
+      # This assumes that there isn't both a Type and an Engine option.
+      for my $idx ( 0..$#{$options} ) {
+        my ($key, $value) = %{ $options->[$idx] };
+
+        next unless uc $key eq 'ENGINE' || uc $key eq 'TYPE';
+
+        # if the extra.mysql_table_type is given, use that
+        delete $options->[$idx]{$key};
+        return $options->[$idx]{ENGINE} = $value || $extra_type;
+
+      }
+  
+      if ($extra_type) {
+        push @$options, { ENGINE => $extra_type };
+        return $extra_type;
+      }
+
+    };
+
+    #
+    # Work out which tables need to be InnoDB to support foreign key
+    # constraints. We do this first as we need InnoDB at both ends.
+    #
+    foreach my $table ( $schema->get_tables ) {
+       
+        $mysql_table_type_to_options->($table);
+
+        foreach ( $table->get_constraints ) {
+            next unless $_->type eq FOREIGN_KEY;
+            for my $meth (qw/table reference_table/) {
+                my $table = $schema->get_table($_->$meth) || next;
+                next if $mysql_table_type_to_options->($table);
+                $table->options( { 'ENGINE' => 'InnoDB' } );
+            }
+        }
+    }
+}
+
 sub produce {
     my $translator     = shift;
     local $DEBUG       = $translator->debug;
@@ -144,18 +206,7 @@ sub produce {
     # \todo Don't set if MySQL 3.x is set on command line
     $create .= "SET foreign_key_checks=0;\n\n";
 
-    #
-    # Work out which tables need to be InnoDB to support foreign key
-    # constraints. We do this first as we need InnoDB at both ends.
-    #
-    foreach ( map { $_->get_constraints } $schema->get_tables ) {
-        next unless $_->type eq FOREIGN_KEY;
-        foreach my $meth (qw/table reference_table/) {
-            my $table = $schema->get_table($_->$meth) || next;
-            next if $table->extra('mysql_table_type');
-            $table->extra( 'mysql_table_type' => 'InnoDB');
-        }
-    }
+    __PACKAGE__->preprocess_schema($schema);
 
     #
     # Generate sql
@@ -257,8 +308,9 @@ sub generate_table_options
       if uc $key eq 'ENGINE' or uc $key eq 'TYPE';
     $create .= " $key=$value";
   }
+
   my $mysql_table_type = $table->extra('mysql_table_type');
-  $create .= " Type=$mysql_table_type"
+  $create .= " ENGINE=$mysql_table_type"
     if $mysql_table_type && !$table_type_defined;
   my $charset          = $table->extra('mysql_charset');
   my $collate          = $table->extra('mysql_collate');
diff --git a/lib/SQL/Translator/Schema/Field.pm b/lib/SQL/Translator/Schema/Field.pm
index 2f20ee5..93d25e2 100644
--- a/lib/SQL/Translator/Schema/Field.pm
+++ b/lib/SQL/Translator/Schema/Field.pm
@@ -61,6 +61,38 @@ use overload
     fallback => 1,
 ;
 
+use DBI qw(:sql_types);
+
+# Mapping from string to sql contstant
+our %type_mapping = (
+  integer => SQL_INTEGER,
+  int     => SQL_INTEGER,
+
+  smallint => SQL_SMALLINT,
+  bigint => 9999, # DBI doesn't export a constatn for this. Le suck
+
+  double => SQL_DOUBLE,
+
+  decimal => SQL_DECIMAL,
+  numeric => SQL_NUMERIC,
+  dec => SQL_DECIMAL,
+
+  bit => SQL_BIT,
+
+  date => SQL_DATE,
+  datetime => SQL_DATETIME,
+  timestamp => SQL_TIMESTAMP,
+  time => SQL_TIME,
+
+  char => SQL_CHAR,
+  varchar => SQL_VARCHAR,
+  binary => SQL_BINARY,
+  varbinary => SQL_VARBINARY,
+  tinyblob => SQL_BLOB,
+  blob => SQL_BLOB,
+  text => SQL_LONGVARCHAR
+
+);
 # ----------------------------------------------------------------------
 
 __PACKAGE__->_attributes( qw/
@@ -132,7 +164,10 @@ Get or set the field's data type.
 =cut
 
     my $self = shift;
-    $self->{'data_type'} = shift if @_;
+    if (@_) {
+      $self->{'data_type'} = $_[0];
+      $self->{'sql_data_type'} = $type_mapping{lc $_[0]} || SQL_UNKNOWN_TYPE unless exists $self->{sql_data_type};
+    }
     return $self->{'data_type'} || '';
 }
 
diff --git a/t/38-mysql-producer.t b/t/38-mysql-producer.t
index 25937b8..3f0a507 100644
--- a/t/38-mysql-producer.t
+++ b/t/38-mysql-producer.t
@@ -130,7 +130,7 @@ my @stmts = (
   `description` text CHARACTER SET utf8 COLLATE utf8_general_ci,
   PRIMARY KEY (`id`),
   UNIQUE `idx_unique_name` (`name`)
-) Type=InnoDB DEFAULT CHARACTER SET latin1 COLLATE latin1_danish_ci;\n\n",
+) ENGINE=InnoDB DEFAULT CHARACTER SET latin1 COLLATE latin1_danish_ci;\n\n",
 
 "DROP TABLE IF EXISTS `thing2`;\n",
 "CREATE TABLE `thing2` (
@@ -144,7 +144,7 @@ my @stmts = (
   PRIMARY KEY (`id`, `foo`),
   CONSTRAINT `fk_thing` FOREIGN KEY (`foo`) REFERENCES `thing` (`id`),
   CONSTRAINT `fk_thing_1` FOREIGN KEY (`foo2`) REFERENCES `thing` (`id`)
-) Type=InnoDB;\n\n",
+) ENGINE=InnoDB;\n\n",
 
 "SET foreign_key_checks=1;\n\n"