From: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Date: Fri, 22 Mar 2013 22:52:14 +0000 (+0000)
Subject: Handle NULLS clauses when mangling ordering
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=refs%2Fheads%2Fwip%2Forder_by_nulls_dbic_portion;p=dbsrgits%2FDBIx-Class.git

Handle NULLS clauses when mangling ordering

TODO: Tests for GenericSubQ (needs SQL::Abstract::Tree support)
---

diff --git a/lib/DBIx/Class/SQLMaker.pm b/lib/DBIx/Class/SQLMaker.pm
index ea69e07..e7dd9f1 100644
--- a/lib/DBIx/Class/SQLMaker.pm
+++ b/lib/DBIx/Class/SQLMaker.pm
@@ -445,11 +445,12 @@ sub _split_order_chunk {
   my ($self, $chunk) = @_;
 
   # strip off sort modifiers, but always succeed, so $1 gets reset
-  $chunk =~ s/ (?: \s+ (ASC|DESC) )? \s* $//ix;
+  $chunk =~ s/ (?: \s+ (ASC|DESC) )? (?: \s+ NULLS \s+ (FIRST|LAST) )? \s* $//ix;
 
   return (
     $chunk,
     ( $1 and uc($1) eq 'DESC' ) ? 1 : 0,
+    $2 ? uc($2) : undef
   );
 }
 
diff --git a/lib/DBIx/Class/SQLMaker/LimitDialects.pm b/lib/DBIx/Class/SQLMaker/LimitDialects.pm
index 89e63e0..764b110 100644
--- a/lib/DBIx/Class/SQLMaker/LimitDialects.pm
+++ b/lib/DBIx/Class/SQLMaker/LimitDialects.pm
@@ -355,10 +355,15 @@ sub _prep_for_skimming_limit {
     for my $ch ($self->_order_by_chunks ($inner_order)) {
       $ch = $ch->[0] if ref $ch eq 'ARRAY';
 
-      ($ch, my $is_desc) = $self->_split_order_chunk($ch);
-
-      # !NOTE! outside chunks come in reverse order ( !$is_desc )
-      push @out_chunks, { ($is_desc ? '-asc' : '-desc') => \$ch };
+      ($ch, my ($is_desc, $nulls_pos) ) = $self->_split_order_chunk($ch);
+
+      # !NOTE! outside chunks come in reverse order ( !$is_desc, !$nulls_pos )
+      push @out_chunks, {
+        ($is_desc ? '-asc' : '-desc') => \$ch,
+        $nulls_pos ? (
+          -nulls => ($nulls_pos eq 'FIRST' ? 'LAST' : 'FIRST')
+        ) : (),
+      };
     }
 
     $sq_attrs->{order_by_middle} = $self->_order_by (\@out_chunks);
@@ -586,13 +591,16 @@ sub _GenericSubQ {
 
   for my $bit (@order_bits) {
 
-    ($bit, my $is_desc) = $self->_split_order_chunk($bit);
+    ($bit, my ($is_desc, $nulls_pos)) = $self->_split_order_chunk($bit);
 
     push @is_desc, $is_desc;
     push @unqualified_names, $usable_order_colinfo->{$bit}{-colname};
     push @qualified_names, $usable_order_colinfo->{$bit}{-fq_colname};
 
-    push @new_order_by, { ($is_desc ? '-desc' : '-asc') => $usable_order_colinfo->{$bit}{-fq_colname} };
+    push @new_order_by, {
+      ($is_desc ? '-desc' : '-asc') => $usable_order_colinfo->{$bit}{-fq_colname},
+      ($nulls_pos ? ( -nulls => lc $nulls_pos ) : ()),
+    };
   };
 
   my (@where_cond, @skip_colpair_stack);
diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm
index 26cc4c8..3224f8c 100644
--- a/lib/DBIx/Class/Storage/DBIHacks.pm
+++ b/lib/DBIx/Class/Storage/DBIHacks.pm
@@ -726,6 +726,7 @@ sub _group_over_selection {
         map { ref $_ eq 'ARRAY' ? $_ : [ $_ ] } $sql_maker->_order_by_chunks($attrs->{order_by})
       ];
 
+      # FIXME: MIN/MAX can't handle NULLS FIRST/LAST
       my ($chunk, $is_desc) = $sql_maker->_split_order_chunk($order_chunks->[$o_idx][0]);
 
       # we reached that far - wrap any part of the order_by that "responded"
diff --git a/t/sqlmaker/limit_dialects/fetch_first.t b/t/sqlmaker/limit_dialects/fetch_first.t
index 625a464..13adac2 100644
--- a/t/sqlmaker/limit_dialects/fetch_first.t
+++ b/t/sqlmaker/limit_dialects/fetch_first.t
@@ -124,6 +124,17 @@ for my $ord_set (
     order_outer => 'name DESC',
     order_req => 'name',
   },
+  {
+    order_by => [
+        { -asc => 'title', -nulls => 'first' },
+        { -desc => 'bar', -nulls => 'last' },
+    ],
+    order_inner => 'title ASC NULLS FIRST, bar DESC NULLS LAST',
+    order_outer => 'ORDER__BY__001 DESC NULLS LAST, ORDER__BY__002 ASC NULLS FIRST',
+    order_req => 'ORDER__BY__001 ASC NULLS FIRST, ORDER__BY__002 DESC NULLS LAST',
+    exselect_outer => 'ORDER__BY__001, ORDER__BY__002',
+    exselect_inner => 'title AS ORDER__BY__001, bar AS ORDER__BY__002',
+  },
 ) {
   my $o_sel = $ord_set->{exselect_outer}
     ? ', ' . $ord_set->{exselect_outer}
diff --git a/t/sqlmaker/limit_dialects/toplimit.t b/t/sqlmaker/limit_dialects/toplimit.t
index e1c40b8..580dee6 100644
--- a/t/sqlmaker/limit_dialects/toplimit.t
+++ b/t/sqlmaker/limit_dialects/toplimit.t
@@ -155,6 +155,17 @@ for my $ord_set (
     exselect_outer => 'ORDER__BY__001, ORDER__BY__002, ORDER__BY__003',
     exselect_inner => 'title AS ORDER__BY__001, bar AS ORDER__BY__002, sensors AS ORDER__BY__003',
   },
+  {
+    order_by => [
+        { -asc => 'title', -nulls => 'first' },
+        { -desc => 'bar', -nulls => 'last' },
+    ],
+    order_inner => 'title ASC NULLS FIRST, bar DESC NULLS LAST',
+    order_outer => 'ORDER__BY__001 DESC NULLS LAST, ORDER__BY__002 ASC NULLS FIRST',
+    order_req => 'ORDER__BY__001 ASC NULLS FIRST, ORDER__BY__002 DESC NULLS LAST',
+    exselect_outer => 'ORDER__BY__001, ORDER__BY__002',
+    exselect_inner => 'title AS ORDER__BY__001, bar AS ORDER__BY__002',
+  },
 ) {
   my $o_sel = $ord_set->{exselect_outer}
     ? ', ' . $ord_set->{exselect_outer}