Merge pull request #12873 from martinemde/fix-where-assoc-nil

Better support for AR:B.where(belongs_to_assoc: val) which previously generated invalid SQL

3 files changed, 104 insertions, 3 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 712e27d590..d1db4db8d8 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,58 @@
+*   Better support for `where()` conditions that use a `belongs_to`
+    association name.
+
+    Using the name of an association in `where` previously worked only
+    if the value was a single `ActiveRecord::Base` object. e.g.
+
+        Post.where(author: Author.first)
+
+    Any other values, including `nil`, would cause invalid SQL to be
+    generated. This change supports arguments in the `where` query
+    conditions where the key is a `belongs_to` association name and the
+    value is `nil`, an `Array` of `ActiveRecord::Base` objects, or an
+    `ActiveRecord::Relation` object.
+
+        class Post < ActiveRecord::Base
+          belongs_to :author
+        end
+
+    `nil` value finds records where the association is not set:
+
+        Post.where(author: nil)
+        # SELECT "posts".* FROM "posts" WHERE "posts"."author_id" IS NULL
+
+    `Array` values find records where the association foreign key
+    matches the ids of the passed ActiveRecord models, resulting
+    in the same query as `Post.where(author_id: [1,2])`:
+
+        authors_array = [Author.find(1), Author.find(2)]
+        Post.where(author: authors_array)
+        # SELECT "posts".* FROM "posts" WHERE "posts"."author_id" IN (1, 2)
+
+    `ActiveRecord::Relation` values find records using the same
+    query as `Post.where(author_id: Author.where(last_name: "Emde"))`
+
+        Post.where(author: Author.where(last_name: "Emde"))
+        # SELECT "posts".* FROM "posts"
+        # WHERE "posts"."author_id" IN (
+        #   SELECT "authors"."id" FROM "authors"
+        #   WHERE "authors"."last_name" = 'Emde')
+
+    Polymorphic `belongs_to` associations will continue to be handled
+    appropriately, with the polymorphic `association_type` field added
+    to the query to match the base class of the value. This feature
+    previously only worked when the value was a single `ActveRecord::Base`.
+
+        class Post < ActiveRecord::Base
+          belongs_to :author, polymorphic: true
+        end
+
+        Post.where(author: Author.where(last_name: "Emde"))
+        # Generates a query similar to:
+        Post.where(author_id: Author.where(last_name: "Emde"), author_type: "Author")
+
+    *Martin Emde*
+
 *   Respect temporary option when dropping tables with MySQL.
 
     Normal DROP TABLE also works, but commits the transaction.
diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb
index c60cd27a83..1252af7635 100644
--- a/activerecord/lib/active_record/relation/predicate_builder.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder.rb
@@ -55,9 +55,9 @@ module ActiveRecord
       #
       # For polymorphic relationships, find the foreign key and type:
       # PriceEstimate.where(estimate_of: treasure)
-      if klass && value.is_a?(Base) && reflection = klass.reflect_on_association(column.to_sym)
-        if reflection.polymorphic?
-          queries << build(table[reflection.foreign_type], value.class.base_class)
+      if klass && reflection = klass.reflect_on_association(column.to_sym)
+        if reflection.polymorphic? && base_class = polymorphic_base_class_from_value(value)
+          queries << build(table[reflection.foreign_type], base_class)
         end
 
         column = reflection.foreign_key
@@ -67,6 +67,18 @@ module ActiveRecord
       queries
     end
 
+    def self.polymorphic_base_class_from_value(value)
+      case value
+      when Relation
+        value.klass.base_class
+      when Array
+        val = value.compact.first
+        val.class.base_class if val.is_a?(Base)
+      when Base
+        value.class.base_class
+      end
+    end
+
     def self.references(attributes)
       attributes.map do |key, value|
         if value.is_a?(Hash)
diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb
index 56e4605ccc..937f226b1d 100644
--- a/activerecord/test/cases/relation/where_test.rb
+++ b/activerecord/test/cases/relation/where_test.rb
@@ -35,6 +35,21 @@ module ActiveRecord
       assert_equal Post.where(author_id: 1).to_sql, Post.where(author: author).to_sql
     end
 
+    def test_belongs_to_nil_where
+      assert_equal Post.where(author_id: nil).to_sql, Post.where(author: nil).to_sql
+    end
+
+    def test_belongs_to_array_value_where
+      assert_equal Post.where(author_id: [1,2]).to_sql, Post.where(author: [1,2]).to_sql
+    end
+
+    def test_belongs_to_nested_relation_where
+      expected = Post.where(author_id: Author.where(id: [1,2])).to_sql
+      actual   = Post.where(author:    Author.where(id: [1,2])).to_sql
+
+      assert_equal expected, actual
+    end
+
     def test_belongs_to_nested_where
       parent = Comment.new
       parent.id = 1
@@ -55,6 +70,25 @@ module ActiveRecord
       assert_equal expected.to_sql, actual.to_sql
     end
 
+    def test_polymorphic_nested_array_where
+      treasure = Treasure.new
+      treasure.id = 1
+      hidden = HiddenTreasure.new
+      hidden.id = 2
+
+      expected = PriceEstimate.where(estimate_of_type: 'Treasure', estimate_of_id: [treasure, hidden])
+      actual   = PriceEstimate.where(estimate_of: [treasure, hidden])
+
+      assert_equal expected.to_sql, actual.to_sql
+    end
+
+    def test_polymorphic_nested_relation_where
+      expected = PriceEstimate.where(estimate_of_type: 'Treasure', estimate_of_id: Treasure.where(id: [1,2]))
+      actual   = PriceEstimate.where(estimate_of: Treasure.where(id: [1,2]))
+
+      assert_equal expected.to_sql, actual.to_sql
+    end
+
     def test_polymorphic_sti_shallow_where
       treasure = HiddenTreasure.new
       treasure.id = 1