aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorErnie Miller <ernie@erniemiller.org>2012-08-19 08:02:09 -0400
committerErnie Miller <ernie@erniemiller.org>2012-08-19 08:06:31 -0400
commitbf80522be4d7ac521fa3c30d889afa68450a0bc2 (patch)
tree687385bd46b8a6d0761ea35fc82e9e6417c5eccb
parentf9fc26e8007d5e2936cf5374a8168b2c37273490 (diff)
downloadrails-bf80522be4d7ac521fa3c30d889afa68450a0bc2.tar.gz
rails-bf80522be4d7ac521fa3c30d889afa68450a0bc2.tar.bz2
rails-bf80522be4d7ac521fa3c30d889afa68450a0bc2.zip
Fix "last equality wins" logic in relation merge
This is a real fix (as compared to the band-aid in b127d86c), which uses the recently-added equality methods for ARel nodes. It has the side benefit of simplifying the merge code a bit.
-rw-r--r--activerecord/lib/active_record/relation/merger.rb15
-rw-r--r--activerecord/test/cases/relations_test.rb11
2 files changed, 12 insertions, 14 deletions
diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb
index 7531f22494..e5b50673da 100644
--- a/activerecord/lib/active_record/relation/merger.rb
+++ b/activerecord/lib/active_record/relation/merger.rb
@@ -97,18 +97,13 @@ module ActiveRecord
merged_wheres = relation.where_values + values[:where]
unless relation.where_values.empty?
- # Remove duplicate ARel attributes. Last one wins.
- seen = Hash.new { |h,table| h[table] = {} }
+ # Remove equalities with duplicated left-hand. Last one wins.
+ seen = {}
merged_wheres = merged_wheres.reverse.reject { |w|
nuke = false
- # 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]
- seen[table][name] = true
+ if w.respond_to?(:operator) && w.operator == :==
+ nuke = seen[w.left]
+ seen[w.left] = true
end
nuke
}.reverse
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 4ef11590a9..684538940a 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -675,11 +675,14 @@ class RelationTest < ActiveRecord::TestCase
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
+ def test_relation_merging_with_arel_equalities_keeps_last_equality_with_non_attribute_left_hand
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))
+ devs = Developer.where(
+ Arel::Nodes::NamedFunction.new('abs', [salary_attr]).eq(80000)
+ ).merge(
+ Developer.where(
+ Arel::Nodes::NamedFunction.new('abs', [salary_attr]).eq(9000)
+ )
)
assert_equal [developers(:poor_jamis)], devs.to_a
end