From 19d2ff83db5232a816dee201800baf3924705b31 Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Wed, 29 Apr 2009 19:39:53 -0300 Subject: Calculations now use Arel to construct the query. Implemented other methods in AR::Base with Arel support. --- activerecord/lib/active_record/associations.rb | 28 ++-- activerecord/lib/active_record/base.rb | 31 ++-- activerecord/lib/active_record/calculations.rb | 197 +++++++++++-------------- activerecord/lib/active_record/test_case.rb | 2 +- 4 files changed, 123 insertions(+), 135 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 2115878e32..2dd1197192 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -510,14 +510,14 @@ module ActiveRecord # # Since only one table is loaded at a time, conditions or orders cannot reference tables other than the main one. If this is the case # Active Record falls back to the previously used LEFT OUTER JOIN based strategy. For example - # + # # Post.find(:all, :include => [ :author, :comments ], :conditions => ['comments.approved = ?', true]) # # will result in a single SQL query with joins along the lines of: LEFT OUTER JOIN comments ON comments.post_id = posts.id and # LEFT OUTER JOIN authors ON authors.id = posts.author_id. Note that using conditions like this can have unintended consequences. # In the above example posts with no approved comments are not returned at all, because the conditions apply to the SQL statement as a whole # and not just to the association. You must disambiguate column references for this fallback to happen, for example - # :order => "author.name DESC" will work but :order => "name DESC" will not. + # :order => "author.name DESC" will work but :order => "name DESC" will not. # # If you do want eagerload only some members of an association it is usually more natural to :include an association # which has conditions defined on it: @@ -551,10 +551,10 @@ module ActiveRecord # # Address.find(:all, :include => :addressable) # - # will execute one query to load the addresses and load the addressables with one query per addressable type. + # will execute one query to load the addresses and load the addressables with one query per addressable type. # For example if all the addressables are either of class Person or Company then a total of 3 queries will be executed. The list of # addressable types to load is determined on the back of the addresses loaded. This is not supported if Active Record has to fallback - # to the previous implementation of eager loading and will raise ActiveRecord::EagerLoadPolymorphicError. The reason is that the parent + # to the previous implementation of eager loading and will raise ActiveRecord::EagerLoadPolymorphicError. The reason is that the parent # model's type is a column value so its corresponding table name cannot be put in the +FROM+/+JOIN+ clauses of that query. # # == Table Aliasing @@ -869,7 +869,7 @@ module ActiveRecord # but not include the joined columns. Do not forget to include the primary and foreign keys, otherwise it will raise an error. # [:through] # Specifies a Join Model through which to perform the query. Options for :class_name and :foreign_key - # are ignored, as the association uses the source reflection. You can only use a :through query through a + # are ignored, as the association uses the source reflection. You can only use a :through query through a # has_one or belongs_to association on the join model. # [:source] # Specifies the source association name used by has_one :through queries. Only use it if the name cannot be @@ -1123,8 +1123,8 @@ module ActiveRecord # the association will use "project_id" as the default :association_foreign_key. # [:conditions] # Specify the conditions that the associated object must meet in order to be included as a +WHERE+ - # SQL fragment, such as authorized = 1. Record creations from the association are scoped if a hash is used. - # has_many :posts, :conditions => {:published => true} will create published posts with @blog.posts.create + # SQL fragment, such as authorized = 1. Record creations from the association are scoped if a hash is used. + # has_many :posts, :conditions => {:published => true} will create published posts with @blog.posts.create # or @blog.posts.build. # [:order] # Specify the order in which the associated objects are returned as an ORDER BY SQL fragment, @@ -1335,12 +1335,12 @@ module ActiveRecord "#{reflection.class_name}.send(:attr_readonly,\"#{cache_column}\".intern) if defined?(#{reflection.class_name}) && #{reflection.class_name}.respond_to?(:attr_readonly)" ) end - + def add_touch_callbacks(reflection, touch_attribute) method_name = "belongs_to_touch_after_save_or_destroy_for_#{reflection.name}".to_sym define_method(method_name) do association = send(reflection.name) - + if touch_attribute == true association.touch unless association.nil? else @@ -1552,7 +1552,7 @@ module ActiveRecord options[:extend] = create_extension_modules(association_id, extension, options[:extend]) reflection = create_reflection(:has_and_belongs_to_many, association_id, options, self) - + if reflection.association_foreign_key == reflection.primary_key_name raise HasAndBelongsToManyAssociationForeignKeyNeeded.new(reflection) end @@ -1607,6 +1607,14 @@ module ActiveRecord end end + def construct_limited_ids_condition(where, options, join_dependency) + unless (id_list = select_limited_ids_list(options, join_dependency)).empty? + "#{where.blank? ? 'WHERE ' : ' AND '} #{connection.quote_table_name table_name}.#{primary_key} IN (#{id_list}) " + else + throw :invalid_query + end + end + def select_limited_ids_list(options, join_dependency) pk = columns_hash[primary_key] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 8bf92de6d4..8882a00dd5 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -904,9 +904,7 @@ module ActiveRecord #:nodoc: # Both calls delete the affected posts all at once with a single DELETE statement. If you need to destroy dependent # associations or call your before_* or +after_destroy+ callbacks, use the +destroy_all+ method instead. def delete_all(conditions = nil) - sql = "DELETE FROM #{quoted_table_name} " - add_conditions!(sql, conditions, scope(:find)) - connection.delete(sql, "#{name} Delete all") + Arel(table_name).where(construct_conditions(conditions, scope(:find))).delete end # Returns the result of an SQL statement that should only include a COUNT(*) in the SELECT part. @@ -1687,19 +1685,27 @@ module ActiveRecord #:nodoc: end end - def construct_finder_sql(options) + def arel_table(table) + Arel(table) + end + + def construct_finder_arel(options) scope = scope(:find) # TODO add lock to Arel - Arel(table_name). - join(construct_join(options[:joins], scope)). - where(construct_conditions(options[:conditions], scope)). + arel_table(options[:from] || table_name). + join(options[:merged_joins] || construct_join(options[:joins], scope)). + where(options[:merged_conditions] || construct_conditions(options[:conditions], scope)). project(options[:select] || (scope && scope[:select]) || default_select(options[:joins] || (scope && scope[:joins]))). group(construct_group(options[:group], options[:having], scope)). order(construct_order(options[:order], scope)). take(construct_limit(options, scope)). skip(construct_offset(options, scope) - ).to_sql + ) + end + + def construct_finder_sql(options) + construct_finder_arel(options).to_sql end def construct_join(joins, scope = :auto) @@ -1715,6 +1721,8 @@ module ActiveRecord #:nodoc: end when String " #{merged_joins} " + else + "" end end @@ -2644,11 +2652,8 @@ module ActiveRecord #:nodoc: # be made (since they can't be persisted). def destroy unless new_record? - connection.delete( - "DELETE FROM #{self.class.quoted_table_name} " + - "WHERE #{connection.quote_column_name(self.class.primary_key)} = #{quoted_id}", - "#{self.class.name} Destroy" - ) + table = Arel(self.class.table_name) + table.where(table[self.class.primary_key].eq(quoted_id)).delete end freeze diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index f077818d3b..9dbfcdf175 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -54,7 +54,7 @@ module ActiveRecord # # Person.average('age') # => 35.8 def average(column_name, options = {}) - calculate(:avg, column_name, options) + calculate(:average, column_name, options) end # Calculates the minimum value on a given column. The value is returned @@ -63,7 +63,7 @@ module ActiveRecord # # Person.minimum('age') # => 7 def minimum(column_name, options = {}) - calculate(:min, column_name, options) + calculate(:minimum, column_name, options) end # Calculates the maximum value on a given column. The value is returned @@ -72,7 +72,7 @@ module ActiveRecord # # Person.maximum('age') # => 93 def maximum(column_name, options = {}) - calculate(:max, column_name, options) + calculate(:maximum, column_name, options) end # Calculates the sum of values on a given column. The value is returned @@ -124,19 +124,96 @@ module ActiveRecord # Person.sum("2 * age") def calculate(operation, column_name, options = {}) validate_calculation_options(operation, options) - column_name = options[:select] if options[:select] - column_name = '*' if column_name == :all - column = column_for column_name + + scope = scope(:find) + + merged_includes = merge_includes(scope ? scope[:include] : [], options[:include]) + joins = construct_join(options[:joins], scope) + + if merged_includes.any? + join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, joins) + joins << join_dependency.join_associations.collect{|join| join.association_join }.join + end + + if operation == :count + if merged_includes.any? + distinct = true + column_name = options[:select] || primary_key + end + + distinct = nil if column_name.to_s =~ /\s*DISTINCT\s+/i + distinct ||= options[:distinct] + else + distinct = nil + end + catch :invalid_query do + conditions = construct_conditions(options[:conditions], scope) + conditions << construct_limited_ids_condition(conditions, options, join_dependency) if join_dependency && !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) + if options[:group] - return execute_grouped_calculation(operation, column_name, column, options) + return execute_grouped_calculation(operation, column_name, options.merge(:merged_conditions => conditions, :merged_joins => joins, :distinct => distinct)) else - return execute_simple_calculation(operation, column_name, column, options) + return execute_simple_calculation(operation, column_name, options.merge(:merged_conditions => conditions, :merged_joins => joins, :distinct => distinct)) end end 0 end + def execute_simple_calculation(operation, column_name, options) #:nodoc: + table = options[:from] || table_name + value = if operation == :count + if column_name == :all && options[:select].blank? + column_name = "*" + elsif !options[:select].blank? + column_name = options[:select] + end + construct_finder_arel(options.merge(:select => Arel::Attribute.new(Arel(table), column_name).count(options[:distinct]))).select_value + else + construct_finder_arel(options.merge(:select => Arel::Attribute.new(Arel(table), column_name).send(operation))).select_value + end + + type_cast_calculated_value(value, column_for(column_name), operation) + end + + def execute_grouped_calculation(operation, column_name, 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 ? association.primary_key_name : group_attr + group_alias = column_alias_for(group_field) + group_column = column_for group_field + + options[:group] = connection.adapter_name == 'FrontBase' ? group_alias : group_field + + aggregate_alias = column_alias_for(operation, column_name) + if operation == :count && column_name == :all + options[:select] = "COUNT(*) AS count_all, #{group_field} AS #{group_alias}" + else + arel_column = Arel::Attribute.new(Arel(table_name), column_name).send(operation) + options[:select] = "#{arel_column.as(aggregate_alias).to_sql}, #{group_field} AS #{group_alias}" + end + + + sql = construct_finder_arel(options) + + calculated_data = connection.select_all(sql.to_sql) + + if association + key_ids = calculated_data.collect { |row| row[group_alias] } + key_records = association.klass.base_class.find(key_ids) + key_records = key_records.inject({}) { |hsh, r| hsh.merge(r.id => r) } + end + + calculated_data.inject(ActiveSupport::OrderedHash.new) do |all, row| + key = type_cast_calculated_value(row[group_alias], group_column) + key = key_records[key] if associated + value = row[aggregate_alias] + all[key] = type_cast_calculated_value(value, column_for(column_name), operation) + all + end + end + protected def construct_count_options_from_args(*args) options = {} @@ -167,108 +244,6 @@ module ActiveRecord [column_name || :all, options] end - def construct_calculation_sql(operation, column_name, options) #:nodoc: - operation = operation.to_s.downcase - options = options.symbolize_keys - - scope = scope(:find) - merged_includes = merge_includes(scope ? scope[:include] : [], options[:include]) - aggregate_alias = column_alias_for(operation, column_name) - column_name = "#{connection.quote_table_name(table_name)}.#{column_name}" if column_names.include?(column_name.to_s) - - if operation == 'count' - if merged_includes.any? - options[:distinct] = true - column_name = options[:select] || [connection.quote_table_name(table_name), primary_key] * '.' - end - - if options[:distinct] - use_workaround = !connection.supports_count_distinct? - end - end - - if options[:distinct] && column_name.to_s !~ /\s*DISTINCT\s+/i - distinct = 'DISTINCT ' - end - sql = "SELECT #{operation}(#{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] - if options[:from] - sql << " FROM #{options[:from]} " - else - sql << " FROM (SELECT #{distinct}#{column_name}" if use_workaround - sql << " FROM #{connection.quote_table_name(table_name)} " - end - - joins = "" - add_joins!(joins, options[:joins], scope) - - if merged_includes.any? - join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, joins) - sql << join_dependency.join_associations.collect{|join| join.association_join }.join - end - - sql << joins unless joins.blank? - - add_conditions!(sql, options[:conditions], scope) - add_limited_ids_condition!(sql, options, join_dependency) if join_dependency && !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) - - if options[:group] - group_key = connection.adapter_name == 'FrontBase' ? :group_alias : :group_field - sql << " GROUP BY #{options[group_key]} " - end - - if options[:group] && options[:having] - having = sanitize_sql_for_conditions(options[:having]) - - # FrontBase requires identifiers in the HAVING clause and chokes on function calls - if connection.adapter_name == 'FrontBase' - having.downcase! - having.gsub!(/#{operation}\s*\(\s*#{column_name}\s*\)/, aggregate_alias) - end - - sql << " HAVING #{having} " - end - - sql << " ORDER BY #{options[:order]} " if options[:order] - add_limit!(sql, options, scope) - sql << ") #{aggregate_alias}_subquery" if use_workaround - sql - end - - 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, 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 ? association.primary_key_name : group_attr - group_alias = column_alias_for(group_field) - group_column = column_for group_field - 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] } - key_records = association.klass.base_class.find(key_ids) - key_records = key_records.inject({}) { |hsh, r| hsh.merge(r.id => r) } - end - - calculated_data.inject(ActiveSupport::OrderedHash.new) do |all, row| - key = type_cast_calculated_value(row[group_alias], group_column) - key = key_records[key] if associated - value = row[aggregate_alias] - all[key] = type_cast_calculated_value(value, column, operation) - all - end - end private def validate_calculation_options(operation, options = {}) @@ -304,7 +279,7 @@ module ActiveRecord case operation when 'count' then value.to_i when 'sum' then type_cast_using_column(value || '0', column) - when 'avg' then value && (value.is_a?(Fixnum) ? value.to_f : value).to_d + when 'average' then value && (value.is_a?(Fixnum) ? value.to_f : value).to_d else type_cast_using_column(value, column) end end diff --git a/activerecord/lib/active_record/test_case.rb b/activerecord/lib/active_record/test_case.rb index b790eb4343..2dfe2c09ea 100644 --- a/activerecord/lib/active_record/test_case.rb +++ b/activerecord/lib/active_record/test_case.rb @@ -20,7 +20,7 @@ module ActiveRecord patterns_to_match.each do |pattern| failed_patterns << pattern unless $queries_executed.any?{ |sql| pattern === sql } end - assert failed_patterns.empty?, "Query pattern(s) #{failed_patterns.map(&:inspect).join(', ')} not found in #{$queries_executed}" + assert failed_patterns.empty?, "Query pattern(s) #{failed_patterns.map(&:inspect).join(', ')} not found.#{$queries_executed.size == 0 ? '' : "\nQueries:\n#{$queries_executed.join("\n")}"}" end def assert_queries(num = 1) -- cgit v1.2.3