Calculations now use Arel to construct the query.

Implemented other methods in AR::Base with Arel support.

4 files changed, 123 insertions, 135 deletions

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: <tt>LEFT OUTER JOIN comments ON comments.post_id = posts.id</tt> and
     # <tt>LEFT OUTER JOIN authors ON authors.id = posts.author_id</tt>. 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
-    # <tt>:order => "author.name DESC"</tt> will work but <tt>:order => "name DESC"</tt> will not. 
+    # <tt>:order => "author.name DESC"</tt> will work but <tt>:order => "name DESC"</tt> will not.
     #
     # If you do want eagerload only some members of an association it is usually more natural to <tt>:include</tt> 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 <tt>:class_name</tt> and <tt>:foreign_key</tt>
-      #   are ignored, as the association uses the source reflection. You can only use a <tt>:through</tt> query through a 
+      #   are ignored, as the association uses the source reflection. You can only use a <tt>:through</tt> query through a
       #   <tt>has_one</tt> or <tt>belongs_to</tt> association on the join model.
       # [:source]
       #   Specifies the source association name used by <tt>has_one :through</tt> queries.  Only use it if the name cannot be
@@ -1123,8 +1123,8 @@ module ActiveRecord
       #   the association will use "project_id" as the default <tt>:association_foreign_key</tt>.
       # [:conditions]
       #   Specify the conditions that the associated object must meet in order to be included as a +WHERE+
-      #   SQL fragment, such as <tt>authorized = 1</tt>.  Record creations from the association are scoped if a hash is used.  
-      #   <tt>has_many :posts, :conditions => {:published => true}</tt> will create published posts with <tt>@blog.posts.create</tt> 
+      #   SQL fragment, such as <tt>authorized = 1</tt>.  Record creations from the association are scoped if a hash is used.
+      #   <tt>has_many :posts, :conditions => {:published => true}</tt> will create published posts with <tt>@blog.posts.create</tt>
       #   or <tt>@blog.posts.build</tt>.
       # [:order]
       #   Specify the order in which the associated objects are returned as an <tt>ORDER BY</tt> 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 <tt>before_*</tt> 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)