non-destructive hashref handling for connect_info options

[dbsrgits/DBIx-Class.git] / lib / DBIx / Class / Storage / DBI.pm

diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index d7b9cf9..336ab1f 100644 (file)
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -9,7 +9,6 @@ use DBI;
 use SQL::Abstract::Limit;
 use DBIx::Class::Storage::DBI::Cursor;
 use DBIx::Class::Storage::Statistics;
-use IO::File;
 use Scalar::Util qw/blessed weaken/;
 
 __PACKAGE__->mk_group_accessors('simple' =>
@@ -116,7 +115,7 @@ sub _emulate_limit {
 }
 
 sub _recurse_fields {
-  my ($self, $fields) = @_;
+  my ($self, $fields, $params) = @_;
   my $ref = ref $fields;
   return $self->_quote($fields) unless $ref;
   return $$fields if $ref eq 'SCALAR';
@@ -124,10 +123,10 @@ sub _recurse_fields {
   if ($ref eq 'ARRAY') {
     return join(', ', map {
       $self->_recurse_fields($_)
-      .(exists $self->{rownum_hack_count}
-         ? ' AS col'.$self->{rownum_hack_count}++
-         : '')
-     } @$fields);
+        .(exists $self->{rownum_hack_count} && !($params && $params->{no_rownum_hack})
+          ? ' AS col'.$self->{rownum_hack_count}++
+          : '')
+      } @$fields);
   } elsif ($ref eq 'HASH') {
     foreach my $func (keys %$fields) {
       return $self->_sqlcase($func)
@@ -143,7 +142,7 @@ sub _order_by {
   if (ref $_[0] eq 'HASH') {
     if (defined $_[0]->{group_by}) {
       $ret = $self->_sqlcase(' group by ')
-               .$self->_recurse_fields($_[0]->{group_by});
+        .$self->_recurse_fields($_[0]->{group_by}, { no_rownum_hack => 1 });
     }
     if (defined $_[0]->{having}) {
       my $frag;
@@ -383,15 +382,14 @@ each other. In most cases this is simply a C<.>.
 =item unsafe
 
 This Storage driver normally installs its own C<HandleError>, sets
-C<RaiseError> on, and sets C<PrintError> off on all database handles,
-including those supplied by a coderef.  It does this so that it can
-have consistent and useful error behavior.
+C<RaiseError> and C<ShowErrorStatement> on, and sets C<PrintError> off on
+all database handles, including those supplied by a coderef.  It does this
+so that it can have consistent and useful error behavior.
 
 If you set this option to a true value, Storage will not do its usual
-modifications to the database handle's C<RaiseError>, C<PrintError>, and
-C<HandleError> attributes, and instead relies on the settings in your
-connect_info DBI options (or the values you set in your connection
-coderef, in the case that you are connecting via coderef).
+modifications to the database handle's attributes, and instead relies on
+the settings in your connect_info DBI options (or the values you set in
+your connection coderef, in the case that you are connecting via coderef).
 
 Note that your custom settings can cause Storage to malfunction,
 especially if you set a C<HandleError> handler that suppresses exceptions
@@ -481,6 +479,7 @@ sub connect_info {
 
   my $last_info = $dbi_info->[-1];
   if(ref $last_info eq 'HASH') {
+    $last_info = { %$last_info }; # so delete is non-destructive
     for my $storage_opt (qw/on_connect_do disable_sth_caching unsafe/) {
       if(my $value = delete $last_info->{$storage_opt}) {
         $self->$storage_opt($value);
@@ -491,6 +490,8 @@ sub connect_info {
         $self->_sql_maker_opts->{$sql_maker_opt} = $opt_val;
       }
     }
+    # re-insert modified hashref
+    $dbi_info->[-1] = $last_info;
 
     # Get rid of any trailing empty hashref
     pop(@$dbi_info) if !keys %$last_info;
@@ -773,6 +774,7 @@ sub _connect {
       $dbh->{HandleError} = sub {
           $weak_self->throw_exception("DBI Exception: $_[0]")
       };
+      $dbh->{ShowErrorStatement} = 1;
       $dbh->{RaiseError} = 1;
       $dbh->{PrintError} = 0;
     }
@@ -789,6 +791,7 @@ sub _connect {
 
 sub txn_begin {
   my $self = shift;
+  $self->ensure_connected();
   if($self->{transaction_depth}++ == 0) {
     $self->debugobj->txn_begin()
       if $self->debug;
@@ -1200,7 +1203,7 @@ sub bind_attribute_by_data_type {
     return;
 }
 
-=head2 create_ddl_dir (EXPERIMENTAL)
+=head2 create_ddl_dir
 
 =over 4
 
@@ -1211,9 +1214,6 @@ sub bind_attribute_by_data_type {
 Creates a SQL file based on the Schema, for each of the specified
 database types, in the given directory.
 
-Note that this feature is currently EXPERIMENTAL and may not work correctly
-across all databases, or fully handle complex relationships.
-
 =cut
 
 sub create_ddl_dir
@@ -1404,16 +1404,21 @@ sub deployment_statements {
 sub deploy {
   my ($self, $schema, $type, $sqltargs, $dir) = @_;
   foreach my $statement ( $self->deployment_statements($schema, $type, undef, $dir, { no_comments => 1, %{ $sqltargs || {} } } ) ) {
-    for ( split(";\n", $statement)) {
-      next if($_ =~ /^--/);
-      next if(!$_);
-#      next if($_ =~ /^DROP/m);
-      next if($_ =~ /^BEGIN TRANSACTION/m);
-      next if($_ =~ /^COMMIT/m);
-      next if $_ =~ /^\s+$/; # skip whitespace only
-      $self->debugobj->query_start($_) if $self->debug;
-      $self->dbh->do($_); # shouldn't be using ->dbh ?
-      $self->debugobj->query_end($_) if $self->debug;
+    foreach my $line ( split(";\n", $statement)) {
+      next if($line =~ /^--/);
+      next if(!$line);
+#      next if($line =~ /^DROP/m);
+      next if($line =~ /^BEGIN TRANSACTION/m);
+      next if($line =~ /^COMMIT/m);
+      next if $line =~ /^\s+$/; # skip whitespace only
+      $self->debugobj->query_start($line) if $self->debug;
+      eval {
+        $self->dbh->do($line); # shouldn't be using ->dbh ?
+      };
+      if ($@) {
+        warn qq{$@ (running "${line}")};
+      }
+      $self->debugobj->query_end($line) if $self->debug;
     }
   }
 }
@@ -1426,7 +1431,10 @@ Returns the datetime parser class
 
 sub datetime_parser {
   my $self = shift;
-  return $self->{datetime_parser} ||= $self->build_datetime_parser(@_);
+  return $self->{datetime_parser} ||= do {
+    $self->ensure_connected;
+    $self->build_datetime_parser(@_);
+  };
 }
 
 =head2 datetime_parser_type