Skip to content

Commit bf1494a

Browse files
committed
Fix GROUP BY with calculate longer name field to respect table_alias_length
Follow up of c9e4c84.
1 parent 57c7cbb commit bf1494a

File tree

6 files changed

+30
-11
lines changed

6 files changed

+30
-11
lines changed

activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,24 @@
55
module ActiveRecord
66
module ConnectionAdapters # :nodoc:
77
module DatabaseLimits
8+
def max_identifier_length # :nodoc:
9+
64
10+
end
11+
812
# Returns the maximum length of a table alias.
913
def table_alias_length
10-
255
14+
max_identifier_length
1115
end
1216

1317
# Returns the maximum length of a column name.
1418
def column_name_length
15-
64
19+
max_identifier_length
1620
end
1721
deprecate :column_name_length
1822

1923
# Returns the maximum length of a table name.
2024
def table_name_length
21-
64
25+
max_identifier_length
2226
end
2327
deprecate :table_name_length
2428

@@ -33,7 +37,7 @@ def allowed_index_name_length
3337

3438
# Returns the maximum length of an index name.
3539
def index_name_length
36-
64
40+
max_identifier_length
3741
end
3842

3943
# Returns the maximum number of columns per table.

activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,10 @@ def type_to_sql(type, limit: nil, precision: nil, scale: nil, size: limit_to_siz
121121
sql
122122
end
123123

124+
def table_alias_length
125+
256 # https://dev.mysql.com/doc/refman/8.0/en/identifiers.html
126+
end
127+
124128
private
125129
CHARSETS_OF_4BYTES_MAXLEN = ["utf8mb4", "utf16", "utf16le", "utf32"]
126130

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,8 +400,6 @@ def extensions
400400
def max_identifier_length
401401
@max_identifier_length ||= query_value("SHOW max_identifier_length", "SCHEMA").to_i
402402
end
403-
alias table_alias_length max_identifier_length
404-
alias index_name_length max_identifier_length
405403

406404
# Set the authorized user for this session
407405
def session_auth=(user)

activerecord/lib/active_record/relation/calculations.rb

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -319,12 +319,11 @@ def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
319319

320320
group_aliases = group_fields.map { |field|
321321
field = connection.visitor.compile(field) if Arel.arel_node?(field)
322-
column_alias_for(field)
322+
column_alias_for(field.to_s.downcase)
323323
}
324324
group_columns = group_aliases.zip(group_fields)
325325

326-
aggregate_alias = "#{operation}_#{column_name.to_s.downcase}"
327-
aggregate_alias = column_alias_for(aggregate_alias) unless aggregate_alias.match?(/\A\w+\z/)
326+
aggregate_alias = column_alias_for("#{operation}_#{column_name.to_s.downcase}")
328327

329328
select_values = [
330329
operation_over_aggregate_column(
@@ -369,15 +368,17 @@ def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
369368
end]
370369
end
371370

372-
# Converts the given keys to the value that the database adapter returns as
371+
# Converts the given field to the value that the database adapter returns as
373372
# a usable column name:
374373
#
375374
# column_alias_for("users.id") # => "users_id"
376375
# column_alias_for("sum(id)") # => "sum_id"
377376
# column_alias_for("count(distinct users.id)") # => "count_distinct_users_id"
378377
# column_alias_for("count(*)") # => "count_all"
379378
def column_alias_for(field)
380-
column_alias = field.to_s.downcase
379+
return field if field.match?(/\A\w{,#{connection.table_alias_length}}\z/)
380+
381+
column_alias = +field
381382
column_alias.gsub!(/\*/, "all")
382383
column_alias.gsub!(/\W+/, " ")
383384
column_alias.strip!

activerecord/test/cases/calculations_test.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,17 @@ def test_should_group_by_fields_with_table_alias
363363
assert_equal 60, c[2]
364364
end
365365

366+
def test_should_calculate_grouped_with_longer_field
367+
field = "a" * Account.connection.max_identifier_length
368+
369+
Account.update_all("#{field} = credit_limit")
370+
371+
c = Account.group(:firm_id).sum(field)
372+
assert_equal 50, c[1]
373+
assert_equal 105, c[6]
374+
assert_equal 60, c[2]
375+
end
376+
366377
def test_should_calculate_with_invalid_field
367378
assert_equal 6, Account.calculate(:count, "*")
368379
assert_equal 6, Account.calculate(:count, :all)

activerecord/test/schema/schema.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
t.references :firm, index: false
2020
t.string :firm_name
2121
t.integer :credit_limit
22+
t.integer "a" * max_identifier_length
2223
end
2324

2425
create_table :admin_accounts, force: true do |t|

0 commit comments

Comments
 (0)