Added tests and don't remove ordering if limit or offset

Author: aidanharan

lib/arel/visitors/sqlserver.rb

@@ -138,6 +138,12 @@ def visit_Arel_Nodes_SelectStatement_SQLServer_Lock collector, options = {}
       end
 
       def visit_Orders_And_Let_Fetch_Happen o, collector
+
+        # if $debugger
+        #   puts collector.value
+        #   binding.pry
+        # end
+
         make_Fetch_Possible_And_Deterministic o
         unless o.orders.empty?
           collector << " "
@@ -160,7 +166,11 @@ def visit_Make_Fetch_Happen o, collector
 
       def collect_in_clause(left, right, collector)
 
-        # binding.pry if $debugger
+        # if $debugger
+        #   puts collector.value
+        #   binding.pry
+        # end
+
         #
         # if $debugger
         #   binding.pry
@@ -172,16 +182,33 @@ def collect_in_clause(left, right, collector)
 
         if Array === right
           right.each do |node|
+            remove_invalid_ordering_from_in_clause(node)
 
-            if Arel::Nodes::SelectStatement === node
-              node.orders = []
-            end
+            # if Arel::Nodes::SelectStatement === node
+            #   unless node.offset || node.limit
+            #     node.orders = []
+            #   end
+            # end
           end
+        else
+          remove_invalid_ordering_from_in_clause(right)
+        # elsif Arel::Nodes::SelectStatement === right
+        #   right.orders = []
         end
 
         super
       end
 
+
+      def remove_invalid_ordering_from_in_clause(node)
+        return unless Arel::Nodes::SelectStatement === node
+
+        unless node.offset || node.limit
+          node.orders = []
+        end
+      end
+
+
       # SQLServer Helpers
 
       def node_value(node)

test/cases/in_clause_test_sqlserver.rb

@@ -0,0 +1,74 @@
+require 'cases/helper_sqlserver'
+require 'models/post'
+require 'models/author'
+
+class InClauseTestSQLServer < ActiveRecord::TestCase
+
+  fixtures :posts, :authors
+
+  # The ORDER BY clause is invalid in views, inline functions, derived tables, subqueries, and common table expressions, unless TOP, OFFSET or FOR XML is also specified.
+
+  it 'removes ordering from subqueries' do
+
+    ActiveSupport::Notifications.subscribe('sql.active_record') do |_name, _start, _finish, _id, payload|
+      puts payload[:sql]
+    end
+
+    # binding.pry
+
+    # $debugger = true
+
+    # authors_subquery = Author.where(name: ['David', 'Mary']).order(:id)
+    authors_subquery = Author.where(name: ['David', 'Mary', 'Bob']).order(:name)
+    posts = Post.where(author: authors_subquery)
+
+    assert_includes authors_subquery.to_sql, "ORDER BY [authors].[name]"
+    assert_not_includes posts.to_sql, "ORDER BY [authors].[name]"
+    assert_equal 10, posts.length
+
+
+  end
+
+  it 'does not remove ordering from subquery that includes a limit' do
+    ActiveSupport::Notifications.subscribe('sql.active_record') do |_name, _start, _finish, _id, payload|
+      puts payload[:sql]
+    end
+
+
+
+    # EXEC sp_executesql N'SELECT [posts].* FROM [posts] WHERE [posts].[author_id] IN (SELECT [authors].[id] FROM [authors]
+    # WHERE [authors].[name] IN (@0, @1, @2)  ORDER BY [authors].[name] ASC OFFSET 0 ROWS FETCH NEXT @3 ROWS ONLY)',
+    # N'@0 nvarchar(4000), @1 nvarchar(4000), @2 nvarchar(4000), @3 int', @0 = N'David', @1 = N'Mary', @2 = N'Bob', @3 = 2
+
+    authors_subquery = Author.where(name: ['David', 'Mary', 'Bob']).order(:name).limit(2)
+    posts = Post.where(author: authors_subquery)
+
+    assert_includes authors_subquery.to_sql, "ORDER BY [authors].[name]"
+
+    # $debugger = true
+
+    assert_includes posts.to_sql, "ORDER BY [authors].[name]"
+    assert_equal 7, posts.length
+  end
+
+  #
+  it 'does not remove ordering from subquery that includes an offset' do
+    ActiveSupport::Notifications.subscribe('sql.active_record') do |_name, _start, _finish, _id, payload|
+      puts payload[:sql]
+    end
+
+    # EXEC sp_executesql N'SELECT [posts].* FROM [posts] WHERE [posts].[author_id] IN (SELECT [authors].[id] FROM [authors]
+    # WHERE [authors].[name] IN (@0, @1, @2)  ORDER BY [authors].[name] ASC OFFSET @3 ROWS)',
+    # N'@0 nvarchar(4000), @1 nvarchar(4000), @2 nvarchar(4000), @3 int', @0 = N'David', @1 = N'Mary', @2 = N'Bob', @3 = 1
+
+
+    authors_subquery = Author.where(name: ['David', 'Mary', 'Bob']).order(:name).offset(1)
+    posts = Post.where(author: authors_subquery)
+
+    assert_includes authors_subquery.to_sql, "ORDER BY [authors].[name]"
+    assert_includes posts.to_sql, "ORDER BY [authors].[name]"
+    assert_equal 8, posts.length
+  end
+
+
+end