Merge branch 'bindwhere'

* bindwhere:
  avoid creating a set if no where values are removed
  remove bind values for where clauses that were removed
  push partitioning up so bind elimination can get the removed wheres
  push partion logic down and initialization logic up
  partition the where values so we can access the removed ones

activerecord/lib/active_record/relation/merger.rb

@@ -99,8 +99,15 @@ def merge_joins
       end
 
       def merge_multi_values
-        relation.where_values = merged_wheres
-        relation.bind_values  = merged_binds
+        lhs_wheres = relation.where_values
+        rhs_wheres = values[:where] || []
+        lhs_binds  = relation.bind_values
+        rhs_binds  = values[:bind] || []
+
+        removed, kept = partition_overwrites(lhs_wheres, rhs_wheres)
+
+        relation.where_values = kept + rhs_wheres
+        relation.bind_values  = filter_binds(lhs_binds, removed) + rhs_binds
 
         if values[:reordering]
           # override any order specified in the original relation
@@ -123,34 +130,27 @@ def merge_single_values
         end
       end
 
-      def merged_binds
-        if values[:bind]
-          (relation.bind_values + values[:bind]).uniq(&:first)
-        else
-          relation.bind_values
-        end
-      end
+      def filter_binds(lhs_binds, removed_wheres)
+        return lhs_binds if removed_wheres.empty?
 
-      def merged_wheres
-        rhs_wheres = values[:where] || []
-        lhs_wheres = relation.where_values
-
-        if rhs_wheres.empty? || lhs_wheres.empty?
-          lhs_wheres + rhs_wheres
-        else
-          reject_overwrites(lhs_wheres, rhs_wheres) + rhs_wheres
-        end
+        set = Set.new removed_wheres.map { |x| x.left.name }
+        lhs_binds.dup.delete_if { |col,_| set.include? col.name }
       end
 
       # Remove equalities from the existing relation with a LHS which is
       # present in the relation being merged in.
-      def reject_overwrites(lhs_wheres, rhs_wheres)
+      # returns [things_to_remove, things_to_keep]
+      def partition_overwrites(lhs_wheres, rhs_wheres)
+        if lhs_wheres.empty? || rhs_wheres.empty?
+          return [[], lhs_wheres]
+        end
+
         nodes = rhs_wheres.find_all do |w|
           w.respond_to?(:operator) && w.operator == :==
         end
         seen = Set.new(nodes) { |node| node.left }
 
-        lhs_wheres.reject do |w|
+        lhs_wheres.partition do |w|
           w.respond_to?(:operator) && w.operator == :== && seen.include?(w.left)
         end
       end

activerecord/test/cases/relations_test.rb

@@ -1546,4 +1546,26 @@ def __omg__
     assert merged.to_sql.include?("wtf")
     assert merged.to_sql.include?("bbq")
   end
+
+  def test_merging_removes_rhs_bind_parameters
+    left  = Post.where(id: Arel::Nodes::BindParam.new('?'))
+    column = Post.columns_hash['id']
+    left.bind_values += [[column, 20]]
+    right   = Post.where(id: 10)
+
+    merged = left.merge(right)
+    assert_equal [], merged.bind_values
+  end
+
+  def test_merging_keeps_lhs_bind_parameters
+    column = Post.columns_hash['id']
+    binds = [[column, 20]]
+
+    right  = Post.where(id: Arel::Nodes::BindParam.new('?'))
+    right.bind_values += binds
+    left   = Post.where(id: 10)
+
+    merged = left.merge(right)
+    assert_equal binds, merged.bind_values
+  end
 end