Merge pull request #29914 from kamipo/relation_merger_should_not_fill_empty_values

`Relation::Merger` should not fill `values` with empty values

3 files changed, 30 insertions, 19 deletions

diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb
index 182f654897..03824ffff9 100644
--- a/activerecord/lib/active_record/relation/merger.rb
+++ b/activerecord/lib/active_record/relation/merger.rb
@@ -135,19 +135,17 @@ module ActiveRecord
           if other.reordering_value
             # override any order specified in the original relation
             relation.reorder! other.order_values
-          elsif other.order_values
+          elsif other.order_values.any?
             # merge in order_values from relation
             relation.order! other.order_values
           end
 
-          relation.extend(*other.extending_values) unless other.extending_values.blank?
+          extensions = other.extensions - relation.extensions
+          relation.extending!(*extensions) if extensions.any?
         end
 
         def merge_single_values
-          if relation.from_clause.empty?
-            relation.from_clause = other.from_clause
-          end
-          relation.lock_value ||= other.lock_value
+          relation.lock_value ||= other.lock_value if other.lock_value
 
           unless other.create_with_value.blank?
             relation.create_with_value = (relation.create_with_value || {}).merge(other.create_with_value)
@@ -155,11 +153,15 @@ module ActiveRecord
         end
 
         def merge_clauses
-          CLAUSE_METHODS.each do |method|
-            clause = relation.get_value(method)
-            other_clause = other.get_value(method)
-            relation.set_value(method, clause.merge(other_clause))
+          if relation.from_clause.empty? && !other.from_clause.empty?
+            relation.from_clause = other.from_clause
           end
+
+          where_clause = relation.where_clause.merge(other.where_clause)
+          relation.where_clause = where_clause unless where_clause.empty?
+
+          having_clause = relation.having_clause.merge(other.having_clause)
+          relation.having_clause = having_clause unless having_clause.empty?
         end
     end
   end
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index f7fe968b55..bdc5c27328 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -902,16 +902,17 @@ module ActiveRecord
       @arel ||= build_arel
     end
 
-    # Returns a relation value with a given name
-    def get_value(name) # :nodoc:
-      @values[name] || default_value_for(name)
-    end
+    protected
+      # Returns a relation value with a given name
+      def get_value(name) # :nodoc:
+        @values[name] || default_value_for(name)
+      end
 
-    # Sets the relation value with the given name
-    def set_value(name, value) # :nodoc:
-      assert_mutability!
-      @values[name] = value
-    end
+      # Sets the relation value with the given name
+      def set_value(name, value) # :nodoc:
+        assert_mutability!
+        @values[name] = value
+      end
 
     private
 
diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb
index f22fcd7b5a..c1805aa592 100644
--- a/activerecord/test/cases/relation_test.rb
+++ b/activerecord/test/cases/relation_test.rb
@@ -100,6 +100,14 @@ module ActiveRecord
       assert_equal({ "hello" => "world", "id" => 10 }, relation.scope_for_create)
     end
 
+    def test_empty_scope
+      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
+      assert relation.empty_scope?
+
+      relation.merge!(relation)
+      assert relation.empty_scope?
+    end
+
     def test_bad_constants_raise_errors
       assert_raises(NameError) do
         ActiveRecord::Relation::HelloWorld