From: Brandon L. Black Date: Wed, 12 Jul 2006 05:17:40 +0000 (+0000) Subject: Prevent S::A::L caching of $dbh X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=2cc3a7be389cb22aacc99425cf7ed3493b4ebf2a;p=dbsrgits%2FDBIx-Class-Historic.git Prevent S::A::L caching of $dbh Bugfix: dont lose track of sql_maker settings when forking setting sql_maker-related options via connect_info improved and docced better --- diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm index 6034a91..84dc7b8 100644 --- a/lib/DBIx/Class/Storage/DBI.pm +++ b/lib/DBIx/Class/Storage/DBI.pm @@ -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, 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, and the sql_maker options -C, C, and C. Examples: +In either case, if the final argument in your connect_info happens +to be a hashref, C 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 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 '`'>, and user SQL Server you'd +use C [qw/[ ]/]>. + +=item name_sep + +This only needs to be used in conjunction with L, 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 connection attributes, +or placed in a seperate hashref after all other normal L connection +arguments. + +Every time C 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. + +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. +This method is deprecated in favor of setting via L. =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. +See L for details. +For setting, this method is deprecated in favor of L. =item quote_char -Specifies what characters to use to quote table and column names. If -you use this you will want to specify L 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, and user SQL Server you'd -use C. - -This option can also be set via L. +See L for details. +For setting, this method is deprecated in favor of L. =item name_sep -This only needs to be used in conjunction with L, 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. +See L for details. +For setting, this method is deprecated in favor of L. =back diff --git a/t/19quotes_newstyle.t b/t/19quotes_newstyle.t new file mode 100644 index 0000000..65cd3aa --- /dev/null +++ b/t/19quotes_newstyle.t @@ -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"); +