Merge pull request #512 from pivotalneutron/fix_eager_load_with_calculations

Bug fixes for calculations with includes

activerecord/lib/active_record/relation/calculations.rb

@@ -146,7 +146,7 @@ def calculate(operation, column_name, options = {})
       if options.except(:distinct).present?
         apply_finder_options(options.except(:distinct)).calculate(operation, column_name, :distinct => options[:distinct])
       else
-        if eager_loading? || includes_values.present?
+        if eager_loading? || (includes_values.present? && references_eager_loaded_tables?)
           construct_relation_for_association_calculations.calculate(operation, column_name, options)
         else
           perform_calculation(operation, column_name, options)
@@ -161,21 +161,20 @@ def calculate(operation, column_name, options = {})
     def perform_calculation(operation, column_name, options = {})
       operation = operation.to_s.downcase
 
-      distinct = nil
+      distinct = options[:distinct]
 
       if operation == "count"
         column_name ||= (select_for_count || :all)
 
         unless arel.ast.grep(Arel::Nodes::OuterJoin).empty?
           distinct = true
-          column_name = primary_key if column_name == :all
         end
 
+        column_name = primary_key if column_name == :all && distinct
+
         distinct = nil if column_name =~ /\s*DISTINCT\s+/i
       end
 
-      distinct = options[:distinct] || distinct
-
       if @group_values.any?
         execute_grouped_calculation(operation, column_name, distinct)
       else

activerecord/test/cases/associations/cascaded_eager_loading_test.rb

@@ -51,7 +51,9 @@ def test_cascaded_eager_association_loading_with_join_for_count
     categories = Category.joins(:categorizations).includes([{:posts=>:comments}, :authors])
 
     assert_nothing_raised do
-      assert_equal 3, categories.count
+      assert_equal 4, categories.count
+      assert_equal 4, categories.all.count
+      assert_equal 3, categories.count(:distinct => true)
       assert_equal 3, categories.all.uniq.size # Must uniq since instantiating with inner joins will get dupes
     end
   end

activerecord/test/cases/calculations_test.rb

@@ -319,6 +319,17 @@ def test_should_count_selected_field_with_include
     assert_equal 4, Account.count(:distinct => true, :include => :firm, :select => :credit_limit)
   end
 
+  def test_should_not_perform_joined_include_by_default
+    assert_equal Account.count, Account.includes(:firm).count
+    queries = assert_sql { Account.includes(:firm).count }
+    assert_no_match(/join/i, queries.last)
+  end
+
+  def test_should_perform_joined_include_when_referencing_included_tables
+    joined_count = Account.includes(:firm).where(:companies => {:name => '37signals'}).count
+    assert_equal 1, joined_count
+  end
+
   def test_should_count_scoped_select
     Account.update_all("credit_limit = NULL")
     assert_equal 0, Account.scoped(:select => "credit_limit").count