Fix removal of invalid ordering from select statements (#890)

Co-authored-by: Aidan Haran <aharan@fusioneer.com>
Co-authored-by: Wanderson Policarpo <wpolicarpo@gmail.com>

Author: 3 people

CHANGELOG.md

@@ -11,6 +11,7 @@
 - [#880](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/880) Handle any default column class when deduplicating
 - [#861](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/861) Fix Rails 6.1 database config
 - [#885](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/885) Fix the quoting of ActiveModel attributes
+- [#890](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/890) Fix removal of invalid ordering from select statements
 - [#881](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/881) Dump column collation to schema.rb and allow collation changes using column_change
 
 #### Changed

lib/arel/visitors/sqlserver.rb

@@ -184,11 +184,11 @@ def visit_Arel_Nodes_OuterJoin(o, collector)
         end
       end
 
-      def collect_in_clause(left, right, collector)
-        if Array === right
-          right.each { |node| remove_invalid_ordering_from_select_statement(node) }
+      def visit_Arel_Nodes_In(o, collector)
+        if Array === o.right
+          o.right.each { |node| remove_invalid_ordering_from_select_statement(node) }
         else
-          remove_invalid_ordering_from_select_statement(right)
+          remove_invalid_ordering_from_select_statement(o.right)
         end
 
         super

test/cases/in_clause_test_sqlserver.rb

@@ -33,4 +33,31 @@ class InClauseTestSQLServer < ActiveRecord::TestCase
     assert_includes posts.to_sql, "ORDER BY [authors].[name]"
     assert_equal 8, posts.length
   end
+
+  it "removes ordering from 'not' subqueries" do
+    authors_subquery = Author.where.not(name: ["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 5, posts.length
+  end
+
+  it "does not remove ordering from 'not' subquery that includes a limit" do
+    authors_subquery = Author.where.not(name: ["Ronan", "Mary", "Bob"]).order(:name).limit(2)
+    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 5, posts.length
+  end
+
+  it "does not remove ordering from 'not' subquery that includes an offset" do
+    authors_subquery = Author.where.not(name: ["David", "Ronan", "Cian"]).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 3, posts.length
+  end
 end