Merge pull request rails#54823 from skipkayhil/hm-retry-join-symbols

Enable join table references to stay retryable

Author: byroot

activerecord/lib/active_record/relation/query_methods.rb

@@ -1996,8 +1996,10 @@ def arel_column(field)
           yield field
         elsif Arel.arel_node?(field)
           field
+        elsif is_symbol
+          Arel.sql(model.adapter_class.quote_table_name(field), retryable: true)
         else
-          Arel.sql(is_symbol ? model.adapter_class.quote_table_name(field) : field)
+          Arel.sql(field)
         end
       end
 

activerecord/lib/arel/select_manager.rb

@@ -74,8 +74,13 @@ def on(*exprs)
     def group(*columns)
       columns.each do |column|
         # FIXME: backwards compat
-        column = Nodes::SqlLiteral.new(column) if String === column
-        column = Nodes::SqlLiteral.new(column.to_s) if Symbol === column
+        case column
+        when Nodes::SqlLiteral
+        when String
+          column = Nodes::SqlLiteral.new(column)
+        when Symbol
+          column = Nodes::SqlLiteral.new(column.name)
+        end
 
         @ctx.groups.push Nodes::Group.new column
       end

activerecord/test/cases/adapter_test.rb

@@ -708,12 +708,13 @@ def teardown
           assert Post.find_by(title: "Welcome to the weblog")
           assert_predicate Post, :exists?
           a.books.to_a
+          Author.select(:status).joins(:books).group(:status).to_a
         end.select { |n| n.payload[:name] != "SCHEMA" }
 
-        assert_equal 6, notifications.length
+        assert_equal 7, notifications.length
 
         notifications.each do |n|
-          assert n.payload[:allow_retry]
+          assert n.payload[:allow_retry], "#{n.payload[:sql]} was not retryable"
         end
       end
 
@@ -727,9 +728,10 @@ def teardown
           assert_not_nil Post.find_by(title: "Welcome to the weblog")
           assert_predicate Post, :exists?
           a.books.to_a
+          Author.select(:status).joins(:books).group(:status).to_a
         end.select { |n| n.payload[:name] != "SCHEMA" }
 
-        assert_equal 6, notifications.length
+        assert_equal 7, notifications.length
 
         notifications.each do |n|
           assert n.payload[:allow_retry], "#{n.payload[:sql]} was not retryable"