From bff5e799c056a1914653ae53166949b4adff3d4b Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 1 Mar 2019 21:27:47 +0900
Subject: [PATCH] Relax table name detection in `from` to allow any extension
 like INDEX hint

#35360 allows table name qualified if `from` has original table name.
But that is still too strict. We have a valid use case that `from` with
INDEX hint (e.g. `from("comments USE INDEX (PRIMARY)")`).
So I've relaxed the table name detection in `from` to allow any
extension like INDEX hint.

Fixes #35359.
---
 .../active_record/relation/query_methods.rb   |  7 ++--
 activerecord/test/cases/relations_test.rb     | 32 +++++++++++++------
 2 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index f88493df8a61a..24a50db619bf8 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -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
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index a7f09e6de05d1..85719cb8d0f3d 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -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
 
@@ -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