Fix a bug where using groups and counts with long table names would return incorrect results.

Fixed a bug that caused the alias name of "group by" to be too long and the first half of the name would be the same in both cases if it was cut by max identifier length.

Fix #46285

Co-authored-by: Yusaku ONO <yono@users.noreply.github.com>

activerecord/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Fix a bug where using groups and counts with long table names would return incorrect results.
+
+    *Shota Toguchi, Yusaku Ono*
+
 *   Fix encryption of column default values.
 
     Previously, encrypted attributes that used column default values appeared to

activerecord/lib/active_record/relation/calculations.rb

@@ -4,6 +4,47 @@ require "active_support/core_ext/enumerable"
 
 module ActiveRecord
   module Calculations
+    class ColumnAliasTracker # :nodoc:
+      def initialize(connection)
+        @connection = connection
+        @aliases = Hash.new(0)
+      end
+
+      def alias_for(field)
+        aliased_name = column_alias_for(field)
+
+        if @aliases[aliased_name] == 0
+          @aliases[aliased_name] = 1
+          aliased_name
+        else
+          # Update the count
+          count = @aliases[aliased_name] += 1
+          "#{truncate(aliased_name)}_#{count}"
+        end
+      end
+
+      private
+        # Converts the given field to the value that the database adapter returns as
+        # a usable column name:
+        #
+        #   column_alias_for("users.id")                 # => "users_id"
+        #   column_alias_for("sum(id)")                  # => "sum_id"
+        #   column_alias_for("count(distinct users.id)") # => "count_distinct_users_id"
+        #   column_alias_for("count(*)")                 # => "count_all"
+        def column_alias_for(field)
+          column_alias = +field
+          column_alias.gsub!(/\*/, "all")
+          column_alias.gsub!(/\W+/, " ")
+          column_alias.strip!
+          column_alias.gsub!(/ +/, "_")
+          @connection.table_alias_for(column_alias)
+        end
+
+        def truncate(name)
+          name.slice(0, @connection.table_alias_length - 2)
+        end
+    end
+
     # Count the records.
     #
     #   Person.count
@@ -386,14 +427,16 @@ module ActiveRecord
         end
         group_fields = arel_columns(group_fields)
 
+        column_alias_tracker = ColumnAliasTracker.new(connection)
+
         group_aliases = group_fields.map { |field|
           field = connection.visitor.compile(field) if Arel.arel_node?(field)
-          column_alias_for(field.to_s.downcase)
+          column_alias_tracker.alias_for(field.to_s.downcase)
         }
         group_columns = group_aliases.zip(group_fields)
 
         column = aggregate_column(column_name)
-        column_alias = column_alias_for("#{operation} #{column_name.to_s.downcase}")
+        column_alias = column_alias_tracker.alias_for("#{operation} #{column_name.to_s.downcase}")
         select_value = operation_over_aggregate_column(column, operation, distinct)
         select_value.as(connection.quote_column_name(column_alias))
 
@@ -449,23 +492,6 @@ module ActiveRecord
         end
       end
 
-      # Converts the given field to the value that the database adapter returns as
-      # a usable column name:
-      #
-      #   column_alias_for("users.id")                 # => "users_id"
-      #   column_alias_for("sum(id)")                  # => "sum_id"
-      #   column_alias_for("count(distinct users.id)") # => "count_distinct_users_id"
-      #   column_alias_for("count(*)")                 # => "count_all"
-      def column_alias_for(field)
-        column_alias = +field
-        column_alias.gsub!(/\*/, "all")
-        column_alias.gsub!(/\W+/, " ")
-        column_alias.strip!
-        column_alias.gsub!(/ +/, "_")
-
-        connection.table_alias_for(column_alias)
-      end
-
       def type_for(field, &block)
         field_name = field.respond_to?(:name) ? field.name.to_s : field.to_s.split(".").last
         @klass.type_for_attribute(field_name, &block)

activerecord/test/cases/calculations_test.rb

@@ -21,6 +21,7 @@ require "models/developer"
 require "models/post"
 require "models/comment"
 require "models/rating"
+require "models/too_long_table_name"
 require "support/stubs/strong_parameters"
 require "support/async_helper"
 
@@ -145,6 +146,19 @@ class CalculationsTest < ActiveRecord::TestCase
     [ [nil, 50], [1, 50], [6, 50], [6, 55], [9, 53], [2, 60] ].each { |firm_and_limit| assert_includes c.keys, firm_and_limit }
   end
 
+  def test_should_group_by_multiple_fields_when_table_name_is_too_long
+    2.times do
+      TooLongTableName.create!(
+        toooooooo_long_a_id: 1,
+        toooooooo_long_b_id: 2
+      )
+    end
+
+    res = TooLongTableName.group(:toooooooo_long_a_id, :toooooooo_long_b_id).count
+
+    assert_equal({ [1, 2] => 2 }, res)
+  end
+
   def test_should_group_by_multiple_fields_having_functions
     c = Topic.group(:author_name, "COALESCE(type, title)").count(:all)
     assert_equal 1, c[["Carl", "The Third Topic of the day"]]

activerecord/test/models/too_long_table_name.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class TooLongTableName < ActiveRecord::Base
+  self.table_name = "toooooooooooooooooooooooooooooooooo_long_table_names"
+end

activerecord/test/schema/schema.rb

@@ -1317,6 +1317,11 @@ ActiveRecord::Schema.define do
     t.integer :id
     t.datetime :created_at
   end
+
+  create_table :toooooooooooooooooooooooooooooooooo_long_table_names, force: true do |t|
+    t.bigint :toooooooo_long_a_id, null: false
+    t.bigint :toooooooo_long_b_id, null: false
+  end
 end
 
 Course.connection.create_table :courses, force: true do |t|