From 4251662c1a4dbec8c69d4ab1b677950a0b4cfb58 Mon Sep 17 00:00:00 2001 From: Rick Olson Date: Tue, 25 Apr 2006 05:25:04 +0000 Subject: Allow all calculations to take the :include option, not just COUNT (closes #4840) [Rick] git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4264 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/CHANGELOG | 2 + activerecord/lib/active_record/associations.rb | 33 ----------- activerecord/lib/active_record/calculations.rb | 77 ++++++++++++++------------ activerecord/test/calculations_test.rb | 11 +++- 4 files changed, 55 insertions(+), 68 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 0f331f3d9a..296070b865 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Allow all calculations to take the :include option, not just COUNT (closes #4840) [Rick] + * Update inconsistent migrations documentation. #4683 [machomagna@gmail.com] * Add ActiveRecord::Errors#to_xml [Jamis Buck] diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index c0fdca3da9..4a637d5dab 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -966,14 +966,6 @@ module ActiveRecord end end - def count_with_associations(options = {}) - catch :invalid_query do - join_dependency = JoinDependency.new(self, merge_includes(scope(:find, :include), options[:include]), options[:joins]) - return count_by_sql(construct_counter_sql_with_included_associations(options, join_dependency)) - end - 0 - end - def find_with_associations(options = {}) catch :invalid_query do join_dependency = JoinDependency.new(self, merge_includes(scope(:find, :include), options[:include]), options[:joins]) @@ -1117,31 +1109,6 @@ module ActiveRecord "#{name} Load Including Associations" ) end - - def construct_counter_sql_with_included_associations(options, join_dependency) - scope = scope(:find) - sql = "SELECT COUNT(DISTINCT #{table_name}.#{primary_key})" - - # A (slower) workaround if we're using a backend, like sqlite, that doesn't support COUNT DISTINCT. - if !Base.connection.supports_count_distinct? - sql = "SELECT COUNT(*) FROM (SELECT DISTINCT #{table_name}.#{primary_key}" - end - - sql << " FROM #{table_name} " - sql << join_dependency.join_associations.collect{|join| join.association_join }.join - - add_joins!(sql, options, scope) - add_conditions!(sql, options[:conditions], scope) - add_limited_ids_condition!(sql, options, join_dependency) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) - - add_limit!(sql, options, scope) if using_limitable_reflections?(join_dependency.reflections) - - if !Base.connection.supports_count_distinct? - sql << ")" - end - - return sanitize_sql(sql) - end def construct_finder_sql_with_included_associations(options, join_dependency) scope = scope(:find) diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index acab77870f..9a747f661b 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -1,6 +1,6 @@ module ActiveRecord module Calculations #:nodoc: - CALCULATIONS_OPTIONS = [:conditions, :joins, :order, :select, :group, :having, :distinct, :limit, :offset] + CALCULATIONS_OPTIONS = [:conditions, :joins, :order, :select, :group, :having, :distinct, :limit, :offset, :include] def self.included(base) base.extend(ClassMethods) end @@ -42,13 +42,7 @@ module ActiveRecord # # Note: Person.count(:all) will not work because it will use :all as the condition. Use Person.count instead. def count(*args) - column_name, options = construct_count_options_from_legacy_args(*args) - - if options[:include] || scope(:find, :include) - count_with_associations(options) - else - calculate(:count, column_name, options) - end + calculate(:count, *construct_count_options_from_legacy_args(*args)) end # Calculates average value on a given column. The value is returned as a float. See #calculate for examples with options. @@ -120,13 +114,14 @@ module ActiveRecord column_name = options[:select] if options[:select] column_name = '*' if column_name == :all column = column_for column_name - aggregate = select_aggregate(operation, column_name, options) - aggregate_alias = column_alias_for(operation, column_name) - if options[:group] - execute_grouped_calculation(operation, column_name, column, aggregate, aggregate_alias, options) - else - execute_simple_calculation(operation, column_name, column, aggregate, aggregate_alias, options) + catch :invalid_query do + if options[:group] + return execute_grouped_calculation(operation, column_name, column, options) + else + return execute_simple_calculation(operation, column_name, column, options) + end end + 0 end protected @@ -143,7 +138,7 @@ module ActiveRecord else # Handle legacy paramter options: def count(conditions=nil, joins=nil) options.merge!(:conditions => args[0]) if args.length > 0 - options.merge!(:joins => args[1]) if args.length > 1 + options.merge!(:joins => args[1]) if args.length > 1 end else raise(ArgumentError, "Unexpected parameters passed to count(*args): expected either count(conditions=nil, joins=nil) or count(options={})") @@ -151,34 +146,56 @@ module ActiveRecord [column_name, options] end - def construct_calculation_sql(aggregate, aggregate_alias, options) #:nodoc: - scope = scope(:find) - sql = "SELECT #{aggregate} AS #{aggregate_alias}" + def construct_calculation_sql(operation, column_name, options) #:nodoc: + scope = scope(:find) + merged_includes = merge_includes(scope ? scope[:include] : [], options[:include]) + aggregate_alias = column_alias_for(operation, column_name) + use_workaround = !Base.connection.supports_count_distinct? && options[:distinct] && operation.to_s.downcase == 'count' + join_dependency = nil + + if merged_includes.any? && operation.to_s.downcase == 'count' + options[:distinct] = true + column_name = [table_name, primary_key] * '.' + end + + sql = "SELECT #{operation}(#{'DISTINCT ' if options[:distinct]}#{column_name}) AS #{aggregate_alias}" + + # A (slower) workaround if we're using a backend, like sqlite, that doesn't support COUNT DISTINCT. + sql = "SELECT COUNT(*) AS #{aggregate_alias}" if use_workaround + sql << ", #{options[:group_field]} AS #{options[:group_alias]}" if options[:group] + sql << " FROM (SELECT DISTINCT #{column_name}" if use_workaround sql << " FROM #{table_name} " + if merged_includes.any? + join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, options[:joins]) + sql << join_dependency.join_associations.collect{|join| join.association_join }.join + end add_joins!(sql, options, scope) add_conditions!(sql, options[:conditions], scope) sql << " GROUP BY #{options[:group_field]}" if options[:group] - sql << " HAVING #{options[:having]}" if options[:group] && options[:having] - sql << " ORDER BY #{options[:order]}" if options[:order] - add_limit!(sql, options) + sql << " HAVING #{options[:having]}" if options[:group] && options[:having] + sql << " ORDER BY #{options[:order]}" if options[:order] + add_limited_ids_condition!(sql, options, join_dependency) if join_dependency && !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) + add_limit!(sql, options, scope) + sql << ')' if use_workaround sql end - def execute_simple_calculation(operation, column_name, column, aggregate, aggregate_alias, options) #:nodoc: - value = connection.select_value(construct_calculation_sql(aggregate, aggregate_alias, options)) + def execute_simple_calculation(operation, column_name, column, options) #:nodoc: + value = connection.select_value(construct_calculation_sql(operation, column_name, options)) type_cast_calculated_value(value, column, operation) end - def execute_grouped_calculation(operation, column_name, column, aggregate, aggregate_alias, options) #:nodoc: + def execute_grouped_calculation(operation, column_name, column, options) #:nodoc: group_attr = options[:group].to_s association = reflect_on_association(group_attr.to_sym) associated = association && association.macro == :belongs_to # only count belongs_to associations group_field = (associated ? "#{options[:group]}_id" : options[:group]).to_s group_alias = column_alias_for(group_field) group_column = column_for group_field - sql = construct_calculation_sql(aggregate, aggregate_alias, options.merge(:group_field => group_field, :group_alias => group_alias)) + sql = construct_calculation_sql(operation, column_name, options.merge(:group_field => group_field, :group_alias => group_alias)) calculated_data = connection.select_all(sql) + aggregate_alias = column_alias_for(operation, column_name) if association key_ids = calculated_data.collect { |row| row[group_alias] } @@ -195,15 +212,7 @@ module ActiveRecord private def validate_calculation_options(operation, options = {}) - if operation.to_s == 'count' - options.assert_valid_keys(CALCULATIONS_OPTIONS + [:include]) - else - options.assert_valid_keys(CALCULATIONS_OPTIONS) - end - end - - def select_aggregate(operation, column_name, options) - "#{operation}(#{'DISTINCT ' if options[:distinct]}#{column_name})" + options.assert_valid_keys(CALCULATIONS_OPTIONS) end # converts a given key to the value that the database adapter returns as diff --git a/activerecord/test/calculations_test.rb b/activerecord/test/calculations_test.rb index fbf830e9e6..c4e41b1dda 100644 --- a/activerecord/test/calculations_test.rb +++ b/activerecord/test/calculations_test.rb @@ -21,6 +21,16 @@ class CalculationsTest < Test::Unit::TestCase assert_equal 60, Account.maximum(:credit_limit) end + def test_should_get_maximum_of_field_with_include + assert_equal 50, Account.maximum(:credit_limit, :include => :firm, :conditions => "companies.name != 'Summit'") + end + + def test_should_get_maximum_of_field_with_scoped_include + Account.with_scope :find => { :include => :firm, :conditions => "companies.name != 'Summit'" } do + assert_equal 50, Account.maximum(:credit_limit) + end + end + def test_should_get_minimum_of_field assert_equal 50, Account.minimum(:credit_limit) end @@ -174,7 +184,6 @@ class CalculationsTest < Test::Unit::TestCase Company.send(:validate_calculation_options, :count, :include => true) end - assert_raises(ArgumentError) { Company.send(:validate_calculation_options, :sum, :include => :posts) } assert_raises(ArgumentError) { Company.send(:validate_calculation_options, :sum, :foo => :bar) } assert_raises(ArgumentError) { Company.send(:validate_calculation_options, :count, :foo => :bar) } end -- cgit v1.2.3