From 9675c7da28758432ec7bd6e7ca1814801b2591f5 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 15 Jul 2013 16:29:01 -0700 Subject: [PATCH] reorder bind parameters when merging relations --- .../lib/active_record/relation/merger.rb | 20 +++++++++++++++++-- activerecord/test/cases/relation_test.rb | 9 ++++++++- activerecord/test/cases/relations_test.rb | 17 ++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index f5d668dae8..da13152e01 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -97,13 +97,29 @@ module ActiveRecord def merge_multi_values 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 + where_values = kept + rhs_wheres + bind_values = filter_binds(lhs_binds, removed) + rhs_binds + + conn = relation.klass.connection + bviter = bind_values.each.with_index + where_values.map! do |node| + if Arel::Nodes::Equality === node && Arel::Nodes::BindParam === node.right + (column, _), i = bviter.next + substitute = conn.substitute_at column, i + Arel::Nodes::Equality.new(node.left, substitute) + else + node + end + end + + relation.where_values = where_values + relation.bind_values = bind_values if values[:reordering] # override any order specified in the original relation diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 7862d96bd7..7c90f54343 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -9,6 +9,9 @@ module ActiveRecord fixtures :posts, :comments, :authors class FakeKlass < Struct.new(:table_name, :name) + def self.connection + Post.connection + end end def test_construction @@ -204,6 +207,10 @@ module ActiveRecord def arel_table Post.arel_table end + + def connection + Post.connection + end end def relation @@ -295,7 +302,7 @@ module ActiveRecord assert_equal({foo: 'bar'}, relation.create_with_value) end - test 'merge!' do + def test_merge! assert relation.merge!(where: :foo).equal?(relation) assert_equal [:foo], relation.where_values end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index b205472cf5..d0f8731b9a 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1556,4 +1556,21 @@ class RelationTest < ActiveRecord::TestCase merged = left.merge(right) assert_equal binds, merged.bind_values end + + def test_merging_reorders_bind_params + post = Post.first + id_column = Post.columns_hash['id'] + title_column = Post.columns_hash['title'] + + bv = Post.connection.substitute_at id_column, 0 + + right = Post.where(id: bv) + right.bind_values += [[id_column, post.id]] + + left = Post.where(title: bv) + left.bind_values += [[title_column, post.title]] + + merged = left.merge(right) + assert_equal post, merged.first + end end