- limit/offset no longer uses ROW NUMBER OVER to limit rows,

      and instead uses subqueries in conjunction with a special
      Oracle optimization comment.  Allows LIMIT/OFFSET to work
      in conjunction with DISTINCT. [ticket:536]

1 files changed, 43 insertions, 15 deletions

diff --git a/lib/sqlalchemy/databases/oracle.py b/lib/sqlalchemy/databases/oracle.py
index 12b22f445..ed2cb3ef2 100644
--- a/lib/sqlalchemy/databases/oracle.py
+++ b/lib/sqlalchemy/databases/oracle.py
@@ -654,7 +654,7 @@ class OracleCompiler(compiler.DefaultCompiler):
 
     def visit_select(self, select, **kwargs):
         """Look for ``LIMIT`` and OFFSET in a select statement, and if
-        so tries to wrap it in a subquery with ``row_number()`` criterion.
+        so tries to wrap it in a subquery with ``rownum`` criterion.
         """
 
         if not getattr(select, '_oracle_visit', None):
@@ -671,26 +671,54 @@ class OracleCompiler(compiler.DefaultCompiler):
                     select._oracle_visit = True
 
             if select._limit is not None or select._offset is not None:
-                # to use ROW_NUMBER(), an ORDER BY is required.
-                orderby = self.process(select._order_by_clause)
-                if not orderby:
-                    orderby = list(select.oid_column.proxies)[0]
-                    orderby = self.process(orderby)
-
-                select = select.column(sql.literal_column("ROW_NUMBER() OVER (ORDER BY %s)" % orderby).label("ora_rn")).order_by(None)
+                # See http://www.oracle.com/technology/oramag/oracle/06-sep/o56asktom.html
+                #
+                # Generalized form of an Oracle pagination query:
+                #   select ... from (
+                #     select /*+ FIRST_ROWS(N) */ ...., rownum as ora_rn from (
+                #         select distinct ... where ... order by ...
+                #     ) where ROWNUM <= :limit+:offset
+                #   ) where ora_rn > :offset
+                # Outer select and "ROWNUM as ora_rn" can be dropped if limit=0
+
+                # TODO: use annotations instead of clone + attr set ?
+                select = select._generate()
                 select._oracle_visit = True
 
-                limitselect = sql.select([c for c in select.c if c.key!='ora_rn'])
+                # Wrap the middle select and add the hint
+                limitselect = sql.select([c for c in select.c])
+                if select._limit:
+                    limitselect = limitselect.prefix_with("/*+ FIRST_ROWS(%d) */" % select._limit)
+
                 limitselect._oracle_visit = True
                 limitselect._is_wrapper = True
 
-                if select._offset is not None:
-                    limitselect.append_whereclause("ora_rn>%d" % select._offset)
-                    if select._limit is not None:
-                        limitselect.append_whereclause("ora_rn<=%d" % (select._limit + select._offset))
+                # If needed, add the limiting clause
+                if select._limit is not None:
+                    max_row = select._limit
+                    if select._offset is not None:
+                        max_row += select._offset
+                    limitselect.append_whereclause(
+                            sql.literal_column("ROWNUM")<=max_row)
+ 
+                # If needed, add the ora_rn, and wrap again with offset.
+                if select._offset is None:
+                    select = limitselect
                 else:
-                    limitselect.append_whereclause("ora_rn<=%d" % select._limit)
-                select = limitselect
+                     limitselect = limitselect.column(
+                             sql.literal_column("ROWNUM").label("ora_rn"))
+                     limitselect._oracle_visit = True
+                     limitselect._is_wrapper = True
+ 
+                     offsetselect = sql.select(
+                             [c for c in limitselect.c if c.key!='ora_rn'])
+                     offsetselect._oracle_visit = True
+                     offsetselect._is_wrapper = True
+ 
+                     offsetselect.append_whereclause(
+                             sql.literal_column("ora_rn")>select._offset)
+ 
+                     select = offsetselect
 
         kwargs['iswrapper'] = getattr(select, '_is_wrapper', False)
         return compiler.DefaultCompiler.visit_select(self, select, **kwargs)