TEST_CODEOWNERS/rails
3
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Fix merge error when Equality LHS is non-attribute

Browse Source 
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
This commit is contained in:
Ernie Miller 2012-08-17 11:31:23 -04:00
parent 1411fc1986
commit b127d86c18
2 changed files with 21 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
activerecord/lib/active_record/relation/merger.rb
View File

@ -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]

16
activerecord/test/cases/relations_test.rb
View File

@ -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)