From: Peter Rabbitson <ribasushi@cpan.org>
Date: Sat, 14 Apr 2012 08:45:32 +0000 (+0200)
Subject: Take into account resultset conditions when determining rs order stability
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=5f11e54f1dc812354b8d160d5b286502cc227cbf;p=dbsrgits%2FDBIx-Class-Historic.git

Take into account resultset conditions when determining rs order stability

This takes care of the overly-aggressive exception scenario introduced by
86bb5a27
---

diff --git a/lib/DBIx/Class/SQLMaker/LimitDialects.pm b/lib/DBIx/Class/SQLMaker/LimitDialects.pm
index 3df6daa..84cc5a3 100644
--- a/lib/DBIx/Class/SQLMaker/LimitDialects.pm
+++ b/lib/DBIx/Class/SQLMaker/LimitDialects.pm
@@ -279,7 +279,7 @@ EOS
     $rs_attrs->{order_by}
       and
     $rs_attrs->{_rsroot_rsrc}->storage->_order_by_is_stable(
-      $rs_attrs->{from}, $rs_attrs->{order_by}
+      @{$rs_attrs}{qw/from order_by where/}
     )
   ) {
     push @{$self->{limit_bind}}, [ $self->__total_bindtype => $offset + $rows ], [ $self->__offset_bindtype => $offset + 1 ];
@@ -331,10 +331,11 @@ sub _prep_for_skimming_limit {
     if ($sq_attrs->{order_by_requested}) {
       $self->throw_exception (
         'Unable to safely perform "skimming type" limit with supplied unstable order criteria'
-      ) unless $rs_attrs->{_rsroot_rsrc}->schema->storage->_order_by_is_stable(
+      ) unless ($rs_attrs->{_rsroot_rsrc}->schema->storage->_order_by_is_stable(
         $rs_attrs->{from},
-        $requested_order
-      );
+        $requested_order,
+        $rs_attrs->{where},
+      ));
 
       $inner_order = $requested_order;
     }
diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm
index 3f3662b..ec6a32f 100644
--- a/lib/DBIx/Class/Storage/DBIHacks.pm
+++ b/lib/DBIx/Class/Storage/DBIHacks.pm
@@ -681,11 +681,12 @@ sub _extract_order_criteria {
 }
 
 sub _order_by_is_stable {
-  my ($self, $ident, $order_by) = @_;
+  my ($self, $ident, $order_by, $where) = @_;
 
-  my $colinfo = $self->_resolve_column_info(
-    $ident, [ map { $_->[0] } $self->_extract_order_criteria($order_by) ]
-  );
+  my $colinfo = $self->_resolve_column_info($ident, [
+    (map { $_->[0] } $self->_extract_order_criteria($order_by)),
+    $where ? @{$self->_extract_fixed_condition_columns($where)} :(),
+  ]);
 
   return undef unless keys %$colinfo;
 
@@ -700,4 +701,41 @@ sub _order_by_is_stable {
   return undef;
 }
 
+# returns an arrayref of column names which *definitely* have som
+# sort of non-nullable equality requested in the given condition
+# specification. This is used to figure out if a resultset is
+# constrained to a column which is part of a unique constraint,
+# which in turn allows us to better predict how ordering will behave
+# etc.
+#
+# this is a rudimentary, incomplete, and error-prone extractor
+# however this is OK - it is conservative, and if we can not find
+# something that is in fact there - the stack will recover gracefully
+# Also - DQ and the mst it rode in on will save us all RSN!!!
+sub _extract_fixed_condition_columns {
+  my ($self, $where, $nested) = @_;
+
+  return unless ref $where eq 'HASH';
+
+  my @cols;
+  for my $lhs (keys %$where) {
+    if ($lhs =~ /^\-and$/i) {
+      push @cols, ref $where->{$lhs} eq 'ARRAY'
+        ? ( map { $self->_extract_fixed_condition_columns($_, 1) } @{$where->{$lhs}} )
+        : $self->_extract_fixed_condition_columns($where->{$lhs}, 1)
+      ;
+    }
+    elsif ($lhs !~ /^\-/) {
+      my $val = $where->{$lhs};
+
+      push @cols, $lhs if (defined $val and (
+        ! ref $val
+          or
+        (ref $val eq 'HASH' and keys %$val == 1 and defined $val->{'='})
+      ));
+    }
+  }
+  return $nested ? @cols : \@cols;
+}
+
 1;