1 files changed, 34 insertions, 24 deletions

diff --git a/lib/arel/visitors/oracle.rb b/lib/arel/visitors/oracle.rb
index 1441a20dbc..91f6e0223e 100644
--- a/lib/arel/visitors/oracle.rb
+++ b/lib/arel/visitors/oracle.rb
@@ -3,12 +3,12 @@ module Arel
     class Oracle < Arel::Visitors::ToSql
       private
 
-      def visit_Arel_Nodes_SelectStatement o
+      def visit_Arel_Nodes_SelectStatement o, collector
         o = order_hacks(o)
 
         # if need to select first records without ORDER BY and GROUP BY and without DISTINCT
         # then can use simple ROWNUM in WHERE clause
-        if o.limit && o.orders.empty? && !o.offset && o.cores.first.projections.first !~ /^DISTINCT /
+        if o.limit && o.orders.empty? && o.cores.first.groups.empty? && !o.offset && o.cores.first.set_quantifier.class.to_s !~ /Distinct/
           o.cores.last.wheres.push Nodes::LessThanOrEqual.new(
             Nodes::SqlLiteral.new('ROWNUM'), o.limit.expr
           )
@@ -17,55 +17,65 @@ module Arel
 
         if o.limit && o.offset
           o        = o.dup
-          limit    = o.limit.expr.to_i
+          limit    = o.limit.expr.expr
           offset   = o.offset
           o.offset = nil
-          sql = super(o)
-          return <<-eosql
+          collector << "
               SELECT * FROM (
                 SELECT raw_sql_.*, rownum raw_rnum_
-                FROM (#{sql}) raw_sql_
+                FROM ("
+
+          collector = super(o, collector)
+          collector << ") raw_sql_
+                WHERE rownum <= #{offset.expr.to_i + limit}
               )
-              WHERE raw_rnum_ between #{offset.expr.to_i + 1 } and #{offset.expr.to_i + limit}
-          eosql
+              WHERE "
+          return visit(offset, collector)
         end
 
         if o.limit
           o       = o.dup
           limit   = o.limit.expr
-          return "SELECT * FROM (#{super(o)}) WHERE ROWNUM <= #{visit limit}"
+          collector << "SELECT * FROM ("
+          collector = super(o, collector)
+          collector << ") WHERE ROWNUM <= "
+          return visit limit, collector
         end
 
         if o.offset
           o        = o.dup
           offset   = o.offset
           o.offset = nil
-          sql = super(o)
-          return <<-eosql
-              SELECT * FROM (
+          collector << "SELECT * FROM (
                 SELECT raw_sql_.*, rownum raw_rnum_
-                FROM (#{sql}) raw_sql_
+                FROM ("
+          collector = super(o, collector)
+          collector << ") raw_sql_
               )
-              WHERE #{visit offset}
-          eosql
+              WHERE "
+          return visit offset, collector
         end
 
         super
       end
 
-      def visit_Arel_Nodes_Limit o
+      def visit_Arel_Nodes_Limit o, collector
+        collector
       end
 
-      def visit_Arel_Nodes_Offset o
-        "raw_rnum_ > #{visit o.expr}"
+      def visit_Arel_Nodes_Offset o, collector
+        collector << "raw_rnum_ > "
+        visit o.expr, collector
       end
 
-      def visit_Arel_Nodes_Except o
-        "( #{visit o.left} MINUS #{visit o.right} )"
+      def visit_Arel_Nodes_Except o, collector
+        collector << "( "
+        collector = infix_value o, collector, " MINUS "
+        collector << " )"
       end
 
-      def visit_Arel_Nodes_UpdateStatement o
-        # Oracle does not allow ORDER BY/LIMIT in UPDATEs. 
+      def visit_Arel_Nodes_UpdateStatement o, collector
+        # Oracle does not allow ORDER BY/LIMIT in UPDATEs.
         if o.orders.any? && o.limit.nil?
           # However, there is no harm in silently eating the ORDER BY clause if no LIMIT has been provided,
           # otherwise let the user deal with the error
@@ -82,7 +92,7 @@ module Arel
         return o if o.orders.empty?
         return o unless o.cores.any? do |core|
           core.projections.any? do |projection|
-            /DISTINCT.*FIRST_VALUE/ === projection
+            /FIRST_VALUE/ === projection
           end
         end
         # Previous version with join and split broke ORDER BY clause
@@ -90,7 +100,7 @@ module Arel
         #
         # orders   = o.orders.map { |x| visit x }.join(', ').split(',')
         orders   = o.orders.map do |x|
-          string = visit x
+          string = visit(x, Arel::Collectors::SQLString.new).value
           if string.include?(',')
             split_order_string(string)
           else