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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 );