Merge pull request #29950 from MaxLap/avoid_or_clause_duplicates

Avoid duplicate clauses when using #or

3 files changed, 66 insertions, 6 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 1ff7010c2f..aa581ed1e6 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   When using #or, extract the common conditions and put them before the OR condition.
+
+    *Maxime Handfield Lapointe*
+
 *   `Relation#or` now accepts two relations who have different values for
     `references` only, as `references` can be implicitly called by `where`.
 
diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb
index ef2bca9155..752bb38481 100644
--- a/activerecord/lib/active_record/relation/where_clause.rb
+++ b/activerecord/lib/active_record/relation/where_clause.rb
@@ -15,6 +15,12 @@ module ActiveRecord
         )
       end
 
+      def -(other)
+        WhereClause.new(
+          predicates - other.predicates,
+        )
+      end
+
       def merge(other)
         WhereClause.new(
           predicates_unreferenced_by(other) + other.predicates,
@@ -26,14 +32,17 @@ module ActiveRecord
       end
 
       def or(other)
-        if empty?
-          self
-        elsif other.empty?
-          other
+        left = self - other
+        common = self - left
+        right = other - common
+
+        if left.empty? || right.empty?
+          common
         else
-          WhereClause.new(
-            [ast.or(other.ast)],
+          or_clause = WhereClause.new(
+            [left.ast.or(right.ast)],
           )
+          common + or_clause
         end
       end
 
diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb
index f3a81f3c70..e5eb159d36 100644
--- a/activerecord/test/cases/relation/where_clause_test.rb
+++ b/activerecord/test/cases/relation/where_clause_test.rb
@@ -181,6 +181,53 @@ class ActiveRecord::Relation
       assert_equal WhereClause.empty, WhereClause.empty.or(where_clause)
     end
 
+    test "or places common conditions before the OR" do
+      a = WhereClause.new(
+        [table["id"].eq(bind_param(1)), table["name"].eq(bind_param("Sean"))],
+      )
+      b = WhereClause.new(
+        [table["id"].eq(bind_param(1)), table["hair_color"].eq(bind_param("black"))],
+      )
+
+      common = WhereClause.new(
+        [table["id"].eq(bind_param(1))],
+      )
+
+      or_clause = WhereClause.new([table["name"].eq(bind_param("Sean"))])
+        .or(WhereClause.new([table["hair_color"].eq(bind_param("black"))]))
+
+      assert_equal common + or_clause, a.or(b)
+    end
+
+    test "or can detect identical or as being a common condition" do
+      common_or = WhereClause.new([table["name"].eq(bind_param("Sean"))])
+        .or(WhereClause.new([table["hair_color"].eq(bind_param("black"))]))
+
+      a = common_or + WhereClause.new([table["id"].eq(bind_param(1))])
+      b = common_or + WhereClause.new([table["foo"].eq(bind_param("bar"))])
+
+      new_or = WhereClause.new([table["id"].eq(bind_param(1))])
+        .or(WhereClause.new([table["foo"].eq(bind_param("bar"))]))
+
+      assert_equal common_or + new_or, a.or(b)
+    end
+
+    test "or will use only common conditions if one side only has common conditions" do
+      only_common = WhereClause.new([
+        table["id"].eq(bind_param(1)),
+        "foo = bar",
+      ])
+
+      common_with_extra = WhereClause.new([
+        table["id"].eq(bind_param(1)),
+        "foo = bar",
+        table["extra"].eq(bind_param("pluto")),
+      ])
+
+      assert_equal only_common, only_common.or(common_with_extra)
+      assert_equal only_common, common_with_extra.or(only_common)
+    end
+
     private
 
       def table