From: Ash Berlin Date: Thu, 13 Dec 2007 17:53:49 +0000 (+0000) Subject: Some work on sanely normalizing fields X-Git-Tag: v0.11008~348^2~13 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=07d6e5f7a00c14f44c4f2cab58bbb0a4dbe65567;p=dbsrgits%2FSQL-Translator.git Some work on sanely normalizing fields --- diff --git a/lib/SQL/Translator/Diff.pm b/lib/SQL/Translator/Diff.pm index 33085a4..2638e83 100644 --- a/lib/SQL/Translator/Diff.pm +++ b/lib/SQL/Translator/Diff.pm @@ -151,7 +151,6 @@ sub produce_diff_sql { my $tar_table = $target_schema->get_table($table) || $source_schema->get_table($table); - $DB::single = 1 if $table eq 'deleted'; push @diffs, $batch_alter->($tar_table, { map { $func_map{$_} => $self->table_diff_hash->{$table}{$_} @@ -312,8 +311,13 @@ sub diff_table_fields { next; } - ## field exists, something changed. - if ( !$tar_table_field->equals($src_table_field, $self->case_insensitive) ) { + # field exists, something changed. This is a bit complex. Parsers can + # normalize types, but only some of them do, so compare the normalized and + # parsed types for each field to each other + if ( !$tar_table_field->equals($src_table_field, $self->case_insensitive) && + !$tar_table_field->equals($src_table_field->parsed_field, $self->case_insensitive) && + !$tar_table_field->parsed_field->equals($src_table_field, $self->case_insensitive) && + !$tar_table_field->parsed_field->equals($src_table_field->parsed_field, $self->case_insensitive) ) { # Some producers might need src field to diff against push @{$self->table_diff_hash->{$tar_table}{fields_to_alter}}, [ $src_table_field, $tar_table_field ]; diff --git a/lib/SQL/Translator/Parser/MySQL.pm b/lib/SQL/Translator/Parser/MySQL.pm index 89a2194..86027e1 100644 --- a/lib/SQL/Translator/Parser/MySQL.pm +++ b/lib/SQL/Translator/Parser/MySQL.pm @@ -140,6 +140,7 @@ $DEBUG = 0 unless defined $DEBUG; use Data::Dumper; use Parse::RecDescent; use Exporter; +use Storable qw(dclone); use base qw(Exporter); @EXPORT_OK = qw(parse); @@ -211,7 +212,7 @@ statement_body : (string | nonstring)(s?) insert : /insert/i statement_body "$delimiter" delimiter : /delimiter/i /[\S]+/ - { $delimiter = $item[2] } + { $delimiter = $item[2] } empty_statement : "$delimiter" @@ -298,13 +299,13 @@ create : CREATE UNIQUE(?) /(index|key)/i index_name /on/i table_name '(' field_n } create : CREATE /trigger/i NAME not_delimiter "$delimiter" - { - @table_comments = (); - } + { + @table_comments = (); + } create : CREATE PROCEDURE NAME not_delimiter "$delimiter" - { - @table_comments = (); + { + @table_comments = (); my $func_name = $item[3]; my $owner = ''; my $sql = "$item[1] $item[2] $item[3] $item[4]"; @@ -313,14 +314,14 @@ create : CREATE PROCEDURE NAME not_delimiter "$delimiter" $procedures{ $func_name }{'name'} = $func_name; $procedures{ $func_name }{'owner'} = $owner; $procedures{ $func_name }{'sql'} = $sql; - } + } PROCEDURE : /procedure/i - | /function/i + | /function/i create : CREATE algorithm /view/i NAME not_delimiter "$delimiter" - { - @table_comments = (); + { + @table_comments = (); my $view_name = $item[4]; my $sql = "$item[1] $item[2] $item[3] $item[4] $item[5]"; @@ -330,12 +331,12 @@ create : CREATE algorithm /view/i NAME not_delimiter "$delimiter" $views{ $view_name }{'order'} = ++$view_order; $views{ $view_name }{'name'} = $view_name; $views{ $view_name }{'sql'} = $sql; - } + } algorithm : /algorithm/i /=/ WORD - { - $return = "$item[1]=$item[3]"; - } + { + $return = "$item[1]=$item[3]"; + } not_delimiter : /.*?(?=$delimiter)/is @@ -535,42 +536,6 @@ data_type : WORD parens_value_list(s?) type_qualifier(s?) $list = []; } - if ( @{ $size || [] } == 0 && !$thisparser->{local}{sqlt_parser_args}{no_default_sizes} ) { - if ( lc $type eq 'tinyint' ) { - $size = 4; - } - elsif ( lc $type eq 'smallint' ) { - $size = 6; - } - elsif ( lc $type eq 'mediumint' ) { - $size = 9; - } - elsif ( $type =~ /^int(eger)?$/i ) { - $type = 'int'; - $size = 11; - } - elsif ( lc $type eq 'bigint' ) { - $size = 20; - } - elsif ( - lc $type =~ /(float|double|decimal|numeric|real|fixed|dec)/ - ) { - $size = [8,2]; - } - } - - if ( $type =~ /^tiny(text|blob)$/i ) { - $size = 255; - } - elsif ( $type =~ /^(blob|text)$/i ) { - $size = 65_535; - } - elsif ( $type =~ /^medium(blob|text)$/i ) { - $size = 16_777_215; - } - elsif ( $type =~ /^long(blob|text)$/i ) { - $size = 4_294_967_295; - } $return = { type => $type, @@ -753,9 +718,9 @@ VALUE : /[-+]?\.?\d+(?:[eE]\d+)?/ { 'NULL' } CURRENT_TIMESTAMP : /current_timestamp(\(\))?/i - | /now\(\)/i - { 'CURRENT_TIMESTAMP' } - + | /now\(\)/i + { 'CURRENT_TIMESTAMP' } + END_OF_GRAMMAR # ------------------------------------------------------------------- @@ -770,9 +735,6 @@ sub parse { return $translator->error("Error instantiating Parse::RecDescent ". "instance: Bad grammer"); } - - # This is the only way to get args into the productions/actions - $parser->{local}{sqlt_parser_args} = $translator->parser_args; # Preprocess for MySQL-specific and not-before-version comments from mysqldump my $parser_version = $translator->parser_args->{mysql_parser_version} || DEFAULT_PARSER_VERSION; @@ -820,7 +782,7 @@ sub parse { $table->primary_key( $field->name ) if $fdata->{'is_primary_key'}; for my $qual ( qw[ binary unsigned zerofill list collate ], - 'character set', 'on update' ) { + 'character set', 'on update' ) { if ( my $val = $fdata->{ $qual } || $fdata->{ uc $qual } ) { next if ref $val eq 'ARRAY' && !@$val; $field->extra( $qual, $val ); @@ -857,6 +819,7 @@ sub parse { $cdata->{'fields'} ||= [ $field->name ]; push @{ $tdata->{'constraints'} }, $cdata; } + } for my $idata ( @{ $tdata->{'indices'} || [] } ) { @@ -883,32 +846,102 @@ sub parse { on_update => $cdata->{'on_update'} || $cdata->{'on_update_do'}, ) or die $table->error; } + + # After the constrains and PK/idxs have been created, we normalize fields + normalize_field($_) for $table->get_fields; } my @procedures = sort { $result->{procedures}->{ $a }->{'order'} <=> $result->{procedures}->{ $b }->{'order'} } keys %{ $result->{procedures} }; foreach my $proc_name (@procedures) { - $schema->add_procedure( - name => $proc_name, - owner => $result->{procedures}->{$proc_name}->{owner}, - sql => $result->{procedures}->{$proc_name}->{sql}, - ); + $schema->add_procedure( + name => $proc_name, + owner => $result->{procedures}->{$proc_name}->{owner}, + sql => $result->{procedures}->{$proc_name}->{sql}, + ); } my @views = sort { $result->{views}->{ $a }->{'order'} <=> $result->{views}->{ $b }->{'order'} } keys %{ $result->{views} }; foreach my $view_name (keys %{ $result->{views} }) { - $schema->add_view( - name => $view_name, - sql => $result->{views}->{$view_name}->{sql}, - ); + $schema->add_view( + name => $view_name, + sql => $result->{views}->{$view_name}->{sql}, + ); } return 1; } +# Takes a field, and returns +sub normalize_field { + my ($field) = @_; + my ($size, $type, $list, $changed) = @_; + + $size = $field->size; + $type = $field->data_type; + $list = $field->extra->{list} || []; + + if ( !ref $size && $size eq 0 ) { + if ( lc $type eq 'tinyint' ) { + $changed = $size != 4; + $size = 4; + } + elsif ( lc $type eq 'smallint' ) { + $changed = $size != 6; + $size = 6; + } + elsif ( lc $type eq 'mediumint' ) { + $changed = $size != 9; + $size = 9; + } + elsif ( $type =~ /^int(eger)?$/i ) { + $changed = $size != 11 || $type ne 'int'; + $type = 'int'; + $size = 11; + } + elsif ( lc $type eq 'bigint' ) { + $changed = $size != 20; + $size = 20; + } + elsif ( lc $type =~ /(float|double|decimal|numeric|real|fixed|dec)/ ) { + my $old_size = (ref $size || '') eq 'ARRAY' ? $size : []; + $changed = @$old_size != 2 && $old_size->[0] != 8 && $old_size->[1] != 2; + $size = [8,2]; + } + } + + if ( $type =~ /^tiny(text|blob)$/i ) { + $changed = $size != 255; + $size = 255; + } + elsif ( $type =~ /^(blob|text)$/i ) { + $changed = $size != 65_535; + $size = 65_535; + } + elsif ( $type =~ /^medium(blob|text)$/i ) { + $changed = $size != 16_777_215; + $size = 16_777_215; + } + elsif ( $type =~ /^long(blob|text)$/i ) { + $changed = $size != 4_294_967_295; + $size = 4_294_967_295; + } + $DB::single = 1 if $field->name eq 'employee_id'; + if ($changed) { + # We only want to clone the field, not *everything* + { local $field->{table} = undef; + $field->parsed_field(dclone($field)); + $field->parsed_field->{table} = $field->table; + } + $field->size($size); + $field->data_type($type); + $field->extra->{list} = $list if @$list; + } +} + 1; # ------------------------------------------------------------------- diff --git a/lib/SQL/Translator/Producer/MySQL.pm b/lib/SQL/Translator/Producer/MySQL.pm index 4fd5cee..945829f 100644 --- a/lib/SQL/Translator/Producer/MySQL.pm +++ b/lib/SQL/Translator/Producer/MySQL.pm @@ -636,7 +636,6 @@ sub drop_table { my ($table) = @_; # Drop (foreign key) constraints so table drops cleanly - $DB::single = 1; my @sql = batch_alter_table($table, { alter_drop_constraint => [ grep { $_->type eq 'FOREIGN KEY' } $table->get_constraints ] }); return join("\n", @sql, "DROP TABLE $table;"); diff --git a/lib/SQL/Translator/Schema/Field.pm b/lib/SQL/Translator/Schema/Field.pm index 5b3a189..37730e1 100644 --- a/lib/SQL/Translator/Schema/Field.pm +++ b/lib/SQL/Translator/Schema/Field.pm @@ -548,6 +548,24 @@ also be used to get the table name. return $self->{'table'}; } +sub parsed_field { + +=head2 + +Returns the field exactly as the parser found it + +=cut + + my $self = shift; + + if (@_) { + my $value = shift; + $self->{parsed_field} = $value; + return $value || $self; + } + return $self->{parsed_field} || $self; +} + # ---------------------------------------------------------------------- sub equals { diff --git a/t/02mysql-parser.t b/t/02mysql-parser.t index a756b9f..553f02d 100644 --- a/t/02mysql-parser.t +++ b/t/02mysql-parser.t @@ -10,7 +10,7 @@ use SQL::Translator::Schema::Constants; use Test::SQL::Translator qw(maybe_plan); BEGIN { - maybe_plan(235, "SQL::Translator::Parser::MySQL"); + maybe_plan(228, "SQL::Translator::Parser::MySQL"); SQL::Translator::Parser::MySQL->import('parse'); } @@ -601,26 +601,4 @@ BEGIN { like($proc2->sql, qr/CREATE PROCEDURE sp_update_security_acl/, "Detected procedure sp_update_security_acl"); } -{ - my $tr = SQL::Translator->new({parser_args => { no_default_sizes => 1 } }); - my $data = q|create table sessions ( - age int - );|; - - my $val = parse($tr, $data); - my $schema = $tr->schema; - is( $schema->is_valid, 1, 'Schema is valid' ); - my @tables = $schema->get_tables; - is( scalar @tables, 1, 'Right number of tables (1)' ); - my $table = shift @tables; - is( $table->name, 'sessions', 'Found "sessions" table' ); - - my @fields = $table->get_fields; - is( scalar @fields, 1, 'Right number of fields (1)' ); - my $f1 = shift @fields; - is( $f1->name, 'age', 'First field name is "id"' ); - is( $f1->data_type, 'int', 'Type is "int"' ); - is( $f1->size, 0, 'No default size' ); - -} diff --git a/t/30sqlt-new-diff-mysql.t b/t/30sqlt-new-diff-mysql.t index 6876d5c..eca0a2d 100644 --- a/t/30sqlt-new-diff-mysql.t +++ b/t/30sqlt-new-diff-mysql.t @@ -11,13 +11,13 @@ use Test::More; use Test::Differences; use Test::SQL::Translator qw(maybe_plan); -plan tests => 4; +plan tests => 5; use_ok('SQL::Translator::Diff') or die "Cannot continue\n"; my $tr = SQL::Translator->new; -my ( $source_schema, $target_schema ) = map { +my ( $source_schema, $target_schema, $parsed_sql_schema ) = map { my $t = SQL::Translator->new; $t->parser( 'YAML' ) or die $tr->error; @@ -29,7 +29,7 @@ my ( $source_schema, $target_schema ) = map { $schema->name( $_ ); } ($schema); -} (qw/create1.yml create2.yml/); +} (qw( create1.yml create2.yml )); # Test for differences my $out = SQL::Translator::Diff::schema_diff( $source_schema, 'MySQL', $target_schema, 'MySQL', { no_batch_alters => 1} ); @@ -70,8 +70,6 @@ DROP TABLE deleted; COMMIT; ## END OF DIFF -#die $out; - $out = SQL::Translator::Diff::schema_diff($source_schema, 'MySQL', $target_schema, 'MySQL', { ignore_index_names => 1, ignore_constraint_names => 1 @@ -121,4 +119,57 @@ eq_or_diff($out, <<'## END OF DIFF', "No differences found"); ## END OF DIFF -=cut +{ + my $t = SQL::Translator->new; + $t->parser( 'MySQL' ) + or die $tr->error; + my $out = $t->translate( catfile($Bin, qw/data mysql create.sql/ ) ) + or die $tr->error; + + my $schema = $t->schema; + unless ( $schema->name ) { + $schema->name( 'create.sql' ); + } + + # Now lets change the type of one of the 'integer' columns so that it + # matches what the mysql parser sees for ' interger'. + my $field = $target_schema->get_table('employee')->get_field('employee_id'); + $field->data_type('integer'); + $field->size(0); + $out = SQL::Translator::Diff::schema_diff($schema, 'MySQL', $target_schema, 'MySQL' ); + eq_or_diff($out, <<'## END OF DIFF', "No differences found"); +-- Convert schema 'create.sql' to 'create2.yml': + +BEGIN TRANSACTION; + +SET foreign_key_checks=0; + + +CREATE TABLE added ( + id integer(11) +); + + +SET foreign_key_checks=1; + + +ALTER TABLE employee DROP FOREIGN KEY FK5302D47D93FE702E, + DROP COLUMN job_title, + ADD CONSTRAINT FK5302D47D93FE702E_diff_1 FOREIGN KEY (employee_id) REFERENCES person (person_id); +ALTER TABLE person DROP UNIQUE UC_age_name, + DROP INDEX u_name, + ADD COLUMN is_rock_star tinyint(4) DEFAULT '1', + CHANGE COLUMN person_id person_id integer(11) NOT NULL auto_increment, + CHANGE COLUMN name name varchar(20) NOT NULL, + CHANGE COLUMN age age integer(11) DEFAULT '18', + CHANGE COLUMN iq iq integer(11) DEFAULT '0', + CHANGE COLUMN description physical_description text, + ADD UNIQUE INDEX unique_name (name), + ADD UNIQUE UC_person_id (person_id), + ADD UNIQUE UC_age_name (age, name), + ENGINE=InnoDB; +DROP TABLE deleted; + +COMMIT; +## END OF DIFF +} diff --git a/t/data/diff/create2.yml b/t/data/diff/create2.yml index ccdb6e6..444a447 100644 --- a/t/data/diff/create2.yml +++ b/t/data/diff/create2.yml @@ -53,7 +53,7 @@ schema: data_type: int default_value: ~ extra: {} - is_nullable: 0 + is_nullable: 1 is_primary_key: 1 is_unique: 0 name: employee_id