From: Peter Rabbitson Date: Tue, 29 Jul 2014 05:12:16 +0000 (+0200) Subject: The fix in f9b5239ac was both shortsighted and insufficient X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=08ac7648;p=dbsrgits%2FDBIx-Class-Historic.git The fix in f9b5239ac was both shortsighted and insufficient Move the quote_names => quote_identifiers inferrence into ::Storage::DBI proper, and ensure tests cover it this time Also bump SQLT dep, as the previous version throws on diffing DBICTest::Schema under SQLite --- diff --git a/Changes b/Changes index 64a472d..2831119 100644 --- a/Changes +++ b/Changes @@ -47,6 +47,8 @@ Revision history for DBIx::Class - Ensure that setting a column to a literal invariably marks it dirty - Work around exception objects with broken string overloading in one additional codepath (missed in 0.08260) + - Fix more inconsistencies of the quote_names attribute propagating + to SQL::Translator (partially RT#87731) - Fix inability to handle multiple consecutive transactions with savepoints on DBD::SQLite < 1.39 - Fix CDBICompat to match Class::DBI behavior handling non-result diff --git a/lib/DBIx/Class/Optional/Dependencies.pm b/lib/DBIx/Class/Optional/Dependencies.pm index 0bbc6c4..2806159 100644 --- a/lib/DBIx/Class/Optional/Dependencies.pm +++ b/lib/DBIx/Class/Optional/Dependencies.pm @@ -150,7 +150,7 @@ my $reqs = { deploy => { req => { - 'SQL::Translator' => '0.11016', + 'SQL::Translator' => '0.11018', }, pod => { title => 'Storage::DBI::deploy()', diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm index 139dc49..bd201a1 100644 --- a/lib/DBIx/Class/Storage/DBI.pm +++ b/lib/DBIx/Class/Storage/DBI.pm @@ -2872,6 +2872,7 @@ sub create_ddl_dir { add_drop_table => 1, ignore_constraint_names => 1, ignore_index_names => 1, + quote_identifiers => !!$self->sql_maker->_quote_chars, %{$sqltargs || {}} }; @@ -2966,10 +2967,21 @@ sub create_ddl_dir { unless $dest_schema->name; } - my $diff = SQL::Translator::Diff::schema_diff($source_schema, $db, - $dest_schema, $db, - $sqltargs - ); + my $diff = do { + # FIXME - this is a terrible workaround for + # https://github.com/dbsrgits/sql-translator/commit/2d23c1e + # Fixing it in this sloppy manner so that we don't hve to + # lockstep an SQLT release as well. Needs to be removed at + # some point, and SQLT dep bumped + local $SQL::Translator::Producer::SQLite::NO_QUOTES + if $SQL::Translator::Producer::SQLite::NO_QUOTES; + + SQL::Translator::Diff::schema_diff($source_schema, $db, + $dest_schema, $db, + $sqltargs + ); + }; + if(!open $file, ">$difffile") { $self->throw_exception("Can't write to $difffile ($!)"); next; @@ -3029,6 +3041,10 @@ sub deployment_statements { $sqltargs->{parser_args}{sources} = delete $sqltargs->{sources} if exists $sqltargs->{sources}; + $sqltargs->{quote_identifiers} + = !!$self->sql_maker->_quote_chars + if ! exists $sqltargs->{quote_identifiers}; + my $tr = SQL::Translator->new( producer => "SQL::Translator::Producer::${type}", %$sqltargs, diff --git a/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm b/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm index d763953..84799f1 100644 --- a/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm +++ b/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm @@ -103,9 +103,6 @@ sub deployment_statements { my ($schema, $type, $version, $dir, $sqltargs, @rest) = @_; $sqltargs ||= {}; - my $quote_char = $self->schema->storage->sql_maker->quote_char; - $sqltargs->{quote_table_names} = $quote_char ? 1 : 0; - $sqltargs->{quote_field_names} = $quote_char ? 1 : 0; if ( ! exists $sqltargs->{producer_args}{oracle_version} diff --git a/lib/DBIx/Class/Storage/DBI/SQLite.pm b/lib/DBIx/Class/Storage/DBI/SQLite.pm index fa6f806..ca21607 100644 --- a/lib/DBIx/Class/Storage/DBI/SQLite.pm +++ b/lib/DBIx/Class/Storage/DBI/SQLite.pm @@ -245,10 +245,6 @@ sub deployment_statements { $sqltargs->{producer_args}{sqlite_version} = $dver; } - $sqltargs->{quote_identifiers} - = !!$self->sql_maker->_quote_chars - if ! exists $sqltargs->{quote_identifiers}; - $self->next::method($schema, $type, $version, $dir, $sqltargs, @rest); } diff --git a/t/storage/deploy.t b/t/storage/deploy.t index 433f58e..46e6a36 100644 --- a/t/storage/deploy.t +++ b/t/storage/deploy.t @@ -31,18 +31,42 @@ lives_ok( sub { $parse_schema->resultset("Artist")->all(); }, 'artist table deployed correctly' ); -my $schema = DBICTest->init_schema(); +my $schema = DBICTest->init_schema( quote_names => 1 ); my $var = dir ("t/var/ddl_dir-$$"); $var->mkpath unless -d $var; my $test_dir_1 = $var->subdir ('test1', 'foo', 'bar' ); $test_dir_1->rmtree if -d $test_dir_1; -$schema->create_ddl_dir( undef, undef, $test_dir_1 ); +$schema->create_ddl_dir( [qw(SQLite MySQL)], 1, $test_dir_1 ); ok( -d $test_dir_1, 'create_ddl_dir did a make_path on its target dir' ); ok( scalar( glob $test_dir_1.'/*.sql' ), 'there are sql files in there' ); +my $less = $schema->clone; +$less->unregister_source('BindType'); +$less->create_ddl_dir( [qw(SQLite MySQL)], 2, $test_dir_1, 1 ); + +for ( + [ SQLite => '"' ], + [ MySQL => '`' ], +) { + my $type = $_->[0]; + my $q = quotemeta($_->[1]); + + for my $f (map { $test_dir_1->file("DBICTest-Schema-${_}-$type.sql") } qw(1 2) ) { + like scalar $f->slurp, qr/CREATE TABLE ${q}track${q}/, "Proper quoting in $f"; + } + + { + local $TODO = 'SQLT::Producer::MySQL has no knowledge of the mythical beast of quoting...' + if $type eq 'MySQL'; + + my $f = $test_dir_1->file("DBICTest-Schema-1-2-$type.sql"); + like scalar $f->slurp, qr/DROP TABLE ${q}bindtype_test${q}/, "Proper quoting in diff $f"; + } +} + { local $TODO = 'we should probably add some tests here for actual deployability of the DDL?'; ok( 0 );