Skip to content

Commit 0fbf30d

Browse files
committed
Fix count queries on includes+references for models with composite primary keys
1 parent 2c35684 commit 0fbf30d

File tree

2 files changed

+25
-3
lines changed

2 files changed

+25
-3
lines changed

activerecord/lib/active_record/relation/calculations.rb

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ def calculate(operation, column_name)
234234
if operation == "count"
235235
unless distinct_value || distinct_select?(column_name || select_for_count)
236236
relation.distinct!
237-
relation.select_values = [ klass.primary_key || table[Arel.star] ]
237+
relation.select_values = Array(klass.primary_key || table[Arel.star])
238238
end
239239
# PostgreSQL: ORDER BY expressions must appear in SELECT list when using DISTINCT
240240
relation.order_values = [] if group_values.empty?
@@ -459,7 +459,7 @@ def operation_over_aggregate_column(column, operation, distinct)
459459
end
460460

461461
def execute_simple_calculation(operation, column_name, distinct) # :nodoc:
462-
if operation == "count" && (column_name == :all && distinct || has_limit_or_offset?)
462+
if build_count_subquery?(operation, column_name, distinct)
463463
# Shortcut when limit is zero.
464464
return 0 if limit_value == 0
465465

@@ -632,12 +632,30 @@ def type_cast_calculated_value(value, operation, type)
632632
def select_for_count
633633
if select_values.present?
634634
return select_values.first if select_values.one?
635-
select_values.join(", ")
635+
636+
select_values.map do |field|
637+
column = arel_column(field.to_s) do |attr_name|
638+
Arel.sql(attr_name)
639+
end
640+
641+
if column.is_a?(Arel::Nodes::SqlLiteral)
642+
column
643+
else
644+
"#{adapter_class.quote_table_name(column.relation.name)}.#{adapter_class.quote_column_name(column.name)}"
645+
end
646+
end.join(", ")
636647
else
637648
:all
638649
end
639650
end
640651

652+
def build_count_subquery?(operation, column_name, distinct)
653+
# SQLite and older MySQL does not support `COUNT DISTINCT` with `*` or
654+
# multiple columns, so we need to use subquery for this.
655+
operation == "count" &&
656+
(((column_name == :all || select_values.many?) && distinct) || has_limit_or_offset?)
657+
end
658+
641659
def build_count_subquery(relation, column_name, distinct)
642660
if column_name == :all
643661
column_alias = Arel.star

activerecord/test/cases/calculations_test.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,10 @@ def test_group_by_count_for_a_composite_primary_key_model
403403
assert_equal(expected, Cpk::Book.where(author_id: book.author_id).group(:author_id).count)
404404
end
405405

406+
def test_count_for_a_composite_primary_key_model_with_includes_and_references
407+
assert_equal Cpk::Book.count, Cpk::Book.includes(:chapters).references(:chapters).count
408+
end
409+
406410
def test_should_group_by_summed_field_having_condition
407411
c = Account.group(:firm_id).having("sum(credit_limit) > 50").sum(:credit_limit)
408412
assert_nil c[1]

0 commit comments

Comments
 (0)