Merge pull request #39250 from kamipo/support_rewhere_for_merge

Support merging option `:rewhere` to allow mergee side condition to be replaced exactly

activerecord/CHANGELOG.md

@@ -1,3 +1,20 @@
+*   Support merging option `:rewhere` to allow mergee side condition to be replaced exactly.
+
+    ```ruby
+    david_and_mary = Author.where(id: david.id..mary.id)
+
+    # both conflict conditions exists
+    david_and_mary.merge(Author.where(id: bob)) # => []
+
+    # mergee side condition is replaced by rewhere
+    david_and_mary.merge(Author.rewhere(id: bob)) # => [bob]
+
+    # mergee side condition is replaced by rewhere option
+    david_and_mary.merge(Author.where(id: bob), rewhere: true) # => [bob]
+    ```
+
+    *Ryuta Kamizono*
+
 *   Add support for finding records based on signed ids, which are tamper-proof, verified ids that can be
     set to expire and scoped with a purpose. This is particularly useful for things like password reset
     or email verification, where you want the bearer of the signed id to be able to interact with the

activerecord/lib/active_record/relation/merger.rb

@@ -7,15 +7,16 @@ module ActiveRecord
     class HashMerger # :nodoc:
       attr_reader :relation, :hash
 
-      def initialize(relation, hash)
+      def initialize(relation, hash, rewhere = nil)
         hash.assert_valid_keys(*Relation::VALUE_METHODS)
 
         @relation = relation
         @hash     = hash
+        @rewhere  = rewhere
       end
 
-      def merge #:nodoc:
+      def merge
-        Merger.new(relation, other).merge
+        Merger.new(relation, other, @rewhere).merge
       end
 
       # Applying values to a relation has some side effects. E.g.
@@ -48,10 +49,11 @@ module ActiveRecord
     class Merger # :nodoc:
       attr_reader :relation, :values, :other
 
-      def initialize(relation, other)
+      def initialize(relation, other, rewhere = nil)
         @relation = relation
         @values   = other.values
         @other    = other
+        @rewhere  = rewhere
       end
 
       NORMAL_VALUES = Relation::VALUE_METHODS -
@@ -172,7 +174,7 @@ module ActiveRecord
         def merge_clauses
           relation.from_clause = other.from_clause if replace_from_clause?
 
-          where_clause = relation.where_clause.merge(other.where_clause)
+          where_clause = relation.where_clause.merge(other.where_clause, @rewhere)
           relation.where_clause = where_clause unless where_clause.empty?
 
           having_clause = relation.having_clause.merge(other.having_clause)

activerecord/lib/active_record/relation/query_methods.rb

@@ -702,15 +702,10 @@ module ActiveRecord
     # This is short-hand for <tt>unscope(where: conditions.keys).where(conditions)</tt>.
     # Note that unlike reorder, we're only unscoping the named conditions -- not the entire where statement.
     def rewhere(conditions)
-      attrs = []
       scope = spawn
-
       where_clause = scope.build_where_clause(conditions)
-      where_clause.each_attribute do |attr|
-        attrs << attr
-      end
 
-      scope.unscope!(where: attrs)
+      scope.unscope!(where: where_clause.extract_attributes)
       scope.where_clause += where_clause
       scope
     end

activerecord/lib/active_record/relation/spawn_methods.rb

@@ -28,21 +28,22 @@ module ActiveRecord
     #   # => Post.where(published: true).joins(:comments)
     #
     # This is mainly intended for sharing common conditions between multiple associations.
-    def merge(other)
+    def merge(other, *rest)
       if other.is_a?(Array)
         records & other
       elsif other
-        spawn.merge!(other)
+        spawn.merge!(other, *rest)
       else
         raise ArgumentError, "invalid argument: #{other.inspect}."
       end
     end
 
-    def merge!(other) # :nodoc:
+    def merge!(other, *rest) # :nodoc:
+      options = rest.extract_options!
       if other.is_a?(Hash)
-        Relation::HashMerger.new(self, other).merge
+        Relation::HashMerger.new(self, other, options[:rewhere]).merge
       elsif other.is_a?(Relation)
-        Relation::Merger.new(self, other).merge
+        Relation::Merger.new(self, other, options[:rewhere]).merge
       elsif other.respond_to?(:to_proc)
         instance_exec(&other)
       else

activerecord/lib/active_record/relation/where_clause.rb

@@ -10,21 +10,21 @@ module ActiveRecord
       end
 
       def +(other)
-        WhereClause.new(
+        WhereClause.new(predicates + other.predicates)
-          predicates + other.predicates,
-        )
       end
 
       def -(other)
-        WhereClause.new(
+        WhereClause.new(predicates - other.predicates)
-          predicates - other.predicates,
-        )
       end
 
