Prevent S::A::L caching of $dbh
Brandon L. Black [Wed, 12 Jul 2006 05:17:40 +0000 (05:17 +0000)]
Bugfix: dont lose track of sql_maker settings when forking
setting sql_maker-related options via connect_info improved and docced better

lib/DBIx/Class/Storage/DBI.pm
t/19quotes_newstyle.t [new file with mode: 0644]

index 6034a91..84dc7b8 100644 (file)
@@ -17,6 +17,25 @@ package DBIC::SQL::Abstract; # Would merge upstream, but nate doesn't reply :(
 
 use base qw/SQL::Abstract::Limit/;
 
+# This prevents the caching of $dbh in S::A::L, I believe
+sub new {
+  my $self = shift->SUPER::new(@_);
+
+  # If limit_dialect is a ref (like a $dbh), go ahead and replace
+  #   it with what it resolves to:
+  $self->{limit_dialect} = $self->_find_syntax($self->{limit_dialect})
+    if ref $self->{limit_dialect};
+
+  $self;
+}
+
+# While we're at it, this should make LIMIT queries more efficient,
+#  without digging into things too deeply
+sub _find_syntax {
+  my ($self, $syntax) = @_;
+  $self->{_cached_syntax} ||= $self->SUPER::_find_syntax($syntax);
+}
+
 sub select {
   my ($self, $table, $fields, $where, $order, @rest) = @_;
   $table = $self->_quote($table) unless ref($table);
@@ -229,8 +248,8 @@ use base qw/DBIx::Class/;
 __PACKAGE__->load_components(qw/AccessorGroup/);
 
 __PACKAGE__->mk_group_accessors('simple' =>
-  qw/_connect_info _dbh _sql_maker _conn_pid _conn_tid debug debugobj
-     cursor on_connect_do transaction_depth/);
+  qw/_connect_info _dbh _sql_maker _sql_maker_opts _conn_pid _conn_tid
+     debug debugobj cursor on_connect_do transaction_depth/);
 
 =head1 NAME
 
@@ -268,6 +287,7 @@ sub new {
   }
   $new->debugfh($fh);
   $new->debug(1) if $debug_env;
+  $new->_sql_maker_opts({});
   return $new;
 }
 
@@ -294,40 +314,96 @@ The arrayref can either contain the same set of arguments one would
 normally pass to L<DBI/connect>, or a lone code reference which returns
 a connected database handle.
 
-In either case, there is an optional final element within the arrayref
-which can hold a hashref of connection-specific Storage::DBI options.
-These include C<on_connect_do>, and the sql_maker options
-C<limit_dialect>, C<quote_char>, and C<name_sep>.  Examples:
+In either case, if the final argument in your connect_info happens
+to be a hashref, C<connect_info> will look there for several
+connection-specific options:
+
+=over 4
+
+=item on_connect_do
+
+This can be set to an arrayref of literal sql statements, which will
+be executed immediately after making the connection to the database
+every time we [re-]connect.
+
+=item limit_dialect 
+
+Sets the limit dialect. This is useful for JDBC-bridge among others
+where the remote SQL-dialect cannot be determined by the name of the
+driver alone.
+
+=item quote_char
 
