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

Relax table name detection in from to allow any extension like INDEX hint #35441

Merged
merged 1 commit into from Mar 1, 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
7 changes: 5 additions & 2 deletions activerecord/lib/active_record/relation/query_methods.rb
Expand Up @@ -1091,14 +1091,17 @@ 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 || from == table.name || from == connection.quote_table_name(table.name))
if klass.columns_hash.key?(field) && (!from || table_name_matches?(from))
arel_attribute(field)
else
yield
end
end

def table_name_matches?(from)
/(?:\A|(?<!FROM)\s)(?:\b#{table.name}\b|#{connection.quote_table_name(table.name)})(?!\.)/i.match?(from.to_s)
end

def reverse_sql_order(order_query)
if order_query.empty?
return [arel_attribute(primary_key).desc] if primary_key
Expand Down
32 changes: 22 additions & 10 deletions activerecord/test/cases/relations_test.rb
Expand Up @@ -182,27 +182,27 @@ def test_finding_with_subquery_without_select_does_not_change_the_select
end
end

def test_select_with_original_table_name_in_from
def test_select_with_from_includes_original_table_name
relation = Comment.joins(:post).select(:id).order(:id)
subquery = Comment.from(Comment.table_name).joins(:post).select(:id).order(:id)
subquery = Comment.from("#{Comment.table_name} /*! USE INDEX (PRIMARY) */").joins(:post).select(:id).order(:id)
assert_equal relation.map(&:id), subquery.map(&:id)
end

def test_pluck_with_original_table_name_in_from
def test_pluck_with_from_includes_original_table_name
relation = Comment.joins(:post).order(:id)
subquery = Comment.from(Comment.table_name).joins(:post).order(:id)
subquery = Comment.from("#{Comment.table_name} /*! USE INDEX (PRIMARY) */").joins(:post).order(:id)
assert_equal relation.pluck(:id), subquery.pluck(:id)
end

def test_select_with_quoted_original_table_name_in_from
def test_select_with_from_includes_quoted_original_table_name
relation = Comment.joins(:post).select(:id).order(:id)
subquery = Comment.from(Comment.quoted_table_name).joins(:post).select(:id).order(:id)
subquery = Comment.from("#{Comment.quoted_table_name} /*! USE INDEX (PRIMARY) */").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
def test_pluck_with_from_includes_quoted_original_table_name
relation = Comment.joins(:post).order(:id)
subquery = Comment.from(Comment.quoted_table_name).joins(:post).order(:id)
subquery = Comment.from("#{Comment.quoted_table_name} /*! USE INDEX (PRIMARY) */").joins(:post).order(:id)
assert_equal relation.pluck(:id), subquery.pluck(:id)
end

Expand All @@ -221,13 +221,25 @@ def test_pluck_with_subquery_in_from_uses_original_table_name

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")
subquery = Comment.from(relation, "grouped_#{Comment.table_name}").select("type", "post_count")
assert_equal(relation.map(&:post_count).sort, subquery.map(&:post_count).sort)
end

def test_group_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).group("type").average("post_count")
subquery = Comment.from(relation, "grouped_#{Comment.table_name}").group("type").average("post_count")
assert_equal(relation.map(&:post_count).sort, subquery.values.sort)
end

def test_select_with_subquery_string_in_from_does_not_use_original_table_name
relation = Comment.group(:type).select("COUNT(post_id) AS post_count, type")
subquery = Comment.from("(#{relation.to_sql}) #{Comment.table_name}_grouped").select("type", "post_count")
assert_equal(relation.map(&:post_count).sort, subquery.map(&:post_count).sort)
end

def test_group_with_subquery_string_in_from_does_not_use_original_table_name
relation = Comment.group(:type).select("COUNT(post_id) AS post_count,type")
subquery = Comment.from("(#{relation.to_sql}) #{Comment.table_name}_grouped").group("type").average("post_count")
assert_equal(relation.map(&:post_count).sort, subquery.values.sort)
end

Expand Down