Merge branch 'master' into bug/filtered_parameters_class

6 files changed, 127 insertions, 128 deletions

diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index 35c670f1a1..9cabd1af13 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -37,11 +37,8 @@ module ActiveRecord
     # Note: not all valid {Relation#select}[rdoc-ref:QueryMethods#select] expressions are valid #count expressions. The specifics differ
     # between databases. In invalid cases, an error from the database is thrown.
     def count(column_name = nil)
-      if block_given?
-        to_a.count { |*block_args| yield(*block_args) }
-      else
-        calculate(:count, column_name)
-      end
+      return super() if block_given?
+      calculate(:count, column_name)
     end
 
     # Calculates the average value on a given column. Returns +nil+ if there's
@@ -75,8 +72,8 @@ module ActiveRecord
     # #calculate for examples with options.
     #
     #   Person.sum(:age) # => 4562
-    def sum(column_name = nil, &block)
-      return super(&block) if block_given?
+    def sum(column_name = nil)
+      return super() if block_given?
       calculate(:sum, column_name)
     end
 
@@ -232,7 +229,7 @@ module ActiveRecord
           query_builder = build_count_subquery(spawn, column_name, distinct)
         else
           # PostgreSQL doesn't like ORDER BY when there are no GROUP BY
-          relation = unscope(:order)
+          relation = unscope(:order).distinct!(false)
 
           column = aggregate_column(column_name)
 
@@ -282,7 +279,7 @@ module ActiveRecord
             operation,
             distinct).as(aggregate_alias)
         ]
-        select_values += select_values unless having_clause.empty?
+        select_values += self.select_values unless having_clause.empty?
 
         select_values.concat group_columns.map { |aliaz, field|
           if field.respond_to?(:as)
@@ -292,7 +289,7 @@ module ActiveRecord
           end
         }
 
-        relation = except(:group)
+        relation = except(:group).distinct!(false)
         relation.group_values  = group_fields
         relation.select_values = select_values
 
diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb
index d3ba724507..50378f9d99 100644
--- a/activerecord/lib/active_record/relation/delegation.rb
+++ b/activerecord/lib/active_record/relation/delegation.rb
@@ -36,9 +36,9 @@ module ActiveRecord
     # may vary depending on the klass of a relation, so we create a subclass of Relation
     # for each different klass, and the delegations are compiled into that subclass only.
 
-    delegate :to_xml, :encode_with, :length, :collect, :map, :each, :all?, :include?, :to_ary, :join,
+    delegate :to_xml, :encode_with, :length, :each, :uniq, :to_ary, :join,
              :[], :&, :|, :+, :-, :sample, :reverse, :compact, :in_groups, :in_groups_of,
-             :to_sentence, :to_formatted_s,
+             :to_sentence, :to_formatted_s, :as_json,
              :shuffle, :split, :index, to: :records
 
     delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key,
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index 4548944fe6..5d24f5f5ca 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -147,7 +147,7 @@ module ActiveRecord
     def last(limit = nil)
       return find_last(limit) if loaded? || limit_value
 
-      result = limit(limit || 1)
+      result = limit(limit)
       result.order!(arel_attribute(primary_key)) if order_values.empty? && primary_key
       result = result.reverse_order!
 
@@ -430,140 +430,142 @@ module ActiveRecord
         reflections.none?(&:collection?)
       end
 
-      private
+      def find_with_ids(*ids)
+        raise UnknownPrimaryKey.new(@klass) if primary_key.nil?
 
-        def find_with_ids(*ids)
-          raise UnknownPrimaryKey.new(@klass) if primary_key.nil?
+        expects_array = ids.first.kind_of?(Array)
+        return ids.first if expects_array && ids.first.empty?
 
-          expects_array = ids.first.kind_of?(Array)
-          return ids.first if expects_array && ids.first.empty?
+        ids = ids.flatten.compact.uniq
 
-          ids = ids.flatten.compact.uniq
-
-          case ids.size
-          when 0
-            raise RecordNotFound, "Couldn't find #{@klass.name} without an ID"
-          when 1
-            result = find_one(ids.first)
-            expects_array ? [ result ] : result
-          else
-            find_some(ids)
-          end
-        rescue ::RangeError
-          raise RecordNotFound, "Couldn't find #{@klass.name} with an out of range ID"
+        case ids.size
+        when 0
+          raise RecordNotFound, "Couldn't find #{@klass.name} without an ID"
+        when 1
+          result = find_one(ids.first)
+          expects_array ? [ result ] : result
+        else
+          find_some(ids)
         end
