Skip to content

Commit 31d3cae

Browse files
committed
Don't want to duplicate untouched values on merge
I'm ok to allow explicit `select(:id, :id)` duplication, but it is not the expected behavior that values that users has not touched are duplicated each time `merge` is called.
1 parent 0628f72 commit 31d3cae

File tree

2 files changed

+3
-11
lines changed

2 files changed

+3
-11
lines changed

activerecord/lib/active_record/relation/merger.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ def merge_select_values
8585
return if other.select_values.empty?
8686

8787
if other.model == relation.model
88-
relation.select_values += other.select_values
88+
relation.select_values += other.select_values if relation.select_values != other.select_values
8989
else
9090
relation.select_values += other.instance_eval do
9191
arel_columns(select_values)

activerecord/test/cases/relation_test.rb

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,11 @@ def test_multi_values_deduplication_with_merge
4747
unscope: [ :where ],
4848
extending: [ Module.new ],
4949
with: [ foo: Post.all ],
50+
select: [ :id, :id ],
5051
}
5152
expected.default = [ Object.new ]
5253

53-
(Relation::MULTI_VALUE_METHODS - [:select]).each do |method|
54+
Relation::MULTI_VALUE_METHODS.each do |method|
5455
getter, setter = "#{method}_values", "#{method}_values="
5556
values = expected[method]
5657
relation = Relation.new(FakeKlass)
@@ -293,15 +294,6 @@ def test_relation_merging_with_joins_as_join_dependency_pick_proper_parent
293294
assert_equal 3, relation.where(id: post.id).pluck(:id).size
294295
end
295296

296-
def test_merge_preserves_duplicate_columns
297-
quoted_posts_id = Regexp.escape(quote_table_name("posts.id"))
298-
quoted_posts = Regexp.escape(quote_table_name("posts"))
299-
posts = Post.select(:id)
300-
assert_queries_match(/SELECT #{quoted_posts_id}, #{quoted_posts_id} FROM #{quoted_posts}/i) do
301-
posts.merge(posts).to_a
302-
end
303-
end
304-
305297
def test_merge_raises_with_invalid_argument
306298
assert_raises ArgumentError do
307299
relation = Relation.new(FakeKlass)

0 commit comments

Comments
 (0)