Skip to content
Browse files

Make sure the column_name is different from 'all'.

968c581 have fixed the EagerLoadTest, but
not in the correct way.

The problem was when `empty?` or `size` was called on relation. It was
triggering `count(:all)`, which was passing `:all` as the column name to `count`
on Calculations.

On the other hand, the method `calculate` on Calculations was calling
`construct_relation_for_association_calculations` instead of `perform_calculation`,
because `has_include?` was returning `true` since `column_name` was present.

To prevent calling the wrong method to perform the calculation, we have to check
if the `column_name` is present and if it is different from `:all` (which is now used
to correctly do `count` with `select`).

More information here: 968c581#commitcomment-6006135
  • Loading branch information...
1 parent 34945e4 commit a9416a407b3e5256a0a3fbe6c6ee5112ca3b6703 @laurocaetano laurocaetano committed Apr 14, 2014
Showing with 1 addition and 1 deletion.
  1. +1 −1 activerecord/lib/active_record/relation/calculations.rb
2 activerecord/lib/active_record/relation/calculations.rb
@@ -188,7 +188,7 @@ def ids
def has_include?(column_name)
- eager_loading? || (includes_values.present? && (column_name || references_eager_loaded_tables?))
+ eager_loading? || (includes_values.present? && ((column_name && column_name != :all) || references_eager_loaded_tables?))
def perform_calculation(operation, column_name, options = {})

0 comments on commit a9416a4

Please sign in to comment.
Something went wrong with that request. Please try again.