Skip to content
Browse files

Fix merge error when Equality LHS is non-attribute

This is at best a band-aid for a more proper fix, since it won't truly
handle the removal of the previous equality condition of these other
nodes. I'm planning to put in some work on ARel toward supporting that
goal.

Related: rails/arel#130, activerecord-hackery/squeel#153, activerecord-hackery/squeel#156
  • Loading branch information...
1 parent 1411fc1 commit b127d86c1823d60a29191ecc3c21c97ee3ac502c @ernie ernie committed Aug 17, 2012
Showing with 21 additions and 2 deletions.
  1. +5 −2 activerecord/lib/active_record/relation/merger.rb
  2. +16 −0 activerecord/test/cases/relations_test.rb
View
7 activerecord/lib/active_record/relation/merger.rb
@@ -97,11 +97,14 @@ def merged_wheres
merged_wheres = relation.where_values + values[:where]
unless relation.where_values.empty?
- # Remove duplicates, last one wins.
+ # Remove duplicate ARel attributes. Last one wins.
seen = Hash.new { |h,table| h[table] = {} }
merged_wheres = merged_wheres.reverse.reject { |w|
nuke = false
- if w.respond_to?(:operator) && w.operator == :==
+ # We might have non-attributes on the left side of equality nodes,
+ # so we need to make sure they quack like an attribute.
+ if w.respond_to?(:operator) && w.operator == :== &&
+ w.left.respond_to?(:relation)
name = w.left.name
table = w.left.relation.name
nuke = seen[table][name]
View
16 activerecord/test/cases/relations_test.rb
@@ -668,6 +668,22 @@ def test_relation_merging
assert_equal [developers(:poor_jamis)], dev_with_count.to_a
end
+ def test_relation_merging_with_arel_equalities_keeps_last_equality
+ devs = Developer.where(Developer.arel_table[:salary].eq(80000)).merge(
+ Developer.where(Developer.arel_table[:salary].eq(9000))
+ )
+ assert_equal [developers(:poor_jamis)], devs.to_a
+ end
+
+ def test_relation_merging_with_arel_equalities_with_a_non_attribute_left_hand_ignores_non_attributes_when_discarding_equalities
+ salary_attr = Developer.arel_table[:salary]
+ devs = Developer.where(salary_attr.eq(80000)).merge(
+ Developer.where(salary_attr.eq(9000)).
+ where(Arel::Nodes::NamedFunction.new('abs', [salary_attr]).eq(9000))
+ )
+ assert_equal [developers(:poor_jamis)], devs.to_a
+ end
+
def test_relation_merging_with_eager_load
relations = []
relations << Post.order('comments.id DESC').merge(Post.eager_load(:last_comment)).merge(Post.all)

0 comments on commit b127d86

Please sign in to comment.
Something went wrong with that request. Please try again.