Permalink
Browse files

Merge pull request #512 from pivotalneutron/fix_eager_load_with_calcu…

…lations

Bug fixes for calculations with includes
  • Loading branch information...
2 parents c53d392 + 1db49ce commit d192d85668e967547a20cdbd0a0f40a8477b272a @jonleighton jonleighton committed May 11, 2011
@@ -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
@@ -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
@@ -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

0 comments on commit d192d85

Please sign in to comment.