Fix removal of invalid ordering from select statements

Author: Aidan Haran

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
 
 #### 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/aido.rb

@@ -0,0 +1,40 @@
+
+require "cases/helper_sqlserver"
+require "models/developer"
+require "models/computer"
+require "models/project"
+require "models/company"
+require "models/ship"
+require "models/pirate"
+require "models/car"
+require "models/bulb"
+require "models/author"
+require "models/image"
+require "models/post"
+require "models/drink_designer"
+require "models/chef"
+require "models/department"
+require "models/club"
+require "models/membership"
+
+class AidoTestSQLServer < ActiveRecord::TestCase
+  fixtures :accounts, :companies, :developers, :projects, :developers_projects,
+           :ships, :pirates, :authors, :author_addresses, :books, :memberships, :clubs
+
+  def setup
+    Account.destroyed_account_ids.clear
+  end
+
+  def test_dependence_with_missing_association
+
+    # byebug
+
+    Account.destroy_all
+    firm = Firm.find(1)
+    assert_nil firm.account
+
+    $DEBUG = true
+
+    firm.destroy
+  end
+end

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