+Specifies what characters to use to quote table and column names. If 
+you use this you will want to specify L<name_sep> as well.
+
+quote_char expects either a single character, in which case is it is placed
+on either side of the table/column, or an arrayref of length 2 in which case the
+table/column name is placed between the elements.
+
+For example under MySQL you'd use C<quote_char =E<gt> '`'>, and user SQL Server you'd 
+use C<quote_char =E<gt> [qw/[ ]/]>.
+
+=item name_sep
+
+This only needs to be used in conjunction with L<quote_char>, and is used to 
+specify the charecter that seperates elements (schemas, tables, columns) from 
+each other. In most cases this is simply a C<.>.
+
+=back
+
+These options can be mixed in with your other L<DBI> connection attributes,
+or placed in a seperate hashref after all other normal L<DBI> connection
+arguments.
+
+Every time C<connect_info> is invoked, any previous settings for
+these options will be cleared before setting the new ones, regardless of
+whether any options are specified in the new C<connect_info>.
+
+Examples:
+
+  # Simple SQLite connection
   ->connect_info([ 'dbi:SQLite:./foo.db' ]);
 
+  # Connect via subref
   ->connect_info([ sub { DBI->connect(...) } ]);
 
+  # A bit more complicated
   ->connect_info(
     [
       'dbi:Pg:dbname=foo',
       'postgres',
       'my_pg_password',
       { AutoCommit => 0 },
-      { quote_char => q{`}, name_sep => q{@} },
+      { quote_char => q{"}, name_sep => q{.} },
+    ]
+  );
+
+  # Equivalent to the previous example
+  ->connect_info(
+    [
+      'dbi:Pg:dbname=foo',
+      'postgres',
+      'my_pg_password',
+      { AutoCommit => 0, quote_char => q{"}, name_sep => q{.} },
     ]
   );
 
+  # Subref + DBIC-specific connection options
   ->connect_info(
     [
       sub { DBI->connect(...) },
-      { quote_char => q{`}, name_sep => q{@} },
+      {
+          quote_char => q{`},
+          name_sep => q{@},
+          on_connect_do => ['SET search_path TO myschema,otherschema,public'],
+      },
     ]
   );
 
 =head2 on_connect_do
 
-  $schema->storage->on_connect_do(['PRAGMA synchronous = OFF']);
-
-Call this after C<< $schema->connect >> to have the sql statements
-given executed on every db connect.
-
-This option can also be set via L</connect_info>.
+This method is deprecated in favor of setting via L</connect_info>.
 
 =head2 debug
 
@@ -404,13 +480,11 @@ sub connected { my ($self) = @_;
 
   if(my $dbh = $self->_dbh) {
       if(defined $self->_conn_tid && $self->_conn_tid != threads->tid) {
-          $self->_sql_maker(undef);
           return $self->_dbh(undef);
       }
       elsif($self->_conn_pid != $$) {
           $self->_dbh->{InactiveDestroy} = 1;
-          $self->_sql_maker(undef);
-          return $self->_dbh(undef)
+          return $self->_dbh(undef);
       }
       return ($dbh->FETCH('Active') && $dbh->ping);
   }
@@ -449,7 +523,7 @@ sub dbh {
 sub _sql_maker_args {
     my ($self) = @_;
     
-    return ( limit_dialect => $self->dbh );
+    return ( limit_dialect => $self->dbh, %{$self->_sql_maker_opts} );
 }
 
 =head2 sql_maker
@@ -471,30 +545,28 @@ sub connect_info {
   my ($self, $info_arg) = @_;
 
   if($info_arg) {
-    my %sql_maker_opts;
+    # Kill sql_maker/_sql_maker_opts, so we get a fresh one with only
+    #  the new set of options
+    $self->_sql_maker(undef);
+    $self->_sql_maker_opts({});
+
     my $info = [ @$info_arg ]; # copy because we can alter it
     my $last_info = $info->[-1];
     if(ref $last_info eq 'HASH') {
-      my $used;
-      if(my $on_connect_do = $last_info->{on_connect_do}) {
-        $used = 1;
+      if(my $on_connect_do = delete $last_info->{on_connect_do}) {
         $self->on_connect_do($on_connect_do);
       }
       for my $sql_maker_opt (qw/limit_dialect quote_char name_sep/) {
-        if(my $opt_val = $last_info->{$sql_maker_opt}) {
-          $used = 1;
-          $sql_maker_opts{$sql_maker_opt} = $opt_val;
+        if(my $opt_val = delete $last_info->{$sql_maker_opt}) {
+          $self->_sql_maker_opts->{$sql_maker_opt} = $opt_val;
         }
       }
 
-      # remove our options hashref if it was there, to avoid confusing
-      #   DBI in the case the user didn't use all 4 DBI options, as in:
-      #   [ 'dbi:SQLite:foo.db', { quote_char => q{`} } ]
-      pop(@$info) if $used;
+      # Get rid of any trailing empty hashref
+      pop(@$info) if !keys %$last_info;
     }
 
     $self->_connect_info($info);
-    $self->sql_maker->$_($sql_maker_opts{$_}) for(keys %sql_maker_opts);
   }
 
   $self->_connect_info;
@@ -1036,33 +1108,18 @@ The following methods are extended:-
 
 =item limit_dialect
 
-Accessor for setting limit dialect. This is useful
-for JDBC-bridge among others where the remote SQL-dialect cannot
-be determined by the name of the driver alone.
-
-This option can also be set via L</connect_info>.
+See L</connect_info> for details.
+For setting, this method is deprecated in favor of L</connect_info>.
 
 =item quote_char
 
-Specifies what characters to use to quote table and column names. If 
-you use this you will want to specify L<name_sep> as well.
-
-quote_char expectes either a single character, in which case is it is placed
-on either side of the table/column, or an arrayref of length 2 in which case the
-table/column name is placed between the elements.
-
-For example under MySQL you'd use C<quote_char('`')>, and user SQL Server you'd 
-use C<quote_char(qw/[ ]/)>.
-
-This option can also be set via L</connect_info>.
+See L</connect_info> for details.
+For setting, this method is deprecated in favor of L</connect_info>.
 
 =item name_sep
 
-This only needs to be used in conjunction with L<quote_char>, and is used to 
-specify the charecter that seperates elements (schemas, tables, columns) from 
-each other. In most cases this is simply a C<.>.
-
-This option can also be set via L</connect_info>.
+See L</connect_info> for details.
+For setting, this method is deprecated in favor of L</connect_info>.
 
 =back
 
diff --git a/t/19quotes_newstyle.t b/t/19quotes_newstyle.t
new file mode 100644 (file)
index 0000000..65cd3aa
--- /dev/null
@@ -0,0 +1,64 @@
+use strict;
+use warnings;
+
+use Test::More;
+use IO::File;
+
+BEGIN {
+    eval "use DBD::SQLite";
+    plan $@
+        ? ( skip_all => 'needs DBD::SQLite for testing' )
+        : ( tests => 6 );
+}
+
+use lib qw(t/lib);
+
+use_ok('DBICTest');
+DBICTest->init_schema();
+
+my $dsn = DBICTest->schema->storage->connect_info->[0];
+
+DBICTest->schema->connection($dsn, { quote_char => "'", name_sep => '.' });
+
+my $rs = DBICTest::CD->search(
+           { 'me.year' => 2001, 'artist.name' => 'Caterwauler McCrae' },
+           { join => 'artist' });
+
+cmp_ok( $rs->count, '==', 1, "join with fields quoted");
+
+$rs = DBICTest::CD->search({},
+            { 'order_by' => 'year DESC'});
+{
+       my $warnings = '';
+       local $SIG{__WARN__} = sub { $warnings .= $_[0] };
+       my $first = eval{ $rs->first() };
+       like( $warnings, qr/ORDER BY terms/, "Problem with ORDER BY quotes" );
+}
+
+my $order = 'year DESC';
+$rs = DBICTest::CD->search({},
+            { 'order_by' => \$order });
+{
+       my $warnings = '';
+       local $SIG{__WARN__} = sub { $warnings .= $_[0] };
+       my $first = $rs->first();
+       ok( $warnings !~ /ORDER BY terms/,
+            "No problem handling ORDER by scalaref" );
+}
+
+DBICTest->schema->connection($dsn, { quote_char => [qw/[ ]/], name_sep => '.' });
+
+$rs = DBICTest::CD->search(
+           { 'me.year' => 2001, 'artist.name' => 'Caterwauler McCrae' },
+           { join => 'artist' });
+cmp_ok($rs->count,'==', 1,"join quoted with brackets.");
+
+my %data = (
+       name => 'Bill',
+       order => '12'
+);
+
+DBICTest->schema->connection($dsn, { quote_char => '`', name_sep => '.' });
+
+cmp_ok(DBICTest->schema->storage->sql_maker->update('group', \%data), 'eq', 'UPDATE `group` SET `name` = ?, `order` = ?', "quoted table names for UPDATE");
+