Enable join table references to stay retryable

Previously, symbol references to columns on a join table would make
queries non-retryable due to two reasons. First, `#arel_column` wraps
symbols in a non-retryable `SqlLiteral` if they aren't attributes on the
primary model. Second, `#group` would wrap retryable `SqlLiteral`s in a
non-retryable `SqlLiteral` (because `SqlLiteral` is a subclass of
`String`).

This commit fixes both of these issues so that symbol references to join
table columns do not make a query non-retryable. `#arel_column` now
marks `SqlLiteral`s wrapping a symbol as retryable (since the symbols
are quoted they should be safe to retry). Additionally, `#group` now
doesn't re-wrap `SqlLiteral`s so passed in `SqlLiteral`s maintain their
retryable state.

Author: skipkayhil

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"