A workable fix for the resultset multicol update/delete debacle - by default fallback...

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index 22c7237..079e0e4 100644 (file)
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -1326,6 +1326,18 @@ sub first {
   return $_[0]->reset->next;
 }
 
+
+# _update_delete_via_subq
+#
+# Presence of some rs attributes requires a subquery to reliably 
+# update/deletre
+#
+
+sub _update_delete_via_subq {
+  return $_[0]->_has_attr (qw/join seen_join group_by row offset page/);
+}
+
+
 # _cond_for_update_delete
 #
 # update/delete require the condition to be modified to handle
@@ -1340,18 +1352,6 @@ sub _cond_for_update_delete {
   # No-op. No condition, we're updating/deleting everything
   return $cond unless ref $full_cond;
 
-  # Some attributes when present require a subquery
-  # This might not work on some database (mysql), but...
-  # it won't work without the subquery either so who cares
-  if ($self->_has_attr (qw/join seen_join group_by row offset page/) ) {
-
-    foreach my $pk ($self->result_source->primary_columns) {
-      $cond->{$pk} = { -in => $self->get_column($pk)->as_query };
-    }
-
-    return $cond;
-  }
-
   if (ref $full_cond eq 'ARRAY') {
     $cond = [
       map {
@@ -1391,7 +1391,7 @@ sub _cond_for_update_delete {
   else {
     $self->throw_exception("Can't update/delete on resultset with condition unless hash or array");
   }
- 
+
   return $cond;
 }
 
@@ -1414,9 +1414,14 @@ if no records were updated; exact type of success value is storage-dependent.
 
 sub update {
   my ($self, $values) = @_;
-  $self->throw_exception("Values for update must be a hash")
+  $self->throw_exception('Values for update must be a hash')
     unless ref $values eq 'HASH';
 
+  # rs operations with subqueries are Storage dependent - delegate
+  if ($self->_update_delete_via_subq) {
+    return $self->result_source->storage->subq_update_delete($self, 'update', $values);
+  }
+
   my $cond = $self->_cond_for_update_delete;
 
   return $self->result_source->storage->update(
@@ -1441,7 +1446,7 @@ will run DBIC cascade triggers, while L</update> will not.
 
 sub update_all {
   my ($self, $values) = @_;
-  $self->throw_exception("Values for update must be a hash")
+  $self->throw_exception('Values for update_all must be a hash')
     unless ref $values eq 'HASH';
   foreach my $obj ($self->all) {
     $obj->set_columns($values)->update;
@@ -1472,9 +1477,14 @@ need to respecify your query in a way that can be expressed without a join.
 =cut
 
 sub delete {
-  my ($self) = @_;
-  $self->throw_exception("Delete should not be passed any arguments")
-    if $_[1];
+  my $self = shift;
+  $self->throw_exception('delete does not accept any arguments')
+    if @_;
+
+  # rs operations with subqueries are Storage dependent - delegate
+  if ($self->_update_delete_via_subq) {
+    return $self->result_source->storage->subq_update_delete($self, 'delete');
+  }
 
   my $cond = $self->_cond_for_update_delete;
 
@@ -1498,7 +1508,10 @@ will run DBIC cascade triggers, while L</delete> will not.
 =cut
 
 sub delete_all {
-  my ($self) = @_;
+  my $self = shift;
+  $self->throw_exception('delete_all does not accept any arguments')
+    if @_;
+
   $_->delete for $self->all;
   return 1;
 }

diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index 8df1894..8eb67fa 100644 (file)
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -1051,6 +1051,85 @@ sub delete {
   return $self->_execute('delete' => [], $source, $bind_attrs, @_);
 }
 
+# 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)
+#
+# Genarating a single PK column subquery is trivial and supported
+# by all RDBMS. However if we have a multicolumn PK, things get ugly.
+# Look at multipk_update_delete()
+sub subq_update_delete {
+  my $self = shift;
+  my ($rs, $op, $values) = @_;
+
+  if ($rs->result_source->primary_columns == 1) {
+    return $self->_onepk_update_delete (@_);
+  }
+  else {
+    return $self->_multipk_update_delete (@_);
+  }
+}
+
+# Generally a single PK resultset operation is trivially expressed
+# with PK IN (subquery). However some databases (mysql) do not support
+# modification of a table mentioned in the subselect. This method
+# should be overriden in the appropriate storage class to be smarter
+# in such situations
+sub _onepk_update_delete {
+
+  my $self = shift;
+  my ($rs, $op, $values) = @_;
+
+  my $rsrc = $rs->result_source;
+  my @pcols = $rsrc->primary_columns;
+
+  return $self->$op (
+    $rsrc,
+    $op eq 'update' ? $values : (),
+    { $pcols[0] => { -in => $rs->get_column ($pcols[0])->as_query } },
+  );
+}
+
+# ANSI SQL does not provide a reliable way to perform a multicol-PK
+# resultset update/delete involving subqueries. So resort to simple
+# (and inefficient) delete_all style per-row opearations, while allowing
+# specific storages to override this with a faster implementation.
+#
+# We do not use $row->$op style queries, because resultset update/delete
+# is not expected to cascade (this is what delete_all/update_all is for).
+#
+# There should be no race conditions as the entire operation is rolled
+# in a transaction.
+sub _multipk_update_delete {
+  my $self = shift;
+  my ($rs, $op, $values) = @_;
+
+  my $rsrc = $rs->result_source;
+  my @pcols = $rsrc->primary_columns;
+
+  my $guard = $self->txn_scope_guard;
+
+  my $subrs_cur = $rs->search ({}, { columns => \@pcols })->cursor;
+  while (my @pks = $subrs_cur->next) {
+
+    my $cond;
+    for my $i (0.. $#pcols) {
+      $cond->{$pcols[$i]} = $pks[$i];
+    }
+
+    $self->$op (
+      $rsrc,
+      $op eq 'update' ? $values : (),
+      $cond,
+    );
+  }
+
+  $guard->commit;
+
+  return 1;
+}
+
+
 sub _select {
   my $self = shift;
   my $sql_maker = $self->sql_maker;