diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index 8a44dc7df1..fc6cb793ab 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -2,8 +2,6 @@ module ActiveRecord module Calculations #:nodoc: extend ActiveSupport::Concern - CALCULATIONS_OPTIONS = [:conditions, :joins, :order, :select, :group, :having, :distinct, :limit, :offset, :include, :from] - module ClassMethods # Count operates using three different approaches. # @@ -147,26 +145,11 @@ module ActiveRecord end private - def validate_calculation_options(options = {}) - options.assert_valid_keys(CALCULATIONS_OPTIONS) - end - def construct_calculation_arel(options = {}) - validate_calculation_options(options) - options = options.except(:distinct) - - merge_with_includes = current_scoped_methods ? current_scoped_methods.includes_values : [] - includes = (merge_with_includes + Array.wrap(options[:include])).uniq - - if includes.any? - merge_with_joins = current_scoped_methods ? current_scoped_methods.joins_values : [] - joins = (merge_with_joins + Array.wrap(options[:joins])).uniq - join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, includes, construct_join(joins)) - construct_finder_arel_with_included_associations(options, join_dependency) - else - scoped.apply_finder_options(options) - end - end + def construct_calculation_arel(options = {}) + relation = scoped.apply_finder_options(options.except(:distinct)) + (relation.eager_loading? || relation.includes_values.present?) ? relation.send(:construct_relation_for_association_calculations) : relation + end end end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index e37e692a97..19f91f4278 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -45,12 +45,10 @@ module ActiveRecord def to_a return @records if loaded? - eager_loading = @eager_load_values.any? || (@includes_values.any? && references_eager_loaded_tables?) - - @records = eager_loading ? find_with_associations : @klass.find_by_sql(arel.to_sql) + @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel.to_sql) preload = @preload_values - preload += @includes_values unless eager_loading + preload += @includes_values unless eager_loading? preload.each {|associations| @klass.send(:preload_associations, @records, associations) } # @readonly_value is true only if set explicity. @implicit_readonly is true if there are JOINS and no explicit SELECT. @@ -112,6 +110,7 @@ module ActiveRecord def reset @first = @last = @to_sql = @order_clause = @scope_for_create = @arel = @loaded = nil + @should_eager_load = @join_dependency = nil @records = [] self end @@ -133,6 +132,10 @@ module ActiveRecord end end + def eager_loading? + @should_eager_load ||= (@eager_load_values.any? || (@includes_values.any? && references_eager_loaded_tables?)) + end + protected def method_missing(method, *args, &block) diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 980c5796f3..c48c8fe828 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -53,6 +53,12 @@ module ActiveRecord [] end + def construct_relation_for_association_calculations + including = (@eager_load_values + @includes_values).uniq + join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(@klass, including, arel.joins(arel)) + construct_relation_for_association_find(join_dependency) + end + def construct_relation_for_association_find(join_dependency) relation = except(:includes, :eager_load, :preload, :select).select(@klass.send(:column_aliases, join_dependency)) diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index bd2d471fc7..c3b2e56387 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -246,23 +246,6 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal 8, c['Jadedpixel'] end - def test_should_reject_invalid_options - assert_nothing_raised do - # empty options are valid - Company.send(:validate_calculation_options) - # these options are valid for all calculations - [:select, :conditions, :joins, :order, :group, :having, :distinct].each do |opt| - Company.send(:validate_calculation_options, opt => true) - end - - # :include is only valid on :count - Company.send(:validate_calculation_options, :include => true) - end - - assert_raise(ArgumentError) { Company.send(:validate_calculation_options, :sum, :foo => :bar) } - assert_raise(ArgumentError) { Company.send(:validate_calculation_options, :count, :foo => :bar) } - end - def test_should_count_selected_field_with_include assert_equal 6, Account.count(:distinct => true, :include => :firm) assert_equal 4, Account.count(:distinct => true, :include => :firm, :select => :credit_limit)