Merge pull request rails#49570 from seanpdoyle/excluding-select-primary-key

Optimize `ActiveRecord::QueryMethods.excluding`

Author: jonathanhefner

activerecord/lib/active_record/relation/query_methods.rb

@@ -1453,6 +1453,9 @@ def uniq!(name)
     #   Post.excluding(post_one, post_two)
     #   # SELECT "posts".* FROM "posts" WHERE "posts"."id" NOT IN (1, 2)
     #
+    #   Post.excluding(Post.drafts)
+    #   # SELECT "posts".* FROM "posts" WHERE "posts"."id" NOT IN (3, 4, 5)
+    #
     # This can also be called on associations. As with the above example, either
     # a single record of collection thereof may be specified:
     #
@@ -1468,14 +1471,15 @@ def uniq!(name)
     # is passed in) are not instances of the same model that the relation is
     # scoping.
     def excluding(*records)
+      relations = records.extract! { |element| element.is_a?(Relation) }
       records.flatten!(1)
       records.compact!
 
-      unless records.all?(klass)
+      unless records.all?(klass) && relations.all? { |relation| relation.klass == klass }
         raise ArgumentError, "You must only pass a single or collection of #{klass.name} objects to ##{__callee__}."
       end
 
-      spawn.excluding!(records)
+      spawn.excluding!(records + relations.flat_map(&:ids))
     end
     alias :without :excluding
 

activerecord/test/cases/excluding_test.rb

@@ -22,6 +22,37 @@ def test_result_set_does_not_include_collection_of_excluded_records
     assert_not_includes relation, posts(:thinking)
   end
 
+  def test_result_set_does_not_include_collection_of_excluded_records_from_a_query
+    query = Post.where(id: @post)
+
+    assert_sql(/SELECT #{Regexp.escape Post.connection.quote_table_name("posts.id")} FROM/) do
+      records = Post.excluding(query).to_a
+
+      assert_not_includes records, @post
+    end
+  end
+
+  def test_result_set_does_not_include_collection_of_excluded_records_from_a_loaded_query
+    query = Post.where(id: @post).load
+
+    records = assert_queries 1 do
+      Post.excluding(query).to_a
+    end
+
+    assert_not_includes records, @post
+  end
+
+  def test_result_set_does_not_include_collection_of_excluded_records_and_queries
+    thinking = posts(:thinking)
+
+    records = assert_queries 2 do
+      Post.excluding(@post, Post.where(id: thinking)).to_a
+    end
+
+    assert_not_includes records, @post
+    assert_not_includes records, thinking
+  end
+
   def test_result_set_through_association_does_not_include_single_excluded_record
     comment_greetings, comment_more_greetings = comments(:greetings, :more_greetings)
 
@@ -38,6 +69,30 @@ def test_result_set_through_association_does_not_include_collection_of_excluded_
     assert_not_includes relation, comment_more_greetings
   end
 
+  def test_result_set_through_association_does_not_include_collection_of_excluded_records_from_a_relation
+    relation = @post.comments
+
+    assert_sql(/SELECT #{Regexp.escape Comment.connection.quote_table_name("comments.id")} FROM/) do
+      records = Comment.excluding(relation).to_a
+
+      assert_not_empty records
+      assert_not_empty @post.comments
+      assert_empty records.intersection(@post.comments.to_a)
+    end
+  end
+
+  def test_result_set_through_association_does_not_include_collection_of_excluded_records_from_a_loaded_relation
+    relation = @post.comments.load
+
+    records = assert_queries 1 do
+      Comment.excluding(relation).to_a
+    end
+
+    assert_not_empty records
+    assert_not_empty @post.comments
+    assert_empty records.intersection(@post.comments.to_a)
+  end
+
   def test_does_not_exclude_records_when_no_arguments
     assert_no_excludes Post.excluding
     assert_no_excludes Post.excluding(nil)