-      def merge(other)
+      def merge(other, rewhere = nil)
-        WhereClause.new(
+        predicates = if rewhere
-          predicates_unreferenced_by(other) + other.predicates,
+          except_predicates(other.extract_attributes)
-        )
+        else
+          predicates_unreferenced_by(other)
+        end
+
+        WhereClause.new(predicates + other.predicates)
       end
 
       def except(*columns)
@@ -98,10 +98,12 @@ module ActiveRecord
         end
       end
 
-      def each_attribute(&block)
+      def extract_attributes
+        attrs = []
         predicates.each do |node|
-          Arel.fetch_attribute(node, &block)
+          Arel.fetch_attribute(node) { |attr| attrs << attr }
         end
+        attrs
       end
 
       protected

activerecord/test/cases/relation/merging_test.rb

@@ -13,6 +13,81 @@ require "models/rating"
 class RelationMergingTest < ActiveRecord::TestCase
   fixtures :developers, :comments, :authors, :author_addresses, :posts
 
+  def test_merge_in_clause
+    david, mary, bob = authors(:david, :mary, :bob)
+
+    david_and_mary = Author.where(id: [david, mary]).order(:id)
+    mary_and_bob   = Author.where(id: [mary, bob]).order(:id)
+
+    assert_equal [david, mary], david_and_mary
+    assert_equal [mary, bob],   mary_and_bob
+
+    assert_equal [mary], david_and_mary.merge(Author.where(id: mary))
+    assert_equal [bob],  david_and_mary.merge(Author.where(id: bob))
+
+    assert_equal [mary], david_and_mary.merge(Author.rewhere(id: mary))
+    assert_equal [bob],  david_and_mary.merge(Author.rewhere(id: bob))
+
+    assert_equal [mary], david_and_mary.merge(Author.where(id: mary), rewhere: true)
+    assert_equal [bob],  david_and_mary.merge(Author.where(id: bob), rewhere: true)
+
+    assert_equal [mary, bob], david_and_mary.merge(mary_and_bob)
+    assert_equal [david, mary], mary_and_bob.merge(david_and_mary)
+
+    assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true)
+    assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true)
+  end
+
+  def test_merge_between_clause
+    david, mary, bob = authors(:david, :mary, :bob)
+
+    david_and_mary = Author.where(id: david.id..mary.id).order(:id)
+    mary_and_bob   = Author.where(id: mary.id..bob.id).order(:id)
+
+    assert_equal [david, mary], david_and_mary
+    assert_equal [mary, bob],   mary_and_bob
+
+    assert_equal [mary], david_and_mary.merge(Author.where(id: mary))
+    assert_equal [],     david_and_mary.merge(Author.where(id: bob))
+
+    assert_equal [mary], david_and_mary.merge(Author.rewhere(id: mary))
+    assert_equal [bob],  david_and_mary.merge(Author.rewhere(id: bob))
+
+    assert_equal [mary], david_and_mary.merge(Author.where(id: mary), rewhere: true)
+    assert_equal [bob],  david_and_mary.merge(Author.where(id: bob), rewhere: true)
+
+    assert_equal [mary], david_and_mary.merge(mary_and_bob)
+    assert_equal [mary], mary_and_bob.merge(david_and_mary)
+
+    assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true)
+    assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true)
+  end
+
+  def test_merge_or_clause
+    david, mary, bob = authors(:david, :mary, :bob)
+
+    david_and_mary = Author.where(id: david).or(Author.where(id: mary)).order(:id)
+    mary_and_bob   = Author.where(id: mary).or(Author.where(id: bob)).order(:id)
+
+    assert_equal [david, mary], david_and_mary
+    assert_equal [mary, bob],   mary_and_bob
+
+    assert_equal [mary], david_and_mary.merge(Author.where(id: mary))
+    assert_equal [],     david_and_mary.merge(Author.where(id: bob))
+
+    assert_equal [mary], david_and_mary.merge(Author.rewhere(id: mary))
+    assert_equal [bob],  david_and_mary.merge(Author.rewhere(id: bob))
+
+    assert_equal [mary], david_and_mary.merge(Author.where(id: mary), rewhere: true)
+    assert_equal [bob],  david_and_mary.merge(Author.where(id: bob), rewhere: true)
+
+    assert_equal [mary], david_and_mary.merge(mary_and_bob)
+    assert_equal [mary], mary_and_bob.merge(david_and_mary)
+
+    assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true)
+    assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true)
+  end
+
   def test_relation_merging
     devs = Developer.where("salary >= 80000").merge(Developer.limit(2)).merge(Developer.order("id ASC").where("id < 3"))
     assert_equal [developers(:david), developers(:jamis)], devs.to_a