Fix Top tests, make extra col selector order consistent

diff --git a/lib/DBIx/Class/SQLAHacks.pm b/lib/DBIx/Class/SQLAHacks.pm
index f3fa030..71b89ff 100644 (file)
--- a/lib/DBIx/Class/SQLAHacks.pm
+++ b/lib/DBIx/Class/SQLAHacks.pm
@@ -132,11 +132,7 @@ sub _subqueried_limit_attrs {
 
   my %extra_order_sel;
   if ($scan_order) {
-    for my $chunk ($self->_order_by_chunks (
-      ref $rs_attrs->{order_by} eq 'ARRAY'
-        ? @{$rs_attrs->{order_by}}
-        : $rs_attrs->{order_by}
-    )) {
+    for my $chunk ($self->_order_by_chunks ($rs_attrs->{order_by})) {
       # order with bind
       $chunk = $chunk->[0] if (ref $chunk) eq 'ARRAY';
       $chunk =~ s/\s+ (?: ASC|DESC ) \s* $//ix;
@@ -178,7 +174,10 @@ sub _RowNumberOver {
   # this is the order supplement magic
   my $mid_sel = $out_sel;
   if ($extra_order_sel) {
-    for my $extra_col (keys %$extra_order_sel) {
+    for my $extra_col (sort
+      { $extra_order_sel->{$a} cmp $extra_order_sel->{$b} }
+      keys %$extra_order_sel
+    ) {
       $in_sel .= sprintf (', %s AS %s',
         $extra_col,
         $extra_order_sel->{$extra_col},
@@ -320,19 +319,23 @@ sub _Top {
     my @out_chunks;
     for my $ch ($self->_order_by_chunks ($inner_order)) {
       $ch = $ch->[0] if ref $ch eq 'ARRAY';
+
       $ch =~ s/\s+ ( ASC|DESC ) \s* $//ix;
       my $dir = uc ($1||'ASC');
 
       push @out_chunks, \join (' ', $ch, $dir eq 'ASC' ? 'DESC' : 'ASC' );
     }
 
-    $order_by_reversed = $self->_order_by (@out_chunks);
+    $order_by_reversed = $self->_order_by (\@out_chunks);
   }
 
   # this is the order supplement magic
   my $mid_sel = $out_sel;
   if ($extra_order_sel) {
-    for my $extra_col (keys %$extra_order_sel) {
+    for my $extra_col (sort
+      { $extra_order_sel->{$a} cmp $extra_order_sel->{$b} }
+      keys %$extra_order_sel
+    ) {
       $in_sel .= sprintf (', %s AS %s',
         $extra_col,
         $extra_order_sel->{$extra_col},
@@ -374,18 +377,11 @@ sub _Top {
 
   $sql = sprintf ('SELECT TOP %d %s FROM ( %s ) %s %s',
     $rows,
-    $mid_sel,
-    $sql,
-    $quoted_rs_alias,
-    $order_by_requested,
-  ) if $order_by_requested;
-
-  $sql = sprintf ('SELECT TOP %d %s FROM ( %s ) %s',
-    $rows,
     $out_sel,
     $sql,
     $quoted_rs_alias,
-  ) if ($mid_sel ne $out_sel);
+    $order_by_requested,
+  ) if ($order_by_requested || ($mid_sel ne $out_sel));
 
   return $sql;
 }

diff --git a/t/sqlahacks/limit_dialects/toplimit.t b/t/sqlahacks/limit_dialects/toplimit.t
index 1ad1cda..c856aa9 100644 (file)
--- a/t/sqlahacks/limit_dialects/toplimit.t
+++ b/t/sqlahacks/limit_dialects/toplimit.t
@@ -11,143 +11,170 @@ my $schema = DBICTest->init_schema;
 # Trick the sqlite DB to use Top limit emulation
 # We could test all of this via $sq->$op directly,
 # but some conditions need a $rsrc
+delete $schema->storage->_sql_maker->{_cached_syntax};
 $schema->storage->_sql_maker->limit_dialect ('Top');
 
-my $rs = $schema->resultset ('BooksInLibrary')->search ({}, { prefetch => 'owner', rows => 1, offset => 3 });
-
-sub default_test_order {
-   my $order_by = shift;
-   is_same_sql_bind(
-      $rs->search ({}, {order_by => $order_by})->as_query,
-      "(SELECT
-        TOP 1 me__id, source, owner, title, price, owner__id, name FROM
-         (SELECT
-           TOP 4 me.id AS me__id, me.source, me.owner, me.title, me.price, owner.id AS owner__id, owner.name
-           FROM books me
-           JOIN owners owner ON
-           owner.id = me.owner
-           WHERE ( source = ? )
-           ORDER BY me__id ASC
-         ) me ORDER BY me__id DESC
-       )",
-    [ [ source => 'Library' ] ],
-  );
-}
-
-sub test_order {
-  my $args = shift;
-
-  my $req_order = $args->{order_req}
-    ? "ORDER BY $args->{order_req}"
-    : ''
-  ;
+my $books_45_and_owners = $schema->resultset ('BooksInLibrary')->search ({}, { prefetch => 'owner', rows => 2, offset => 3 });
 
+for my $null_order (
+  undef,
+  '',
+  {},
+  [],
+  [{}],
+) {
+  my $rs = $books_45_and_owners->search ({}, {order_by => $null_order });
   is_same_sql_bind(
-    $rs->search ({}, {order_by => $args->{order_by}})->as_query,
-    "(SELECT
-      me__id, source, owner, title, price, owner__id, name FROM
-      (SELECT
-        TOP 1 me__id, source, owner, title, price, owner__id, name FROM
-         (SELECT
-           TOP 4 me.id AS me__id, me.source, me.owner, me.title, me.price, owner.id AS owner__id, owner.name FROM
-           books me
-           JOIN owners owner ON owner.id = me.owner
-           WHERE ( source = ? )
-           ORDER BY $args->{order_inner}
-         ) me ORDER BY $args->{order_outer}
-      ) me $req_order
-    )",
+      $rs->as_query,
+      '(SELECT TOP 2
+            id, source, owner, title, price, owner__id, owner__name
+          FROM (
+            SELECT TOP 5
+                me.id, me.source, me.owner, me.title, me.price, owner.id AS owner__id, owner.name AS owner__name
+              FROM books me
+              JOIN owners owner ON owner.id = me.owner
+            WHERE ( source = ? )
+            ORDER BY me.id
+          ) me
+        ORDER BY me.id DESC
+       )',
     [ [ source => 'Library' ] ],
   );
 }
 
-my @tests = (
+
+for my $ord_set (
   {
     order_by => \'foo DESC',
-    order_req => 'foo DESC',
     order_inner => 'foo DESC',
-    order_outer => 'foo ASC'
+    order_outer => '__ORDER_BY_1 ASC',
+    order_req => '__ORDER_BY_1 DESC',
+    exselect_outer => '__ORDER_BY_1',
+    exselect_inner => 'foo AS __ORDER_BY_1',
   },
   {
     order_by => { -asc => 'foo'  },
-    order_req => 'foo ASC',
     order_inner => 'foo ASC',
-    order_outer => 'foo DESC',
+    order_outer => '__ORDER_BY_1 DESC',
+    order_req => '__ORDER_BY_1 ASC',
+    exselect_outer => '__ORDER_BY_1',
+    exselect_inner => 'foo AS __ORDER_BY_1',
   },
   {
-    order_by => 'foo',
-    order_req => 'foo',
-    order_inner => 'foo ASC',
-    order_outer => 'foo DESC',
+    order_by => { -desc => 'foo' },
+    order_inner => 'foo DESC',
+    order_outer => '__ORDER_BY_1 ASC',
+    order_req => '__ORDER_BY_1 DESC',
+    exselect_outer => '__ORDER_BY_1',
+    exselect_inner => 'foo AS __ORDER_BY_1',
   },
   {
-    order_by => [ qw{ foo bar}   ],
-    order_req => 'foo, bar',
-    order_inner => 'foo ASC, bar ASC',
-    order_outer => 'foo DESC, bar DESC',
+    order_by => 'foo',
+    order_inner => 'foo',
+    order_outer => '__ORDER_BY_1 DESC',
+    order_req => '__ORDER_BY_1',
+    exselect_outer => '__ORDER_BY_1',
+    exselect_inner => 'foo AS __ORDER_BY_1',
   },
   {
-    order_by => { -desc => 'foo' },
-    order_req => 'foo DESC',
-    order_inner => 'foo DESC',
-    order_outer => 'foo ASC',
+    order_by => [ qw{ foo me.owner}   ],
+    order_inner => 'foo, me.owner',
+    order_outer => '__ORDER_BY_1 DESC, me.owner DESC',
+    order_req => '__ORDER_BY_1, me.owner',
+    exselect_outer => '__ORDER_BY_1',
+    exselect_inner => 'foo AS __ORDER_BY_1',
   },
   {
     order_by => ['foo', { -desc => 'bar' } ],
-    order_req => 'foo, bar DESC',
-    order_inner => 'foo ASC, bar DESC',
-    order_outer => 'foo DESC, bar ASC',
+    order_inner => 'foo, bar DESC',
+    order_outer => '__ORDER_BY_1 DESC, __ORDER_BY_2 ASC',
+    order_req => '__ORDER_BY_1, __ORDER_BY_2 DESC',
+    exselect_outer => '__ORDER_BY_1, __ORDER_BY_2',
+    exselect_inner => 'foo AS __ORDER_BY_1, bar AS __ORDER_BY_2',
   },
   {
     order_by => { -asc => [qw{ foo bar }] },
-    order_req => 'foo ASC, bar ASC',
     order_inner => 'foo ASC, bar ASC',
-    order_outer => 'foo DESC, bar DESC',
+    order_outer => '__ORDER_BY_1 DESC, __ORDER_BY_2 DESC',
+    order_req => '__ORDER_BY_1 ASC, __ORDER_BY_2 ASC',
+    exselect_outer => '__ORDER_BY_1, __ORDER_BY_2',
+    exselect_inner => 'foo AS __ORDER_BY_1, bar AS __ORDER_BY_2',
   },
   {
     order_by => [
-      { -asc => 'foo' },
+      'foo',
       { -desc => [qw{bar}] },
-      { -asc  => [qw{hello sensors}]},
+      { -asc  => [qw{me.owner sensors}]},
     ],
-    order_req => 'foo ASC, bar DESC, hello ASC, sensors ASC',
-    order_inner => 'foo ASC, bar DESC, hello ASC, sensors ASC',
-    order_outer => 'foo DESC, bar ASC, hello DESC, sensors DESC',
+    order_inner => 'foo, bar DESC, me.owner ASC, sensors ASC',
+    order_outer => '__ORDER_BY_1 DESC, __ORDER_BY_2 ASC, me.owner DESC, __ORDER_BY_3 DESC',
+    order_req => '__ORDER_BY_1, __ORDER_BY_2 DESC, me.owner ASC, __ORDER_BY_3 ASC',
+    exselect_outer => '__ORDER_BY_1, __ORDER_BY_2, __ORDER_BY_3',
+    exselect_inner => 'foo AS __ORDER_BY_1, bar AS __ORDER_BY_2, sensors AS __ORDER_BY_3',
   },
-);
-
-my @default_tests = ( undef, '', {}, [] );
-
-test_order ($_) for @tests;
-default_test_order ($_) for @default_tests;
+) {
+  my $o_sel = $ord_set->{exselect_outer}
+    ? ', ' . $ord_set->{exselect_outer}
+    : ''
+  ;
+  my $i_sel = $ord_set->{exselect_inner}
+    ? ', ' . $ord_set->{exselect_inner}
+    : ''
+  ;
 
+  is_same_sql_bind(
+    $books_45_and_owners->search ({}, {order_by => $ord_set->{order_by}})->as_query,
+    "(SELECT TOP 2
+          id, source, owner, title, price, owner__id, owner__name
+        FROM (
+          SELECT TOP 2
+              id, source, owner, title, price, owner__id, owner__name$o_sel
+            FROM (
+              SELECT TOP 5
+                  me.id, me.source, me.owner, me.title, me.price, owner.id AS owner__id, owner.name AS owner__name$i_sel
+                FROM books me
+                JOIN owners owner ON owner.id = me.owner
+              WHERE ( source = ? )
+              ORDER BY $ord_set->{order_inner}
+            ) me
+          ORDER BY $ord_set->{order_outer}
+        ) me
+      ORDER BY $ord_set->{order_req}
+    )",
+    [ [ source => 'Library' ] ],
+  ) || die;
+}
 
+# with groupby
 is_same_sql_bind (
-  $rs->search ({}, { group_by => 'title', order_by => 'title' })->as_query,
-'(SELECT
-me.id, me.source, me.owner, me.title, me.price, owner.id, owner.name FROM
-   ( SELECT
-      id, source, owner, title, price FROM
-      ( SELECT
-         TOP 1 id, source, owner, title, price FROM
-         ( SELECT
-            TOP 4 me.id, me.source, me.owner, me.title, me.price FROM
-            books me  JOIN
-            owners owner ON owner.id = me.owner
-            WHERE ( source = ? )
-            GROUP BY title
-            ORDER BY title ASC
-         ) me
-         ORDER BY title DESC
+  $books_45_and_owners->search ({}, { group_by => 'title', order_by => 'title' })->as_query,
+  '(SELECT me.id, me.source, me.owner, me.title, me.price, owner.id, owner.name
+      FROM (
+        SELECT TOP 2 id, source, owner, title, price
+          FROM (
+            SELECT TOP 2
+                id, source, owner, title, price, __ORDER_BY_1
+              FROM (
+                SELECT TOP 5
+                    me.id, me.source, me.owner, me.title, me.price, title AS __ORDER_BY_1
+                  FROM books me
+                  JOIN owners owner ON owner.id = me.owner
+                WHERE ( source = ? )
+                GROUP BY title
+                ORDER BY title
+              ) me
+            ORDER BY __ORDER_BY_1 DESC
+          ) me
+        ORDER BY __ORDER_BY_1
       ) me
-      ORDER BY title
-   ) me  JOIN
-   owners owner ON owner.id = me.owner WHERE
-   ( source = ? )
-   ORDER BY title)' ,
+      JOIN owners owner ON owner.id = me.owner
+    WHERE ( source = ? )
+    ORDER BY title
+  )',
   [ [ source => 'Library' ], [ source => 'Library' ] ],
 );
 
+# test deprecated column mixing over join boundaries
 my $rs_selectas_top = $schema->resultset ('BooksInLibrary')->search ({}, {
   '+select' => ['owner.name'],
   '+as' => ['owner_name'],
@@ -158,11 +185,11 @@ my $rs_selectas_top = $schema->resultset ('BooksInLibrary')->search ({}, {
 is_same_sql_bind( $rs_selectas_top->search({})->as_query,
                   '(SELECT
                       TOP 1 me.id, me.source, me.owner, me.title, me.price,
-                      owner.name
+                      owner.name AS owner_name
                     FROM books me
                     JOIN owners owner ON owner.id = me.owner
                     WHERE ( source = ? )
-                    ORDER BY me.id ASC
+                    ORDER BY me.id
                    )',
                    [ [ 'source', 'Library' ] ],
                 );