Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pluck and select with from if from has original table name #35360

Merged
merged 2 commits into from Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion activerecord/lib/active_record/relation/query_methods.rb
Expand Up @@ -1068,8 +1068,10 @@ def arel_columns(columns)

def arel_column(field)
field = klass.attribute_alias(field) if klass.attribute_alias?(field)
from = from_clause.name || from_clause.value

if klass.columns_hash.key?(field) && !from_clause.value
if klass.columns_hash.key?(field) &&
(!from || from == table.name || from == connection.quote_table_name(table.name))
arel_attribute(field)
else
yield
Expand Down
39 changes: 39 additions & 0 deletions activerecord/test/cases/relations_test.rb
Expand Up @@ -182,6 +182,45 @@ def test_finding_with_subquery_without_select_does_not_change_the_select
end
end

def test_select_with_original_table_name_in_from
relation = Comment.joins(:post).select(:id).order(:id)
subquery = Comment.from(Comment.table_name).joins(:post).select(:id).order(:id)
assert_equal relation.map(&:id), subquery.map(&:id)
end

def test_pluck_with_original_table_name_in_from
relation = Comment.joins(:post).order(:id)
subquery = Comment.from(Comment.table_name).joins(:post).order(:id)
assert_equal relation.pluck(:id), subquery.pluck(:id)
end

def test_select_with_quoted_original_table_name_in_from
relation = Comment.joins(:post).select(:id).order(:id)
subquery = Comment.from(Comment.quoted_table_name).joins(:post).select(:id).order(:id)
assert_equal relation.map(&:id), subquery.map(&:id)
end

def test_pluck_with_quoted_original_table_name_in_from
relation = Comment.joins(:post).order(:id)
subquery = Comment.from(Comment.quoted_table_name).joins(:post).order(:id)
assert_equal relation.pluck(:id), subquery.pluck(:id)
end

def test_select_with_subquery_in_from_uses_original_table_name
if current_adapter?(:SQLite3Adapter) && ENV["TRAVIS"]
skip "https://travis-ci.org/rails/rails/jobs/496726410#L1198-L1208"
end
relation = Comment.joins(:post).select(:id).order(:id)
subquery = Comment.from(Comment.all, Comment.quoted_table_name).joins(:post).select(:id).order(:id)
assert_equal relation.map(&:id), subquery.map(&:id)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird... This test isn't failed locally... I suspect a bug on SQLite3...

https://travis-ci.org/rails/rails/jobs/496726410#L1198-L1208

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did fail on buildkite as well https://buildkite.com/rails/rails/builds/58981

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah... skipped on buildkite as well for now a333ba3.

end

def test_pluck_with_subquery_in_from_uses_original_table_name
relation = Comment.joins(:post).order(:id)
subquery = Comment.from(Comment.all, Comment.quoted_table_name).joins(:post).order(:id)
assert_equal relation.pluck(:id), subquery.pluck(:id)
end

def test_select_with_subquery_in_from_does_not_use_original_table_name
relation = Comment.group(:type).select("COUNT(post_id) AS post_count, type")
subquery = Comment.from(relation).select("type", "post_count")
Expand Down