Skip to content

Commit

Permalink
Type cast extra select for eager loading
Browse files Browse the repository at this point in the history
It is not a big issue for common types, since type casting in master
branch is much improved, but strictly speaking eager loading should
respect column types from database like as `find_by_sql` also does.

https://github.com/rails/rails/blob/97c34a55fa0f4e8d51504bd137f6e82155336d9a/activerecord/lib/active_record/querying.rb#L46-L52

Fixes #39605.
  • Loading branch information
kamipo committed Jul 7, 2020
1 parent 5394c94 commit 090ec3d
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 11 deletions.
20 changes: 14 additions & 6 deletions activerecord/lib/active_record/associations/join_dependency.rb
Expand Up @@ -104,8 +104,16 @@ def instantiate(result_set, strict_loading_value, &block)
model_cache = Hash.new { |h, klass| h[klass] = {} }
parents = model_cache[join_root]

column_aliases = aliases.column_aliases join_root
column_aliases += explicit_selections(column_aliases, result_set)
column_aliases = aliases.column_aliases(join_root)
column_names = explicit_selections(column_aliases, result_set)

if column_names.empty?
column_types = {}
else
column_types = result_set.column_types
column_types = column_types.slice(*column_names) unless column_types.empty?
column_aliases += column_names.map! { |name| Aliases::Column.new(name, name) }
end

message_bus = ActiveSupport::Notifications.instrumenter

Expand All @@ -117,7 +125,7 @@ def instantiate(result_set, strict_loading_value, &block)
message_bus.instrument("instantiation.active_record", payload) do
result_set.each { |row_hash|
parent_key = primary_key ? row_hash[primary_key] : row_hash
parent = parents[parent_key] ||= join_root.instantiate(row_hash, column_aliases, &block)
parent = parents[parent_key] ||= join_root.instantiate(row_hash, column_aliases, column_types, &block)
construct(parent, join_root, row_hash, seen, model_cache, strict_loading_value)
}
end
Expand All @@ -141,9 +149,9 @@ def each(&block)

def explicit_selections(root_column_aliases, result_set)
root_names = root_column_aliases.map(&:name).to_set
result_set.columns
.reject { |n| root_names.include?(n) || n =~ /\At\d+_r\d+\z/ }
.map { |n| Aliases::Column.new(n, n) }
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
Expand Down
Expand Up @@ -62,8 +62,8 @@ def extract_record(row, column_names_with_alias)
hash
end

def instantiate(row, aliases, &block)
base_klass.instantiate(extract_record(row, aliases), &block)
def instantiate(row, aliases, column_types = {}, &block)
base_klass.instantiate(extract_record(row, aliases), column_types, &block)
end
end
end
Expand Down
20 changes: 17 additions & 3 deletions activerecord/test/cases/relation/select_test.rb
Expand Up @@ -25,19 +25,33 @@ def test_reselect_with_default_scope_select
assert_equal expected, actual
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
end

def test_aliased_select_using_as_with_joins_and_includes
posts = Post.select("posts.id AS field_alias").joins(:comments).includes(:comments)
assert_includes posts.first.attributes, "field_alias"
assert_equal %w(
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

def test_aliased_select_not_using_as_with_joins_and_includes
posts = Post.select("posts.id field_alias").joins(:comments).includes(:comments)
assert_includes posts.first.attributes, "field_alias"
assert_equal %w(
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

def test_star_select_with_joins_and_includes
posts = Post.select("posts.*").joins(:comments).includes(:comments)
assert_not_includes posts.first.attributes, "posts.*"
assert_equal %w(
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
), posts.first.attributes.keys
end
end
end

0 comments on commit 090ec3d

Please sign in to comment.