mssql visitor is working

2 files changed, 33 insertions, 24 deletions

diff --git a/lib/arel/visitors/mssql.rb b/lib/arel/visitors/mssql.rb
index 4085af9069..0e5b75ec59 100644
--- a/lib/arel/visitors/mssql.rb
+++ b/lib/arel/visitors/mssql.rb
@@ -1,13 +1,7 @@
 module Arel
   module Visitors
     class MSSQL < Arel::Visitors::ToSql
-      class RowNumber
-        attr_reader :children
-
-        def initialize node
-          @children = node
-        end
-      end
+      RowNumber = Struct.new :children
 
       private
 
@@ -18,13 +12,14 @@ module Arel
         ""
       end
 
-      def visit_Arel_Visitors_MSSQL_RowNumber o
-        "ROW_NUMBER() OVER (ORDER BY #{o.children.map { |x| visit x }.join ', '}) as _row_num"
+      def visit_Arel_Visitors_MSSQL_RowNumber o, collector
+        collector << "ROW_NUMBER() OVER (ORDER BY "
+        inject_join(o.children, collector, ', ') << ") as _row_num"
       end
 
-      def visit_Arel_Nodes_SelectStatement o
+      def visit_Arel_Nodes_SelectStatement o, collector
         if !o.limit && !o.offset
-          return super o
+          return super
         end
 
         is_select_count = false
@@ -38,12 +33,22 @@ module Arel
           end
         }
 
-        sql = o.cores.map { |x| visit_Arel_Nodes_SelectCore x }.join
+        if is_select_count
+          # fixme count distinct wouldn't work with limit or offset
+          collector << "SELECT COUNT(1) as count_id FROM ("
+        end
+
+        collector << "SELECT _t.* FROM ("
+        collector = o.cores.inject(collector) { |c,x|
+          visit_Arel_Nodes_SelectCore x, c
+        }
+        collector << ") as _t WHERE #{get_offset_limit_clause(o)}"
 
-        sql = "SELECT _t.* FROM (#{sql}) as _t WHERE #{get_offset_limit_clause(o)}"
-        # fixme count distinct wouldn't work with limit or offset
-        sql = "SELECT COUNT(1) as count_id FROM (#{sql}) AS subquery" if is_select_count
-        sql
+        if is_select_count
+          collector << ") AS subquery"
+        else
+          collector
+        end
       end
 
       def get_offset_limit_clause o
diff --git a/test/visitors/test_mssql.rb b/test/visitors/test_mssql.rb
index d62d4b8d1f..a3efcb8b27 100644
--- a/test/visitors/test_mssql.rb
+++ b/test/visitors/test_mssql.rb
@@ -8,9 +8,13 @@ module Arel
         @table = Arel::Table.new "users"
       end
 
+      def compile node
+        @visitor.accept(node, Collectors::SQLString.new).value
+      end
+
       it 'should not modify query if no offset or limit' do
         stmt = Nodes::SelectStatement.new
-        sql = @visitor.accept(stmt)
+        sql = compile(stmt)
         sql.must_be_like "SELECT"
       end
 
@@ -18,15 +22,15 @@ module Arel
         stmt = Nodes::SelectStatement.new
         stmt.cores.first.from = @table
         stmt.limit = Nodes::Limit.new(10)
-        sql = @visitor.accept(stmt)
-        sql.must_be_like "SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY \"users\".\"id\") as _row_num FROM \"users\" ) as _t WHERE _row_num BETWEEN 1 AND 10"
+        sql = compile(stmt)
+        sql.must_be_like "SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY \"users\".\"id\") as _row_num FROM \"users\") as _t WHERE _row_num BETWEEN 1 AND 10"
       end
 
       it 'should go over query ORDER BY if .order()' do
         stmt = Nodes::SelectStatement.new
         stmt.limit = Nodes::Limit.new(10)
         stmt.orders << Nodes::SqlLiteral.new('order_by')
-        sql = @visitor.accept(stmt)
+        sql = compile(stmt)
         sql.must_be_like "SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY order_by) as _row_num) as _t WHERE _row_num BETWEEN 1 AND 10"
       end
 
@@ -34,7 +38,7 @@ module Arel
         stmt = Nodes::SelectStatement.new
         stmt.cores.first.groups << Nodes::SqlLiteral.new('group_by')
         stmt.limit = Nodes::Limit.new(10)
-        sql = @visitor.accept(stmt)
+        sql = compile(stmt)
         sql.must_be_like "SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY group_by) as _row_num GROUP BY group_by) as _t WHERE _row_num BETWEEN 1 AND 10"
       end
 
@@ -42,14 +46,14 @@ module Arel
         stmt = Nodes::SelectStatement.new
         stmt.limit = Nodes::Limit.new(10)
         stmt.offset = Nodes::Offset.new(20)
-        sql = @visitor.accept(stmt)
+        sql = compile(stmt)
         sql.must_be_like "SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY ) as _row_num) as _t WHERE _row_num BETWEEN 21 AND 30"
       end
 
       it 'should use >= if only .offset' do
         stmt = Nodes::SelectStatement.new
         stmt.offset = Nodes::Offset.new(20)
-        sql = @visitor.accept(stmt)
+        sql = compile(stmt)
         sql.must_be_like "SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY ) as _row_num) as _t WHERE _row_num >= 21"
       end
 
@@ -57,7 +61,7 @@ module Arel
         stmt = Nodes::SelectStatement.new
         stmt.limit = Nodes::Limit.new(10)
         stmt.cores.first.projections << Nodes::Count.new('*')
-        sql = @visitor.accept(stmt)
+        sql = compile(stmt)
         sql.must_be_like "SELECT COUNT(1) as count_id FROM (SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY ) as _row_num) as _t WHERE _row_num BETWEEN 1 AND 10) AS subquery"
       end