aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorErnie Miller <ernie@erniemiller.org>2012-08-17 11:31:23 -0400
committerErnie Miller <ernie@erniemiller.org>2012-08-17 11:31:23 -0400
commitb127d86c1823d60a29191ecc3c21c97ee3ac502c (patch)
tree2b55c31b9708d671a355e5cb68354ab9cdf1550e /activerecord
parent1411fc19862060a959f99be19770fb7eb1ad104f (diff)
downloadrails-b127d86c1823d60a29191ecc3c21c97ee3ac502c.tar.gz
rails-b127d86c1823d60a29191ecc3c21c97ee3ac502c.tar.bz2
rails-b127d86c1823d60a29191ecc3c21c97ee3ac502c.zip
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, ernie/squeel#153, ernie/squeel#156
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/lib/active_record/relation/merger.rb7
-rw-r--r--activerecord/test/cases/relations_test.rb16
2 files changed, 21 insertions, 2 deletions
diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb
index 71aaedee1e..7531f22494 100644
--- a/activerecord/lib/active_record/relation/merger.rb
+++ b/activerecord/lib/active_record/relation/merger.rb
@@ -97,11 +97,14 @@ module ActiveRecord
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]
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 84027ea5ae..4ef11590a9 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -668,6 +668,22 @@ class RelationTest < ActiveRecord::TestCase
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)