+      rescue ::RangeError
+        raise RecordNotFound, "Couldn't find #{@klass.name} with an out of range ID"
+      end
 
-        def find_one(id)
-          if ActiveRecord::Base === id
-            raise ArgumentError, <<-MSG.squish
-              You are passing an instance of ActiveRecord::Base to `find`.
-              Please pass the id of the object by calling `.id`.
-            MSG
-          end
-
-          relation = where(primary_key => id)
-          record = relation.take
-
-          raise_record_not_found_exception!(id, 0, 1) unless record
-
-          record
+      def find_one(id)
+        if ActiveRecord::Base === id
+          raise ArgumentError, <<-MSG.squish
+            You are passing an instance of ActiveRecord::Base to `find`.
+            Please pass the id of the object by calling `.id`.
+          MSG
         end
 
-        def find_some(ids)
-          return find_some_ordered(ids) unless order_values.present?
+        relation = where(primary_key => id)
+        record = relation.take
 
-          result = where(primary_key => ids).to_a
+        raise_record_not_found_exception!(id, 0, 1) unless record
 
-          expected_size =
-            if limit_value && ids.size > limit_value
-              limit_value
-            else
-              ids.size
-            end
+        record
+      end
 
-          # 11 ids with limit 3, offset 9 should give 2 results.
-          if offset_value && (ids.size - offset_value < expected_size)
-            expected_size = ids.size - offset_value
-          end
+      def find_some(ids)
+        return find_some_ordered(ids) unless order_values.present?
 
-          if result.size == expected_size
-            result
+        result = where(primary_key => ids).to_a
+
+        expected_size =
+          if limit_value && ids.size > limit_value
+            limit_value
           else
-            raise_record_not_found_exception!(ids, result.size, expected_size)
+            ids.size
           end
+
+        # 11 ids with limit 3, offset 9 should give 2 results.
+        if offset_value && (ids.size - offset_value < expected_size)
+          expected_size = ids.size - offset_value
         end
 
-        def find_some_ordered(ids)
-          ids = ids.slice(offset_value || 0, limit_value || ids.size) || []
+        if result.size == expected_size
+          result
+        else
+          raise_record_not_found_exception!(ids, result.size, expected_size)
+        end
+      end
 
-          result = except(:limit, :offset).where(primary_key => ids).records
+      def find_some_ordered(ids)
+        ids = ids.slice(offset_value || 0, limit_value || ids.size) || []
 
-          if result.size == ids.size
-            pk_type = @klass.type_for_attribute(primary_key)
+        result = except(:limit, :offset).where(primary_key => ids).records
 
-            records_by_id = result.index_by(&:id)
-            ids.map { |id| records_by_id.fetch(pk_type.cast(id)) }
-          else
-            raise_record_not_found_exception!(ids, result.size, ids.size)
-          end
-        end
+        if result.size == ids.size
+          pk_type = @klass.type_for_attribute(primary_key)
 
-        def find_take
-          if loaded?
-            records.first
-          else
-            @take ||= limit(1).records.first
-          end
+          records_by_id = result.index_by(&:id)
+          ids.map { |id| records_by_id.fetch(pk_type.cast(id)) }
+        else
+          raise_record_not_found_exception!(ids, result.size, ids.size)
         end
+      end
 
-        def find_take_with_limit(limit)
-          if loaded?
-            records.take(limit)
-          else
-            limit(limit).to_a
-          end
+      def find_take
+        if loaded?
+          records.first
+        else
+          @take ||= limit(1).records.first
         end
+      end
 
-        def find_nth(index)
-          @offsets[offset_index + index] ||= find_nth_with_limit(index, 1).first
+      def find_take_with_limit(limit)
+        if loaded?
+          records.take(limit)
+        else
+          limit(limit).to_a
         end
+      end
+
+      def find_nth(index)
+        @offsets[offset_index + index] ||= find_nth_with_limit(index, 1).first
+      end
 
-        def find_nth_with_limit(index, limit)
-          if loaded?
-            records[index, limit] || []
+      def find_nth_with_limit(index, limit)
+        if loaded?
+          records[index, limit] || []
+        else
+          relation = if order_values.empty? && primary_key
+            order(arel_attribute(primary_key).asc)
           else
