kotovalexarian-likes-github/rails--rails
1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00
Code Releases Activity

Fix merging select values from a different model's relation

Browse source 
Unlike order values, where/having clause, etc, select values lazily
evaluates the attribute name resolution until building arel.

If select values are merged from a different model's relation, those may
be incorrectly resolved as a column in a wrong table.

In that case, the attribute name resolution should be done before
merging those.
This commit is contained in:
Ryuta Kamizono 2021-03-08 06:33:31 +09:00
parent b881a646da
commit ac6c4bde86
2 changed files with 34 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
activerecord/lib/active_record/relation/merger.rb
View file

@ -51,30 +51,25 @@ module ActiveRecord
@rewhere = rewhere
end
NORMAL_VALUES = Relation::VALUE_METHODS -
Relation::CLAUSE_METHODS -
[:includes, :preload, :joins, :left_outer_joins, :order, :reverse_order, :lock, :create_with, :reordering] # :nodoc:
def normal_values
NORMAL_VALUES
end
NORMAL_VALUES = Relation::VALUE_METHODS - Relation::CLAUSE_METHODS -
[
:select, :includes, :preload, :joins, :left_outer_joins,
:order, :reverse_order, :lock, :create_with, :reordering
]
def merge
normal_values.each do |name|
NORMAL_VALUES.each do |name|
value = values[name]
# The unless clause is here mostly for performance reasons (since the `send` call might be moderately
# expensive), most of the time the value is going to be `nil` or `.blank?`, the only catch is that
# `false.blank?` returns `true`, so there needs to be an extra check so that explicit `false` values
# don't fall through the cracks.
unless value.nil? || (value.blank? && false != value)
if name == :select
relation._select!(*value)
else
relation.public_send("#{name}!", *value)
end
relation.public_send(:"#{name}!", *value)
end
end
merge_select_values
merge_multi_values
merge_single_values
merge_clauses
@ -86,6 +81,18 @@ module ActiveRecord
end
private
def merge_select_values
return if other.select_values.empty?
if other.klass == relation.klass
relation.select_values |= other.select_values
else
relation.select_values |= other.instance_eval do
arel_columns(select_values)
end
end
end
def merge_preloads
return if other.preload_values.empty? && other.includes_values.empty?

14
activerecord/test/cases/relation/select_test.rb
View file

@ -41,6 +41,20 @@ module ActiveRecord
end
private :assert_non_select_columns_wont_be_loaded
def test_merging_select_from_different_model
posts = Post.select(:id, :title).joins(:comments)
comments = Comment.where(body: "Thank you for the welcome")
[
posts.merge(comments.select(:body)).first,
posts.merge(comments.select("comments.body")).first,
].each do |post|
assert_equal 1, post.id
assert_equal "Welcome to the weblog", post.title
assert_equal "Thank you for the welcome", post.body
end
end
def test_type_casted_extra_select_with_eager_loading
posts = Post.select("posts.id * 1.1 AS foo").eager_load(:comments)
assert_equal 1.1, posts.first.foo