Permalink
Browse files

reorder bind parameters when merging relations

  • Loading branch information...
1 parent d345ed4 commit 9675c7da28758432ec7bd6e7ca1814801b2591f5 @tenderlove tenderlove committed Jul 15, 2013
@@ -97,13 +97,29 @@ def merge_joins
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
@@ -9,6 +9,9 @@ class RelationTest < ActiveRecord::TestCase
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 @@ class FakeKlass < Struct.new(:table_name, :name)
def arel_table
Post.arel_table
end
+
+ def connection
+ Post.connection
+ end
end
def relation
@@ -295,7 +302,7 @@ def relation
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
@@ -1556,4 +1556,21 @@ def test_merging_keeps_lhs_bind_parameters
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

1 comment on commit 9675c7d

Contributor

Empact commented on 9675c7d Jul 18, 2013

Was getting ActiveRecord::StatementInvalid: PG::Error: ERROR: bind message supplies 2 parameters, but prepared statement "a11" requires 1 under fairly complicated conditions. Wasn't able to nail down the exact interaction, or reproduce it outside the app, but replacing the rails 4.0.0 version of ActiveRecord::Relation::Merger#merge_multi_values with this one fixed the issue.

Hope this comment helps someone else out there.

Please sign in to comment.