build the ast rather than passing around strings

1 files changed, 10 insertions, 12 deletions

diff --git a/lib/arel/visitors/mssql.rb b/lib/arel/visitors/mssql.rb
index c7ac56b43e..cc5e74af1f 100644
--- a/lib/arel/visitors/mssql.rb
+++ b/lib/arel/visitors/mssql.rb
@@ -2,10 +2,10 @@ module Arel
   module Visitors
     class MSSQL < Arel::Visitors::ToSql
       class RowNumber
-        attr_reader :expr
+        attr_reader :children
 
         def initialize node
-          @expr = node
+          @children = node
         end
       end
 
@@ -19,7 +19,7 @@ module Arel
       end
 
       def visit_Arel_Visitors_MSSQL_RowNumber o
-        "ROW_NUMBER() OVER (ORDER BY #{o.expr}) as _row_num"
+        "ROW_NUMBER() OVER (ORDER BY #{o.children.map { |x| visit x }.join ', '}) as _row_num"
       end
 
       def visit_Arel_Nodes_SelectStatement o
@@ -29,12 +29,12 @@ module Arel
 
         is_select_count = false
         sql = o.cores.map { |x|
-          core_order_by = determine_order_by(o.orders, x)
+          core_order_by = row_num_literal determine_order_by(o.orders, x)
           if select_count? x
-            x.projections = [row_num_literal(core_order_by)]
+            x.projections = [core_order_by]
             is_select_count = true
           else
-            x.projections << row_num_literal(core_order_by)
+            x.projections << core_order_by
           end
 
           visit_Arel_Nodes_SelectCore x
@@ -58,13 +58,11 @@ module Arel
 
       def determine_order_by orders, x
         if orders.any?
-          "#{orders.map { |x| visit x }.join(', ')}"
+          orders
+        elsif x.groups.any?
+          x.groups
         else
-          if x.groups.any?
-            "#{x.groups.map { |g| visit g }.join ', ' }"
-          else
-            "#{find_left_table_pk(x.froms)}"
-          end
+          [Arel.sql(find_left_table_pk(x.froms).to_s)]
         end
       end