mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Fix eager loading that non-select columns will be loaded
Related to #35210. We sometimes use `select` to limit unused columns for performance. For example, `GET /posts/1` (post detail) usually use (almost) all columns, but `GET /posts` (post list) does not always use all columns (e.g. use `id` and `title` for the list view, but `body` is not used). If an association is eager loaded, the limited `select` doesn't works as expected, eager loading will load all columns on the model, plus also load the `select` columns additionally. It works differently with natural load and preload. It means that changing natural load or preload to eager load (or vice versa) is unsafe. This fixes eager loading that always load all columns (plus extra `select` columns), to respect the `select` columns like as others. ```ruby post = Post.select("UPPER(title) AS title").first post.title # => "WELCOME TO THE WEBLOG" post.body # => ActiveModel::MissingAttributeError # Rails 6.0 (ignore the `select` values) post = Post.select("UPPER(title) AS title").eager_load(:comments).first post.title # => "Welcome to the weblog" post.body # => "Such a lovely day" # Rails 6.1 (respect the `select` values) post = Post.select("UPPER(title) AS title").eager_load(:comments).first post.title # => "WELCOME TO THE WEBLOG" post.body # => ActiveModel::MissingAttributeError ```
This commit is contained in:
parent
e8cf45dc4a
commit
4639318215
5 changed files with 67 additions and 22 deletions
|
@ -1,3 +1,23 @@
|
||||||
|
* Respect the `select` values for eager loading.
|
||||||
|
|
||||||
|
```ruby
|
||||||
|
post = Post.select("UPPER(title) AS title").first
|
||||||
|
post.title # => "WELCOME TO THE WEBLOG"
|
||||||
|
post.body # => ActiveModel::MissingAttributeError
|
||||||
|
|
||||||
|
# Rails 6.0 (ignore the `select` values)
|
||||||
|
post = Post.select("UPPER(title) AS title").eager_load(:comments).first
|
||||||
|
post.title # => "Welcome to the weblog"
|
||||||
|
post.body # => "Such a lovely day"
|
||||||
|
|
||||||
|
# Rails 6.1 (respect the `select` values)
|
||||||
|
post = Post.select("UPPER(title) AS title").eager_load(:comments).first
|
||||||
|
post.title # => "WELCOME TO THE WEBLOG"
|
||||||
|
post.body # => ActiveModel::MissingAttributeError
|
||||||
|
```
|
||||||
|
|
||||||
|
*Ryuta Kamizono*
|
||||||
|
|
||||||
* Allow attribute's default to be configured but keeping its own type.
|
* Allow attribute's default to be configured but keeping its own type.
|
||||||
|
|
||||||
```ruby
|
```ruby
|
||||||
|
|
|
@ -34,7 +34,7 @@ module ActiveRecord
|
||||||
Table = Struct.new(:node, :columns) do # :nodoc:
|
Table = Struct.new(:node, :columns) do # :nodoc:
|
||||||
def column_aliases
|
def column_aliases
|
||||||
t = node.table
|
t = node.table
|
||||||
columns.map { |column| t[column.name].as Arel.sql column.alias }
|
columns.map { |column| t[column.name].as(column.alias) }
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
Column = Struct.new(:name, :alias)
|
Column = Struct.new(:name, :alias)
|
||||||
|
@ -105,13 +105,20 @@ module ActiveRecord
|
||||||
parents = model_cache[join_root]
|
parents = model_cache[join_root]
|
||||||
|
|
||||||
column_aliases = aliases.column_aliases(join_root)
|
column_aliases = aliases.column_aliases(join_root)
|
||||||
column_names = explicit_selections(column_aliases, result_set)
|
column_names = []
|
||||||
|
|
||||||
|
result_set.columns.each do |name|
|
||||||
|
column_names << name unless /\At\d+_r\d+\z/.match?(name)
|
||||||
|
end
|
||||||
|
|
||||||
if column_names.empty?
|
if column_names.empty?
|
||||||
column_types = {}
|
column_types = {}
|
||||||
else
|
else
|
||||||
column_types = result_set.column_types
|
column_types = result_set.column_types
|
||||||
column_types = column_types.slice(*column_names) unless column_types.empty?
|
unless column_types.empty?
|
||||||
|
attribute_types = join_root.attribute_types
|
||||||
|
column_types = column_types.slice(*column_names).delete_if { |k, _| attribute_types.key?(k) }
|
||||||
|
end
|
||||||
column_aliases += column_names.map! { |name| Aliases::Column.new(name, name) }
|
column_aliases += column_names.map! { |name| Aliases::Column.new(name, name) }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -134,6 +141,7 @@ module ActiveRecord
|
||||||
end
|
end
|
||||||
|
|
||||||
def apply_column_aliases(relation)
|
def apply_column_aliases(relation)
|
||||||
|
@join_root_alias = relation.select_values.empty?
|
||||||
relation._select!(-> { aliases.columns })
|
relation._select!(-> { aliases.columns })
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -145,18 +153,18 @@ module ActiveRecord
|
||||||
attr_reader :join_root, :join_type
|
attr_reader :join_root, :join_type
|
||||||
|
|
||||||
private
|
private
|
||||||
attr_reader :alias_tracker
|
attr_reader :alias_tracker, :join_root_alias
|
||||||
|
|
||||||
def explicit_selections(root_column_aliases, result_set)
|
|
||||||
root_names = root_column_aliases.map(&:name).to_set
|
|
||||||
result_set.columns.each_with_object([]) do |name, result|
|
|
||||||
result << name unless /\At\d+_r\d+\z/.match?(name) || root_names.include?(name)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
def aliases
|
def aliases
|
||||||
@aliases ||= Aliases.new join_root.each_with_index.map { |join_part, i|
|
@aliases ||= Aliases.new join_root.each_with_index.map { |join_part, i|
|
||||||
columns = join_part.column_names.each_with_index.map { |column_name, j|
|
column_names = if join_part == join_root && !join_root_alias
|
||||||
|
primary_key = join_root.primary_key
|
||||||
|
primary_key ? [primary_key] : []
|
||||||
|
else
|
||||||
|
join_part.column_names
|
||||||
|
end
|
||||||
|
|
||||||
|
columns = column_names.each_with_index.map { |column_name, j|
|
||||||
Aliases::Column.new column_name, "t#{i}_r#{j}"
|
Aliases::Column.new column_name, "t#{i}_r#{j}"
|
||||||
}
|
}
|
||||||
Aliases::Table.new(join_part, columns)
|
Aliases::Table.new(join_part, columns)
|
||||||
|
|
|
@ -17,7 +17,7 @@ module ActiveRecord
|
||||||
# association.
|
# association.
|
||||||
attr_reader :base_klass, :children
|
attr_reader :base_klass, :children
|
||||||
|
|
||||||
delegate :table_name, :column_names, :primary_key, to: :base_klass
|
delegate :table_name, :column_names, :primary_key, :attribute_types, to: :base_klass
|
||||||
|
|
||||||
def initialize(base_klass, children)
|
def initialize(base_klass, children)
|
||||||
@base_klass = base_klass
|
@base_klass = base_klass
|
||||||
|
|
|
@ -1375,7 +1375,14 @@ class FinderTest < ActiveRecord::TestCase
|
||||||
limit: 3, order: "posts.id"
|
limit: 3, order: "posts.id"
|
||||||
).to_a
|
).to_a
|
||||||
assert_equal 3, posts.size
|
assert_equal 3, posts.size
|
||||||
assert_equal [0, 1, 1], posts.map(&:author_id).sort
|
assert_equal [1, 1, nil], posts.map(&:author_id)
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_custom_select_takes_precedence_over_original_value
|
||||||
|
posts = Post.select("UPPER(title) AS title")
|
||||||
|
assert_equal "WELCOME TO THE WEBLOG", posts.first.title
|
||||||
|
assert_equal "WELCOME TO THE WEBLOG", posts.preload(:comments).first.title
|
||||||
|
assert_equal "WELCOME TO THE WEBLOG", posts.eager_load(:comments).first.title
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_eager_load_for_no_has_many_with_limit_and_joins_for_has_many
|
def test_eager_load_for_no_has_many_with_limit_and_joins_for_has_many
|
||||||
|
|
|
@ -25,6 +25,22 @@ module ActiveRecord
|
||||||
assert_equal expected, actual
|
assert_equal expected, actual
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_non_select_columns_wont_be_loaded
|
||||||
|
posts = Post.select("UPPER(title) AS title")
|
||||||
|
|
||||||
|
assert_non_select_columns_wont_be_loaded(posts.first)
|
||||||
|
assert_non_select_columns_wont_be_loaded(posts.preload(:comments).first)
|
||||||
|
assert_non_select_columns_wont_be_loaded(posts.eager_load(:comments).first)
|
||||||
|
end
|
||||||
|
|
||||||
|
def assert_non_select_columns_wont_be_loaded(post)
|
||||||
|
assert_equal "WELCOME TO THE WEBLOG", post.title
|
||||||
|
assert_raise(ActiveModel::MissingAttributeError) do
|
||||||
|
post.body
|
||||||
|
end
|
||||||
|
end
|
||||||
|
private :assert_non_select_columns_wont_be_loaded
|
||||||
|
|
||||||
def test_type_casted_extra_select_with_eager_loading
|
def test_type_casted_extra_select_with_eager_loading
|
||||||
posts = Post.select("posts.id * 1.1 AS foo").eager_load(:comments)
|
posts = Post.select("posts.id * 1.1 AS foo").eager_load(:comments)
|
||||||
assert_equal 1.1, posts.first.foo
|
assert_equal 1.1, posts.first.foo
|
||||||
|
@ -32,18 +48,12 @@ module ActiveRecord
|
||||||
|
|
||||||
def test_aliased_select_using_as_with_joins_and_includes
|
def test_aliased_select_using_as_with_joins_and_includes
|
||||||
posts = Post.select("posts.id AS field_alias").joins(:comments).includes(:comments)
|
posts = Post.select("posts.id AS field_alias").joins(:comments).includes(:comments)
|
||||||
assert_equal %w(
|
assert_equal %w(id field_alias), posts.first.attributes.keys
|
||||||
id author_id title body type legacy_comments_count taggings_with_delete_all_count taggings_with_destroy_count
|
|
||||||
tags_count indestructible_tags_count tags_with_destroy_count tags_with_nullify_count field_alias
|
|
||||||
), posts.first.attributes.keys
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_aliased_select_not_using_as_with_joins_and_includes
|
def test_aliased_select_not_using_as_with_joins_and_includes
|
||||||
posts = Post.select("posts.id field_alias").joins(:comments).includes(:comments)
|
posts = Post.select("posts.id field_alias").joins(:comments).includes(:comments)
|
||||||
assert_equal %w(
|
assert_equal %w(id field_alias), posts.first.attributes.keys
|
||||||
id author_id title body type legacy_comments_count taggings_with_delete_all_count taggings_with_destroy_count
|
|
||||||
tags_count indestructible_tags_count tags_with_destroy_count tags_with_nullify_count field_alias
|
|
||||||
), posts.first.attributes.keys
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_star_select_with_joins_and_includes
|
def test_star_select_with_joins_and_includes
|
||||||
|
|
Loading…
Reference in a new issue