From c8d889905dab071f1d8a166b25fa69cdd31dc176 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Sun, 27 Jan 2013 20:31:01 +0000
Subject: Prevent Relation#merge from collapsing wheres on the RHS

This caused a bug with the new associations implementation, because now
association conditions are represented as Arel nodes internally right up
to when the whole thing gets turned to SQL.

In Rails 3.2, association conditions get turned to raw SQL early on,
which prevents Relation#merge from interfering.

The current implementation was buggy when a default_scope existed on the
target model, since we would basically end up doing:

  default_scope.merge(association_scope)

If default_scope contained a where(foo: 'a') and association_scope
contained a where(foo: 'b').where(foo: 'c') then the merger would see
that the same column is representated on both sides of the merge and
collapse the wheres to all but the last: where(foo: 'c')

Now, the RHS of the merge is left alone.

Fixes #8990
---
 .../associations/association_scope.rb              | 12 +++++---
 activerecord/lib/active_record/relation/merger.rb  | 36 ++++++++++++----------
 2 files changed, 27 insertions(+), 21 deletions(-)

(limited to 'activerecord/lib')

diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb
index 300f67959d..c5fb1fe2c7 100644
--- a/activerecord/lib/active_record/associations/association_scope.rb
+++ b/activerecord/lib/active_record/associations/association_scope.rb
@@ -15,7 +15,6 @@ module ActiveRecord
 
       def scope
         scope = klass.unscoped
-        scope.merge! eval_scope(klass, reflection.scope) if reflection.scope
         scope.extending! Array(options[:extend])
         add_constraints(scope)
       end
@@ -59,7 +58,7 @@ module ActiveRecord
 
           if reflection.source_macro == :belongs_to
             if reflection.options[:polymorphic]
-              key = reflection.association_primary_key(klass)
+              key = reflection.association_primary_key(self.klass)
             else
               key = reflection.association_primary_key
             end
@@ -92,8 +91,13 @@ module ActiveRecord
 
           # Exclude the scope of the association itself, because that
           # was already merged in the #scope method.
-          (scope_chain[i] - [self.reflection.scope]).each do |scope_chain_item|
-            item = eval_scope(reflection.klass, scope_chain_item)
+          scope_chain[i].each do |scope_chain_item|
+            klass = i == 0 ? self.klass : reflection.klass
+            item  = eval_scope(klass, scope_chain_item)
+
+            if scope_chain_item == self.reflection.scope
+              scope.merge! item.except(:where, :includes)
+            end
 
             scope.includes! item.includes_values
             scope.where_values += item.where_values
diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb
index 59226d316e..eb23e92fb8 100644
--- a/activerecord/lib/active_record/relation/merger.rb
+++ b/activerecord/lib/active_record/relation/merger.rb
@@ -1,4 +1,5 @@
 require 'active_support/core_ext/hash/keys'
+require "set"
 
 module ActiveRecord
   class Relation
@@ -105,25 +106,26 @@ module ActiveRecord
       end
 
       def merged_wheres
-        if values[:where]
-          merged_wheres = relation.where_values + values[:where]
-
-          unless relation.where_values.empty?
-            # Remove equalities with duplicated left-hand. Last one wins.
-            seen = {}
-            merged_wheres = merged_wheres.reverse.reject { |w|
-              nuke = false
-              if w.respond_to?(:operator) && w.operator == :==
-                nuke         = seen[w.left]
-                seen[w.left] = true
-              end
-              nuke
-            }.reverse
-          end
+        values[:where] ||= []
 
-          merged_wheres
+        if values[:where].empty? || relation.where_values.empty?
+          relation.where_values + values[:where]
         else
-          relation.where_values
+          # Remove equalities from the existing relation with a LHS which is
+          # present in the relation being merged in.
+
+          seen = Set.new
+          values[:where].each { |w|
+            if w.respond_to?(:operator) && w.operator == :==
+              seen << w.left
+            end
+          }
+
+          relation.where_values.reject { |w|
+            w.respond_to?(:operator) &&
+              w.operator == :== &&
+              seen.include?(w.left)
+          } + values[:where]
         end
       end
     end
-- 
cgit v1.2.3