Bug #39713 for SQL-Translator: [PATCH] default_value is insufficient

It isn't possible to pass arbitrary unquoted text to default values. As discussed on IRC, this patch makes that possible in several producers and documents the method. This also addresses the other open "MAGIC_VALUE gets quoted" bugs. Patch touches docs, several producers, and provides more tests for Postgres. -- rjbs

diff --git a/lib/SQL/Translator.pm b/lib/SQL/Translator.pm index 85021b4..a36ab63 100644 --- a/lib/SQL/Translator.pm +++ b/lib/SQL/Translator.pm @@ -39,6 +39,7 @@ use File::Find; use File::Spec::Functions qw(catfile); use File::Basename qw(dirname); use IO::Dir; +use SQL::Translator::Producer; use SQL::Translator::Schema; # ---------------------------------------------------------------------- diff --git a/lib/SQL/Translator/Producer.pm b/lib/SQL/Translator/Producer.pm index 1b2baf2..c8e9f03 100644 --- a/lib/SQL/Translator/Producer.pm +++ b/lib/SQL/Translator/Producer.pm @@ -26,6 +26,27 @@ $VERSION = sprintf "%d.%02d", q$Revision: 1.8 $ =~ /(\d+)\.(\d+)/; sub produce { "" } +# Do not rely on this if you are not bundled with SQL::Translator. +# -- rjbs, 2008-09-30 +sub _apply_default_value { + my (undef, $field_ref, $default, $special) = @_; + + if ($special and ! ref $default) { + for (my $i = 0; $i < @$special; $i += 2) { + my ($pat, $val) = @$special[ $i, $i + 1 ]; + next if ref $pat and $default !~ $pat; + next if ! ref $pat and $default ne $pat; + + $default = $val; + } + } + + $$field_ref .= ' DEFAULT ' + . (! ref $default ? "'$default'" + : defined $$default ? $$default + : 'NULL'); +} + 1; # ------------------------------------------------------------------- diff --git a/lib/SQL/Translator/Producer/MySQL.pm b/lib/SQL/Translator/Producer/MySQL.pm index 0e2917d..6ebc0ea 100644 --- a/lib/SQL/Translator/Producer/MySQL.pm +++ b/lib/SQL/Translator/Producer/MySQL.pm @@ -542,11 +542,13 @@ sub create_field # Default? XXX Need better quoting! my $default = $field->default_value; if ( defined $default ) { - if ( uc $default eq 'NULL') { - $field_def .= ' DEFAULT NULL'; - } else { - $field_def .= " DEFAULT '$default'"; - } + SQL::Translator::Producer->_apply_default_value( + \$field_def, + $default, + [ + qr/\Anull\z/i => \undef, + ], + ); } if ( my $comments = $field->comments ) { diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index e603124..b43ef99 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -548,7 +548,11 @@ sub create_field { # then sub "1/0," otherwise just test the truthity of the # argument and use that (naive?). # - if ( + if (ref $default and defined $$default) { + $default = $$default; + } elsif (ref $default) { + $default = 'NULL'; + } elsif ( $data_type =~ /^number$/i && $default !~ /^-?\d+$/ && $default !~ m/null/i diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm index a7abf10..14a2b35 100644 --- a/lib/SQL/Translator/Producer/PostgreSQL.pm +++ b/lib/SQL/Translator/Producer/PostgreSQL.pm @@ -440,7 +440,6 @@ sub create_table my $list = $extra{'list'} || []; # todo deal with embedded quotes my $commalist = join( ', ', map { qq['$_'] } @$list ); - my $seq_name; if ($postgres_version >= 8.3 && $field->data_type eq 'enum') { my $type_name = $field->table->name . '_' . $field->name . '_type'; @@ -458,14 +457,15 @@ sub create_table # ? undef : $field->default_value; my $default = $field->default_value; if ( defined $default ) { - my $qd = "'"; - $qd = '' if ($default eq 'now()' || - $default eq 'CURRENT_TIMESTAMP'); - $field_def .= sprintf( ' DEFAULT %s', - ( $field->is_auto_increment && $seq_name ) - ? qq[nextval('"$seq_name"'::text)] : - ( $default =~ m/null/i ) ? 'NULL' : "$qd$default$qd" - ); + SQL::Translator::Producer->_apply_default_value( + \$field_def, + $default, + [ + qr/\Anull\z/i => \undef, + 'now()' => 'now()', + 'CURRENT_TIMESTAMP' => 'CURRENT_TIMESTAMP', + ], + ); } # diff --git a/lib/SQL/Translator/Producer/SQLServer.pm b/lib/SQL/Translator/Producer/SQLServer.pm index 1a0c294..c195faf 100644 --- a/lib/SQL/Translator/Producer/SQLServer.pm +++ b/lib/SQL/Translator/Producer/SQLServer.pm @@ -189,7 +189,6 @@ sub produce { my $list = $extra{'list'} || []; # \todo deal with embedded quotes my $commalist = join( ', ', map { qq['$_'] } @$list ); - my $seq_name; if ( $data_type eq 'enum' ) { my $check_name = mk_name( @@ -256,13 +255,15 @@ sub produce { # my $default = $field->default_value; if ( defined $default ) { - $field_def .= sprintf( ' DEFAULT %s', - ( $field->is_auto_increment && $seq_name ) - ? qq[nextval('"$seq_name"'::text)] : - ( $default =~ m/null/i ) ? 'NULL' : "'$default'" + SQL::Translator::Producer->_apply_default_value( + \$field_def, + $default, + [ + qr/\Anull\z/i => \undef, + ], ); } - + push @field_defs, $field_def; } diff --git a/lib/SQL/Translator/Producer/SQLite.pm b/lib/SQL/Translator/Producer/SQLite.pm index a65043d..0cdcb3d 100644 --- a/lib/SQL/Translator/Producer/SQLite.pm +++ b/lib/SQL/Translator/Producer/SQLite.pm @@ -256,16 +256,19 @@ sub create_field # Default? XXX Need better quoting! my $default = $field->default_value; - if ( defined $default ) { - if ( uc $default eq 'NULL') { - $field_def .= ' DEFAULT NULL'; - } elsif ( $default eq 'now()' || - $default eq 'CURRENT_TIMESTAMP' ) { - $field_def .= ' DEFAULT CURRENT_TIMESTAMP'; - } elsif ( $default =~ /val\(/ ) { - next; - } else { - $field_def .= " DEFAULT '$default'"; + if (defined $default) { + # I don't know what this special case is for, but it was here when I + # got here. -- rjbs, 2008-09-30] + if (ref $default or $default !~ /val\(/) { + SQL::Translator::Producer->_apply_default_value( + \$field_def, + $default, + [ + qr/\Anull\z/i => \undef, + 'now()' => 'now()', + 'CURRENT_TIMESTAMP' => 'CURRENT_TIMESTAMP', + ], + ); } } diff --git a/t/47postgres-producer.t b/t/47postgres-producer.t index d52136f..43987c2 100644 --- a/t/47postgres-producer.t +++ b/t/47postgres-producer.t @@ -14,7 +14,7 @@ use FindBin qw/$Bin/; #============================================================================= BEGIN { - maybe_plan(7, + maybe_plan(11, 'SQL::Translator::Producer::PostgreSQL', 'Test::Differences', ) @@ -22,7 +22,7 @@ BEGIN { use Test::Differences; use SQL::Translator; - +my $PRODUCER = \&SQL::Translator::Producer::PostgreSQL::create_field; my $table = SQL::Translator::Schema::Table->new( name => 'mytable'); @@ -104,3 +104,71 @@ my $field5_sql = SQL::Translator::Producer::PostgreSQL::create_field($field5,{ p is($field5_sql, 'enum_field mytable_enum_field_type NOT NULL', 'Create real enum field works'); +{ + # let's test default values! -- rjbs, 2008-09-30 + my %field = ( + table => $table, + data_type => 'VARCHAR', + size => 10, + is_auto_increment => 0, + is_nullable => 1, + is_foreign_key => 0, + is_unique => 0, + ); + + { + my $simple_default = SQL::Translator::Schema::Field->new( + %field, + name => 'str_default', + default_value => 'foo', + ); + + is( + $PRODUCER->($simple_default), + q{str_default character varying(10) DEFAULT 'foo'}, + 'default str', + ); + } + + { + my $null_default = SQL::Translator::Schema::Field->new( + %field, + name => 'null_default', + default_value => \undef, + ); + + is( + $PRODUCER->($null_default), + q{null_default character varying(10) DEFAULT NULL}, + 'default null', + ); + } + + { + my $null_default = SQL::Translator::Schema::Field->new( + %field, + name => 'null_default_2', + default_value => 'NULL', # XXX: this should go away + ); + + is( + $PRODUCER->($null_default), + q{null_default_2 character varying(10) DEFAULT NULL}, + 'default null from special cased string', + ); + } + + { + my $func_default = SQL::Translator::Schema::Field->new( + %field, + name => 'func_default', + default_value => \'func(funky)', + ); + + is( + $PRODUCER->($func_default), + q{func_default character varying(10) DEFAULT func(funky)}, + 'unquoted default from scalar ref', + ); + } +}