diff options
| -rw-r--r-- | activerecord/lib/active_record/relation/merger.rb | 40 | ||||
| -rw-r--r-- | activerecord/test/cases/relations_test.rb | 22 |
2 files changed, 42 insertions, 20 deletions
diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index 98afeb8cc5..c114ea0c0d 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -99,8 +99,15 @@ module ActiveRecord 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 @@ module ActiveRecord end end - def merged_binds - if values[:bind] - (relation.bind_values + values[:bind]).uniq(&:first) - else - relation.bind_values - end - end - - def merged_wheres - rhs_wheres = values[:where] || [] - lhs_wheres = relation.where_values + def filter_binds(lhs_binds, removed_wheres) + return lhs_binds if removed_wheres.empty? - 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 diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index cf6af4e8f4..b64ff13d29 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1546,4 +1546,26 @@ class RelationTest < ActiveRecord::TestCase 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 |