Merge 'trunk' into 'prefetch_bug-unqualified_column_in_search_related_cond'
Peter Rabbitson [Fri, 13 Nov 2009 09:39:55 +0000 (09:39 +0000)]
r7884@Thesaurus (orig r7872):  ribasushi | 2009-11-13 00:13:40 +0100
The real fix for the non-introspectable condition bug, mst++
r7885@Thesaurus (orig r7873):  ribasushi | 2009-11-13 00:24:56 +0100
Some cleanup
r7887@Thesaurus (orig r7875):  frew | 2009-11-13 10:01:37 +0100
fix subtle bug with Sybase database type determination

lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/Storage/DBI.pm
lib/DBIx/Class/Storage/DBI/Sybase.pm

index 58e9b07..61b2a16 100644 (file)
@@ -1426,8 +1426,12 @@ sub _rs_update_delete {
 
   my $rsrc = $self->result_source;
 
+  # if a condition exists we need to strip all table qualifiers
+  # if this is not possible we'll force a subquery below
+  my $cond = $rsrc->schema->storage->_strip_cond_qualifiers ($self->{cond});
+
   my $needs_group_by_subq = $self->_has_resolved_attr (qw/collapse group_by -join/);
-  my $needs_subq = $self->_has_resolved_attr (qw/row offset/);
+  my $needs_subq = (not defined $cond) || $self->_has_resolved_attr(qw/row offset/);
 
   if ($needs_group_by_subq or $needs_subq) {
 
@@ -1475,7 +1479,7 @@ sub _rs_update_delete {
     return $rsrc->storage->$op(
       $rsrc,
       $op eq 'update' ? $values : (),
-      $self->{cond},
+      $cond,
     );
   }
 }
index aab6f31..e11a8e9 100644 (file)
@@ -1549,48 +1549,81 @@ sub _dbh_execute_inserts_with_no_binds {
 }
 
 sub update {
-  my ($self, $source, $data, $where, @args) = @_; 
+  my ($self, $source, @args) = @_; 
 
   my $bind_attrs = $self->source_bind_attributes($source);
-  $where = $self->_strip_cond_qualifiers ($where);
 
-  return $self->_execute('update' => [], $source, $bind_attrs, $data, $where, @args);
+  return $self->_execute('update' => [], $source, $bind_attrs, @args);
 }
 
 
 sub delete {
-  my ($self, $source, $where, @args) = @_;
+  my ($self, $source, @args) = @_;
 
   my $bind_attrs = $self->source_bind_attributes($source);
-  $where = $self->_strip_cond_qualifiers ($where);
 
-  return $self->_execute('delete' => [], $source, $bind_attrs, $where, @args);
+  return $self->_execute('delete' => [], $source, $bind_attrs, @args);
 }
 
 # Most databases do not allow aliasing of tables in UPDATE/DELETE. Thus
 # a condition containing 'me' or other table prefixes will not work
-# at all. Since we employ subqueries when multiple tables are involved
-# (joins), it is relatively safe to strip all column qualifiers. Worst
-# case scenario the error message will be a bit misleading, if the
-# user supplies a foreign qualifier without a join (the message would
-# be "can't find column X", when in fact the user shoud join T containing
-# T.X)
+# at all. What this code tries to do (badly) is introspect the condition
+# and remove all column qualifiers. If it bails out early (returns undef)
+# the calling code should try another approach (e.g. a subquery)
 sub _strip_cond_qualifiers {
   my ($self, $where) = @_;
 
-  my $sqlmaker = $self->sql_maker;
-  my ($sql, @bind) = $sqlmaker->_recurse_where($where);
-  return undef unless $sql;
+  my $cond = {};
 
-  my ($qquot, $qsep) = map { quotemeta $_ } ( ($sqlmaker->quote_char||''), ($sqlmaker->name_sep||'.') );
-  $sql =~ s/ (?: $qquot [\w\-]+ $qquot | [\w\-]+ ) $qsep //gx;
+  # No-op. No condition, we're updating/deleting everything
+  return $cond unless $where;
 
-  return \[$sql, @bind];
+  if (ref $where eq 'ARRAY') {
+    $cond = [
+      map {
+        my %hash;
+        foreach my $key (keys %{$_}) {
+          $key =~ /([^.]+)$/;
+          $hash{$1} = $_->{$key};
+        }
+        \%hash;
+      } @$where
+    ];
+  }
+  elsif (ref $where eq 'HASH') {
+    if ( (keys %$where) == 1 && ( (keys %{$where})[0] eq '-and' )) {
+      $cond->{-and} = [];
+      my @cond = @{$where->{-and}};
+       for (my $i = 0; $i < @cond; $i++) {
+        my $entry = $cond[$i];
+        my $hash;
+        if (ref $entry eq 'HASH') {
+          $hash = $self->_strip_cond_qualifiers($entry);
+        }
+        else {
+          $entry =~ /([^.]+)$/;
+          $hash->{$1} = $cond[++$i];
+        }
+        push @{$cond->{-and}}, $hash;
+      }
+    }
+    else {
+      foreach my $key (keys %$where) {
+        $key =~ /([^.]+)$/;
+        $cond->{$1} = $where->{$key};
+      }
+    }
+  }
+  else {
+    return undef;
+  }
+
+  return $cond;
 }
 
 # We were sent here because the $rs contains a complex search
 # which will require a subquery to select the correct rows
-# (i.e. joined or limited resultsets)
+# (i.e. joined or limited resultsets, or non-introspectable conditions)
 #
 # Generating a single PK column subquery is trivial and supported
 # by all RDBMS. However if we have a multicolumn PK, things get ugly.
@@ -1601,16 +1634,7 @@ sub _subq_update_delete {
 
   my $rsrc = $rs->result_source;
 
-  # we already check this, but double check naively just in case. Should be removed soon
-  my $sel = $rs->_resolved_attrs->{select};
-  $sel = [ $sel ] unless ref $sel eq 'ARRAY';
   my @pcols = $rsrc->primary_columns;
-  if (@$sel != @pcols) {
-    $self->throw_exception (
-      'Subquery update/delete can not be called on resultsets selecting a'
-     .' number of columns different than the number of primary keys'
-    );
-  }
 
   if (@pcols == 1) {
     return $self->$op (
index eeb4f01..8cb5f5f 100644 (file)
@@ -63,12 +63,13 @@ sub _rebless {
     my $dbtype = eval {
       @{$self->_get_dbh->selectrow_arrayref(qq{sp_server_info \@attribute_id=1})}[2]
     } || '';
+    $self->throw_exception("Unable to estable connection to determine database type: $@")
+      if $@;
 
-    my $exception = $@;
     $dbtype =~ s/\W/_/gi;
     my $subclass = "DBIx::Class::Storage::DBI::Sybase::${dbtype}";
 
-    if (!$exception && $dbtype && $self->load_optional_class($subclass)) {
+    if ($dbtype && $self->load_optional_class($subclass)) {
       bless $self, $subclass;
       $self->_rebless;
     } else { # real Sybase
@@ -189,7 +190,7 @@ sub _populate_dbh {
   my $self = shift;
 
   $self->next::method(@_);
-  
+
   if ($self->_is_bulk_storage) {
 # this should be cleared on every reconnect
     $self->_began_bulk_work(0);
@@ -381,7 +382,7 @@ sub insert {
   # we are already in a transaction, or there are no blobs
   # and we don't need the PK - just (try to) do it
   if ($self->{transaction_depth}
-        || (!$blob_cols && !$dumb_last_insert_id) 
+        || (!$blob_cols && !$dumb_last_insert_id)
   ) {
     return $self->_insert (
       $next, $source, $to_insert, $blob_cols, $identity_col
@@ -511,7 +512,7 @@ EOF
 
 # _execute_array uses a txn anyway, but it ends too early in case we need to
 # select max(col) to get the identity for inserting blobs.
-    ($self, my $guard) = $self->{transaction_depth} == 0 ? 
+    ($self, my $guard) = $self->{transaction_depth} == 0 ?
       ($self->_writer_storage, $self->_writer_storage->txn_scope_guard)
       :
       ($self, undef);