From: Peter Rabbitson <ribasushi@cpan.org>
Date: Thu, 22 Mar 2012 07:44:33 +0000 (+0100)
Subject: Fix pessimization of offset-less Oracle limits, introduced in 6a6394f1
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=cccd1876f48654bb612e8f3f9b16d786321340d2;p=dbsrgits%2FDBIx-Class-Historic.git

Fix pessimization of offset-less Oracle limits, introduced in 6a6394f1

When there is only one RowNum operator, stability of the order is not relevant
---

diff --git a/Changes b/Changes
index 79870ff..6a26965 100644
--- a/Changes
+++ b/Changes
@@ -27,6 +27,8 @@ Revision history for DBIx::Class
         - Fix leakage of $schema on in-memory new_related() calls
         - Fix more cases of $schema leakage in SQLT::Parser::DBIC
         - Fix leakage of $storage in ::Storage::DBI::Oracle
+        - Fix pessimization of Oracle RowNum limit dialect query when no
+          offset has been specified
         - Remove useless vestigial pessimization in Ordered.pm for cases 
           when the position column is part of a unique constraint
         - Fix dbicadmin to no longer ignore the documented 'config' option
diff --git a/lib/DBIx/Class/SQLMaker/LimitDialects.pm b/lib/DBIx/Class/SQLMaker/LimitDialects.pm
index ac145c3..55a44e6 100644
--- a/lib/DBIx/Class/SQLMaker/LimitDialects.pm
+++ b/lib/DBIx/Class/SQLMaker/LimitDialects.pm
@@ -249,6 +249,18 @@ sub _RowNum {
   my $idx_name = $self->_quote ('rownum__index');
   my $order_group_having = $self->_parse_rs_attrs($rs_attrs);
 
+
+  # if no offset (e.g. first page) - we can skip one of the subqueries
+  if (! $offset) {
+    push @{$self->{limit_bind}}, [ $self->__rows_bindtype => $rows ];
+
+    return <<EOS;
+SELECT $outsel FROM (
+  SELECT $insel ${stripped_sql}${order_group_having}
+) $qalias WHERE ROWNUM <= ?
+EOS
+  }
+
   #
   # There are two ways to limit in Oracle, one vastly faster than the other
   # on large resultsets: https://decipherinfosys.wordpress.com/2007/08/09/paging-and-countstopkey-optimization/
@@ -267,27 +279,15 @@ sub _RowNum {
       $rs_attrs->{from}, $rs_attrs->{order_by}
     )
   ) {
-    # if offset is 0 (first page) the we can skip a subquery
-    if (! $offset) {
-      push @{$self->{limit_bind}}, [ $self->__rows_bindtype => $rows ];
+    push @{$self->{limit_bind}}, [ $self->__total_bindtype => $offset + $rows ], [ $self->__offset_bindtype => $offset + 1 ];
 
-      return <<EOS;
-SELECT $outsel FROM (
-  SELECT $insel ${stripped_sql}${order_group_having}
-) $qalias WHERE ROWNUM <= ?
-EOS
-    }
-    else {
-      push @{$self->{limit_bind}}, [ $self->__total_bindtype => $offset + $rows ], [ $self->__offset_bindtype => $offset + 1 ];
-
-      return <<EOS;
+    return <<EOS;
 SELECT $outsel FROM (
   SELECT $outsel, ROWNUM $idx_name FROM (
     SELECT $insel ${stripped_sql}${order_group_having}
   ) $qalias WHERE ROWNUM <= ?
 ) $qalias WHERE $idx_name >= ?
 EOS
-    }
   }
   else {
     push @{$self->{limit_bind}}, [ $self->__offset_bindtype => $offset + 1 ], [ $self->__total_bindtype => $offset + $rows ];
diff --git a/t/73oracle_hq.t b/t/73oracle_hq.t
index aa5ad21..538fdf8 100644
--- a/t/73oracle_hq.t
+++ b/t/73oracle_hq.t
@@ -352,22 +352,18 @@ do_creates($dbh);
           FROM (
             SELECT artistid
               FROM (
-                SELECT artistid, ROWNUM rownum__index
-                  FROM (
-                    SELECT me.artistid
-                      FROM artist me
-                    START WITH name = ?
-                    CONNECT BY parentid = PRIOR artistid
-                  ) me
+                SELECT me.artistid
+                  FROM artist me
+                START WITH name = ?
+                CONNECT BY parentid = PRIOR artistid
               ) me
-            WHERE rownum__index BETWEEN ? AND ?
+            WHERE ROWNUM <= ?
           ) me
       )',
       [
         [ { 'sqlt_datatype' => 'varchar', 'dbic_colname' => 'name', 'sqlt_size' => 100 }
             => 'root'],
-        [ $ROWS => 1 ],
-        [ $TOTAL => 2 ],
+        [ $ROWS => 2 ],
       ],
     );
 
diff --git a/t/sqlmaker/limit_dialects/rownum.t b/t/sqlmaker/limit_dialects/rownum.t
index 522b4d4..8eefd17 100644
--- a/t/sqlmaker/limit_dialects/rownum.t
+++ b/t/sqlmaker/limit_dialects/rownum.t
@@ -8,9 +8,10 @@ use DBICTest;
 use DBIC::SqlMakerTest;
 use DBIx::Class::SQLMaker::LimitDialects;
 
-my ($TOTAL, $OFFSET) = (
+my ($TOTAL, $OFFSET, $ROWS) = (
    DBIx::Class::SQLMaker::LimitDialects->__total_bindtype,
    DBIx::Class::SQLMaker::LimitDialects->__offset_bindtype,
+   DBIx::Class::SQLMaker::LimitDialects->__rows_bindtype,
 );
 
 my $s = DBICTest->init_schema (no_deploy => 1, );
@@ -244,20 +245,17 @@ is_same_sql_bind(
   $rs_selectas_rel->as_query,
   '(
     SELECT id, owner FROM (
-      SELECT id, owner, ROWNUM rownum__index FROM (
-        SELECT me.id, me.owner  FROM books me WHERE ( ( (EXISTS (SELECT COUNT( * ) FROM owners owner WHERE ( books.owner = owner.id ))) AND source = ? ) )
-      ) me
-    ) me WHERE rownum__index BETWEEN ? AND ?
+      SELECT me.id, me.owner  FROM books me WHERE ( ( (EXISTS (SELECT COUNT( * ) FROM owners owner WHERE ( books.owner = owner.id ))) AND source = ? ) )
+    ) me
+    WHERE ROWNUM <= ?
   )',
   [
     [ { sqlt_datatype => 'varchar', sqlt_size => 100, dbic_colname => 'source' } => 'Library' ],
-    [ $OFFSET => 1 ],
-    [ $TOTAL => 1 ],
+    [ $ROWS => 1 ],
   ],
   'Pagination with sub-query in WHERE works'
 );
 
 }
 
-
 done_testing;