-            relation = if order_values.empty? && primary_key
-              order(arel_attribute(primary_key).asc)
-            else
-              self
-            end
+            self
+          end
 
+          if limit_value.nil? || index < limit_value
             relation = relation.offset(offset_index + index) unless index.zero?
             relation.limit(limit).to_a
+          else
+            []
           end
         end
+      end
 
-        def find_nth_from_last(index)
-          if loaded?
-            records[-index]
+      def find_nth_from_last(index)
+        if loaded?
+          records[-index]
+        else
+          relation = if order_values.empty? && primary_key
+            order(arel_attribute(primary_key).asc)
           else
-            relation = if order_values.empty? && primary_key
-              order(arel_attribute(primary_key).asc)
-            else
-              self
-            end
-
-            relation.to_a[-index]
-            # TODO: can be made more performant on large result sets by
-            # for instance, last(index)[-index] (which would require
-            # refactoring the last(n) finder method to make test suite pass),
-            # or by using a combination of reverse_order, limit, and offset,
-            # e.g., reverse_order.offset(index-1).first
+            self
           end
-        end
 
-        def find_last(limit)
-          limit ? records.last(limit) : records.last
+          relation.to_a[-index]
+          # TODO: can be made more performant on large result sets by
+          # for instance, last(index)[-index] (which would require
+          # refactoring the last(n) finder method to make test suite pass),
+          # or by using a combination of reverse_order, limit, and offset,
+          # e.g., reverse_order.offset(index-1).first
         end
+      end
+
+      def find_last(limit)
+        limit ? records.last(limit) : records.last
+      end
   end
 end
diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb
index 18ae10a652..fca914aedd 100644
--- a/activerecord/lib/active_record/relation/predicate_builder.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder.rb
@@ -87,14 +87,19 @@ module ActiveRecord
         binds = []
 
         attributes.each do |column_name, value|
+          binds.concat(value.bound_attributes) if value.is_a?(Relation)
           case
           when value.is_a?(Hash) && !table.has_column?(column_name)
             attrs, bvs = associated_predicate_builder(column_name).create_binds_for_hash(value)
             result[column_name] = attrs
             binds += bvs
-            next
-          when value.is_a?(Relation)
-            binds += value.bound_attributes
+          when table.associated_with?(column_name)
+            # Find the foreign key when using queries such as:
+            # Post.where(author: author)
+            #
+            # For polymorphic relationships, find the foreign key and type:
+            # PriceEstimate.where(estimate_of: treasure)
+            result[column_name] = AssociationQueryHandler.value_for(table, column_name, value)
           when value.is_a?(Range) && !table.type(column_name).respond_to?(:subtype)
             first = value.begin
             last = value.end
@@ -114,15 +119,6 @@ module ActiveRecord
               binds << build_bind_param(column_name, value)
             end
           end
-
-          # Find the foreign key when using queries such as:
-          # Post.where(author: author)
-          #
-          # For polymorphic relationships, find the foreign key and type:
-          # PriceEstimate.where(estimate_of: treasure)
-          if table.associated_with?(column_name)
-            result[column_name] = AssociationQueryHandler.value_for(table, column_name, value)
-          end
         end
 
         [result, binds]
@@ -155,7 +151,6 @@ module ActiveRecord
       end
 
       def can_be_bound?(column_name, value)
-        return if table.associated_with?(column_name)
         case value
         when Array, Range
           table.type(column_name).respond_to?(:subtype)
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 4ee413c805..1178dec706 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -1130,7 +1130,12 @@ module ActiveRecord
             arel_attribute(arg).asc
           when Hash
             arg.map { |field, dir|
-              arel_attribute(field).send(dir.downcase)
+              case field
+              when Arel::Nodes::SqlLiteral
+                field.send(dir.downcase)
+              else
+                arel_attribute(field).send(dir.downcase)
+              end
             }
           else
             arg
diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb
index 417b24c7bb..119910ee79 100644
--- a/activerecord/lib/active_record/relation/where_clause.rb
+++ b/activerecord/lib/active_record/relation/where_clause.rb
@@ -148,10 +148,10 @@ module ActiveRecord
               (binds_index...(binds_index + binds_contains)).each do |i|
                 except_binds[i] = true
               end
-
-              binds_index += binds_contains
             end
 
+            binds_index += binds_contains if binds_contains
+
             except
           end