diff options
Diffstat (limited to 'activerecord/lib/active_record/relation')
17 files changed, 439 insertions, 175 deletions
diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index 221bc73680..3639625722 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -2,6 +2,8 @@ require "active_record/relation/batches/batch_enumerator" module ActiveRecord module Batches + ORDER_OR_LIMIT_IGNORED_MESSAGE = "Scoped order and limit are ignored, it's forced to be batch order and batch size." + # Looping through a collection of records from the database # (using the Scoping::Named::ClassMethods.all method, for example) # is very inefficient since it will try to instantiate all the objects at once. @@ -29,15 +31,18 @@ module ActiveRecord # # ==== Options # * <tt>:batch_size</tt> - Specifies the size of the batch. Default to 1000. - # * <tt>:begin_at</tt> - Specifies the primary key value to start from, inclusive of the value. - # * <tt>:end_at</tt> - Specifies the primary key value to end at, inclusive of the value. + # * <tt>:start</tt> - Specifies the primary key value to start from, inclusive of the value. + # * <tt>:finish</tt> - Specifies the primary key value to end at, inclusive of the value. + # * <tt>:error_on_ignore</tt> - Overrides the application config to specify if an error should be raised when + # the order and limit have to be ignored due to batching. + # # This is especially useful if you want multiple workers dealing with # the same processing queue. You can make worker 1 handle all the records # between id 0 and 10,000 and worker 2 handle from 10,000 and beyond - # (by setting the +:begin_at+ and +:end_at+ option on each worker). + # (by setting the +:start+ and +:finish+ option on each worker). # # # Let's process for a batch of 2000 records, skipping the first 2000 rows - # Person.find_each(begin_at: 2000, batch_size: 2000) do |person| + # Person.find_each(start: 2000, batch_size: 2000) do |person| # person.party_all_night! # end # @@ -48,22 +53,15 @@ module ActiveRecord # # NOTE: You can't set the limit either, that's used to control # the batch sizes. - def find_each(begin_at: nil, end_at: nil, batch_size: 1000, start: nil) - if start - begin_at = start - ActiveSupport::Deprecation.warn(<<-MSG.squish) - Passing `start` value to find_each is deprecated, and will be removed in Rails 5.1. - Please pass `begin_at` instead. - MSG - end + def find_each(start: nil, finish: nil, batch_size: 1000, error_on_ignore: nil) if block_given? - find_in_batches(begin_at: begin_at, end_at: end_at, batch_size: batch_size) do |records| + find_in_batches(start: start, finish: finish, batch_size: batch_size, error_on_ignore: error_on_ignore) do |records| records.each { |record| yield record } end else - enum_for(:find_each, begin_at: begin_at, end_at: end_at, batch_size: batch_size) do + enum_for(:find_each, start: start, finish: finish, batch_size: batch_size, error_on_ignore: error_on_ignore) do relation = self - apply_limits(relation, begin_at, end_at).size + apply_limits(relation, start, finish).size end end end @@ -88,15 +86,18 @@ module ActiveRecord # # ==== Options # * <tt>:batch_size</tt> - Specifies the size of the batch. Default to 1000. - # * <tt>:begin_at</tt> - Specifies the primary key value to start from, inclusive of the value. - # * <tt>:end_at</tt> - Specifies the primary key value to end at, inclusive of the value. + # * <tt>:start</tt> - Specifies the primary key value to start from, inclusive of the value. + # * <tt>:finish</tt> - Specifies the primary key value to end at, inclusive of the value. + # * <tt>:error_on_ignore</tt> - Overrides the application config to specify if an error should be raised when + # the order and limit have to be ignored due to batching. + # # This is especially useful if you want multiple workers dealing with # the same processing queue. You can make worker 1 handle all the records # between id 0 and 10,000 and worker 2 handle from 10,000 and beyond - # (by setting the +:begin_at+ and +:end_at+ option on each worker). + # (by setting the +:start+ and +:finish+ option on each worker). # # # Let's process the next 2000 records - # Person.find_in_batches(begin_at: 2000, batch_size: 2000) do |group| + # Person.find_in_batches(start: 2000, batch_size: 2000) do |group| # group.each { |person| person.party_all_night! } # end # @@ -107,24 +108,16 @@ module ActiveRecord # # NOTE: You can't set the limit either, that's used to control # the batch sizes. - def find_in_batches(begin_at: nil, end_at: nil, batch_size: 1000, start: nil) - if start - begin_at = start - ActiveSupport::Deprecation.warn(<<-MSG.squish) - Passing `start` value to find_in_batches is deprecated, and will be removed in Rails 5.1. - Please pass `begin_at` instead. - MSG - end - + def find_in_batches(start: nil, finish: nil, batch_size: 1000, error_on_ignore: nil) relation = self unless block_given? - return to_enum(:find_in_batches, begin_at: begin_at, end_at: end_at, batch_size: batch_size) do - total = apply_limits(relation, begin_at, end_at).size + return to_enum(:find_in_batches, start: start, finish: finish, batch_size: batch_size, error_on_ignore: error_on_ignore) do + total = apply_limits(relation, start, finish).size (total - 1).div(batch_size) + 1 end end - in_batches(of: batch_size, begin_at: begin_at, end_at: end_at, load: true) do |batch| + in_batches(of: batch_size, start: start, finish: finish, load: true, error_on_ignore: error_on_ignore) do |batch| yield batch.to_a end end @@ -153,18 +146,20 @@ module ActiveRecord # ==== Options # * <tt>:of</tt> - Specifies the size of the batch. Default to 1000. # * <tt>:load</tt> - Specifies if the relation should be loaded. Default to false. - # * <tt>:begin_at</tt> - Specifies the primary key value to start from, inclusive of the value. - # * <tt>:end_at</tt> - Specifies the primary key value to end at, inclusive of the value. + # * <tt>:start</tt> - Specifies the primary key value to start from, inclusive of the value. + # * <tt>:finish</tt> - Specifies the primary key value to end at, inclusive of the value. + # * <tt>:error_on_ignore</tt> - Overrides the application config to specify if an error should be raised when + # the order and limit have to be ignored due to batching. # # This is especially useful if you want to work with the # ActiveRecord::Relation object instead of the array of records, or if # you want multiple workers dealing with the same processing queue. You can # make worker 1 handle all the records between id 0 and 10,000 and worker 2 - # handle from 10,000 and beyond (by setting the +:begin_at+ and +:end_at+ + # handle from 10,000 and beyond (by setting the +:start+ and +:finish+ # option on each worker). # # # Let's process the next 2000 records - # Person.in_batches(of: 2000, begin_at: 2000).update_all(awesome: true) + # Person.in_batches(of: 2000, start: 2000).update_all(awesome: true) # # An example of calling where query method on the relation: # @@ -186,23 +181,23 @@ module ActiveRecord # # NOTE: You can't set the limit either, that's used to control the batch # sizes. - def in_batches(of: 1000, begin_at: nil, end_at: nil, load: false) + def in_batches(of: 1000, start: nil, finish: nil, load: false, error_on_ignore: nil) relation = self unless block_given? - return BatchEnumerator.new(of: of, begin_at: begin_at, end_at: end_at, relation: self) + return BatchEnumerator.new(of: of, start: start, finish: finish, relation: self) end - if logger && (arel.orders.present? || arel.taken.present?) - logger.warn("Scoped order and limit are ignored, it's forced to be batch order and batch size") + if arel.orders.present? || arel.taken.present? + act_on_order_or_limit_ignored(error_on_ignore) end relation = relation.reorder(batch_order).limit(of) - relation = apply_limits(relation, begin_at, end_at) + relation = apply_limits(relation, start, finish) batch_relation = relation loop do if load - records = batch_relation.to_a + records = batch_relation.records ids = records.map(&:id) yielded_relation = self.where(primary_key => ids) yielded_relation.load_records(records) @@ -219,20 +214,30 @@ module ActiveRecord yield yielded_relation break if ids.length < of - batch_relation = relation.where(table[primary_key].gt(primary_key_offset)) + batch_relation = relation.where(arel_attribute(primary_key).gt(primary_key_offset)) end end private - def apply_limits(relation, begin_at, end_at) - relation = relation.where(table[primary_key].gteq(begin_at)) if begin_at - relation = relation.where(table[primary_key].lteq(end_at)) if end_at + def apply_limits(relation, start, finish) + relation = relation.where(arel_attribute(primary_key).gteq(start)) if start + relation = relation.where(arel_attribute(primary_key).lteq(finish)) if finish relation end def batch_order "#{quoted_table_name}.#{quoted_primary_key} ASC" end + + def act_on_order_or_limit_ignored(error_on_ignore) + raise_error = (error_on_ignore.nil? ? self.klass.error_on_ignored_order_or_limit : error_on_ignore) + + if raise_error + raise ArgumentError.new(ORDER_OR_LIMIT_IGNORED_MESSAGE) + elsif logger + logger.warn(ORDER_OR_LIMIT_IGNORED_MESSAGE) + end + end end end diff --git a/activerecord/lib/active_record/relation/batches/batch_enumerator.rb b/activerecord/lib/active_record/relation/batches/batch_enumerator.rb index 153aae9584..333b3a63cf 100644 --- a/activerecord/lib/active_record/relation/batches/batch_enumerator.rb +++ b/activerecord/lib/active_record/relation/batches/batch_enumerator.rb @@ -3,11 +3,11 @@ module ActiveRecord class BatchEnumerator include Enumerable - def initialize(of: 1000, begin_at: nil, end_at: nil, relation:) #:nodoc: + def initialize(of: 1000, start: nil, finish: nil, relation:) #:nodoc: @of = of @relation = relation - @begin_at = begin_at - @end_at = end_at + @start = start + @finish = finish end # Looping through a collection of records from the database (using the @@ -34,19 +34,19 @@ module ActiveRecord def each_record return to_enum(:each_record) unless block_given? - @relation.to_enum(:in_batches, of: @of, begin_at: @begin_at, end_at: @end_at, load: true).each do |relation| - relation.to_a.each { |record| yield record } + @relation.to_enum(:in_batches, of: @of, start: @start, finish: @finish, load: true).each do |relation| + relation.records.each { |record| yield record } end end # Delegates #delete_all, #update_all, #destroy_all methods to each batch. # # People.in_batches.delete_all - # People.in_batches.destroy_all('age < 10') + # People.where('age < 10').in_batches.destroy_all # People.in_batches.update_all('age = age + 1') [:delete_all, :update_all, :destroy_all].each do |method| define_method(method) do |*args, &block| - @relation.to_enum(:in_batches, of: @of, begin_at: @begin_at, end_at: @end_at, load: false).each do |relation| + @relation.to_enum(:in_batches, of: @of, start: @start, finish: @finish, load: false).each do |relation| relation.send(method, *args, &block) end end @@ -58,7 +58,7 @@ module ActiveRecord # relation.update_all(awesome: true) # end def each - enum = @relation.to_enum(:in_batches, of: @of, begin_at: @begin_at, end_at: @end_at, load: false) + enum = @relation.to_enum(:in_batches, of: @of, start: @start, finish: @finish, load: false) return enum.each { |relation| yield relation } if block_given? enum end diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index f45844a9ea..d6d92b8607 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -37,7 +37,11 @@ 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) - calculate(:count, column_name) + if block_given? + to_a.count { |*block_args| yield(*block_args) } + else + calculate(:count, column_name) + end end # Calculates the average value on a given column. Returns +nil+ if there's @@ -89,7 +93,7 @@ module ActiveRecord # # There are two basic forms of output: # - # * Single aggregate value: The single value is type cast to Fixnum for COUNT, Float + # * Single aggregate value: The single value is type cast to Integer for COUNT, Float # for AVG, and the given column's type for everything else. # # * Grouped values: This returns an ordered hash of the values and groups them. It @@ -155,15 +159,7 @@ module ActiveRecord # See also #ids. # def pluck(*column_names) - column_names.map! do |column_name| - if column_name.is_a?(Symbol) && attribute_alias?(column_name) - attribute_alias(column_name) - else - column_name.to_s - end - end - - if loaded? && (column_names - @klass.column_names).empty? + if loaded? && (column_names.map(&:to_s) - @klass.attribute_names - @klass.attribute_aliases.keys).empty? return @records.pluck(*column_names) end @@ -172,7 +168,7 @@ module ActiveRecord else relation = spawn relation.select_values = column_names.map { |cn| - columns_hash.key?(cn) ? arel_table[cn] : cn + @klass.has_attribute?(cn) || @klass.attribute_alias?(cn) ? arel_attribute(cn) : cn } result = klass.connection.select_all(relation.arel, nil, bound_attributes) result.cast_values(klass.attribute_types) diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index 27de313d05..2484cb3264 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -1,4 +1,3 @@ -require 'set' require 'active_support/concern' module ActiveRecord @@ -36,13 +35,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. - BLACKLISTED_ARRAY_METHODS = [ - :compact!, :flatten!, :reject!, :reverse!, :rotate!, :map!, - :shuffle!, :slice!, :sort!, :sort_by!, :delete_if, - :keep_if, :pop, :shift, :delete_at, :select! - ].to_set # :nodoc: - - delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, :join, to: :to_a + delegate :to_xml, :encode_with, :length, :collect, :map, :each, :all?, :include?, :to_ary, :join, + :[], :&, :|, :+, :-, :sample, :reverse, :compact, :in_groups, :in_groups_of, + :shuffle, :split, to: :records delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :connection, :columns_hash, :to => :klass @@ -114,21 +109,14 @@ module ActiveRecord def respond_to?(method, include_private = false) super || @klass.respond_to?(method, include_private) || - array_delegable?(method) || arel.respond_to?(method, include_private) end protected - def array_delegable?(method) - Array.method_defined?(method) && BLACKLISTED_ARRAY_METHODS.exclude?(method) - end - def method_missing(method, *args, &block) if @klass.respond_to?(method) scoping { @klass.public_send(method, *args, &block) } - elsif array_delegable?(method) - to_a.public_send(method, *args, &block) elsif arel.respond_to?(method) arel.public_send(method, *args, &block) else diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 435cef901b..e7e331f88d 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -42,10 +42,10 @@ module ActiveRecord # Person.find_by(name: 'Spartacus', rating: 4) # # returns the first item or nil. # - # Person.where(name: 'Spartacus', rating: 4).first_or_initialize + # Person.find_or_initialize_by(name: 'Spartacus', rating: 4) # # returns the first item or returns a new instance (requires you call .save to persist against the database). # - # Person.where(name: 'Spartacus', rating: 4).first_or_create + # Person.find_or_create_by(name: 'Spartacus', rating: 4) # # returns the first item or creates it and returns it. # # ==== Alternatives for #find @@ -117,9 +117,9 @@ module ActiveRecord # def first(limit = nil) if limit - find_nth_with_limit(offset_index, limit) + find_nth_with_limit_and_offset(0, limit, offset: offset_index) else - find_nth(0, offset_index) + find_nth 0 end end @@ -145,15 +145,21 @@ module ActiveRecord # # [#<Person id:4>, #<Person id:3>, #<Person id:2>] def last(limit = nil) - if limit - if order_values.empty? && primary_key - order(arel_table[primary_key].desc).limit(limit).reverse - else - to_a.last(limit) - end - else - find_last - end + return find_last(limit) if loaded? || limit_value + + result = limit(limit || 1) + result.order!(arel_attribute(primary_key)) if order_values.empty? && primary_key + result = result.reverse_order! + + limit ? result.reverse : result.first + rescue ActiveRecord::IrreversibleOrderError + ActiveSupport::Deprecation.warn(<<-WARNING.squish) + Finding a last element by loading the relation when SQL ORDER + can not be reversed is deprecated. + Rails 5.1 will raise ActiveRecord::IrreversibleOrderError in this case. + Please call `to_a.last` if you still want to load the relation. + WARNING + find_last(limit) end # Same as #last but raises ActiveRecord::RecordNotFound if no record @@ -169,7 +175,7 @@ module ActiveRecord # Person.offset(3).second # returns the second object from OFFSET 3 (which is OFFSET 4) # Person.where(["user_name = :u", { u: user_name }]).second def second - find_nth(1, offset_index) + find_nth 1 end # Same as #second but raises ActiveRecord::RecordNotFound if no record @@ -185,7 +191,7 @@ module ActiveRecord # Person.offset(3).third # returns the third object from OFFSET 3 (which is OFFSET 5) # Person.where(["user_name = :u", { u: user_name }]).third def third - find_nth(2, offset_index) + find_nth 2 end # Same as #third but raises ActiveRecord::RecordNotFound if no record @@ -201,7 +207,7 @@ module ActiveRecord # Person.offset(3).fourth # returns the fourth object from OFFSET 3 (which is OFFSET 6) # Person.where(["user_name = :u", { u: user_name }]).fourth def fourth - find_nth(3, offset_index) + find_nth 3 end # Same as #fourth but raises ActiveRecord::RecordNotFound if no record @@ -217,7 +223,7 @@ module ActiveRecord # Person.offset(3).fifth # returns the fifth object from OFFSET 3 (which is OFFSET 7) # Person.where(["user_name = :u", { u: user_name }]).fifth def fifth - find_nth(4, offset_index) + find_nth 4 end # Same as #fifth but raises ActiveRecord::RecordNotFound if no record @@ -233,7 +239,7 @@ module ActiveRecord # Person.offset(3).forty_two # returns the forty-second object from OFFSET 3 (which is OFFSET 44) # Person.where(["user_name = :u", { u: user_name }]).forty_two def forty_two - find_nth(41, offset_index) + find_nth 41 end # Same as #forty_two but raises ActiveRecord::RecordNotFound if no record @@ -242,6 +248,38 @@ module ActiveRecord find_nth! 41 end + # Find the third-to-last record. + # If no order is defined it will order by primary key. + # + # Person.third_to_last # returns the third-to-last object fetched by SELECT * FROM people + # Person.offset(3).third_to_last # returns the third-to-last object from OFFSET 3 + # Person.where(["user_name = :u", { u: user_name }]).third_to_last + def third_to_last + find_nth_from_last 3 + end + + # Same as #third_to_last but raises ActiveRecord::RecordNotFound if no record + # is found. + def third_to_last! + find_nth_from_last 3 or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql(@klass.arel_engine)}]") + end + + # Find the second-to-last record. + # If no order is defined it will order by primary key. + # + # Person.second_to_last # returns the second-to-last object fetched by SELECT * FROM people + # Person.offset(3).second_to_last # returns the second-to-last object from OFFSET 3 + # Person.where(["user_name = :u", { u: user_name }]).second_to_last + def second_to_last + find_nth_from_last 2 + end + + # Same as #second_to_last but raises ActiveRecord::RecordNotFound if no record + # is found. + def second_to_last! + find_nth_from_last 2 or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql(@klass.arel_engine)}]") + end + # Returns true if a record exists in the table that matches the +id+ or # conditions given, or false otherwise. The argument can take six forms: # @@ -274,7 +312,7 @@ module ActiveRecord conditions = conditions.id ActiveSupport::Deprecation.warn(<<-MSG.squish) You are passing an instance of ActiveRecord::Base to `exists?`. - Please pass the id of the object by calling `.id` + Please pass the id of the object by calling `.id`. MSG end @@ -298,7 +336,7 @@ module ActiveRecord end # This method is called whenever no records are found with either a single - # id or multiple ids and raises a ActiveRecord::RecordNotFound exception. + # id or multiple ids and raises an ActiveRecord::RecordNotFound exception. # # The error message is different depending on whether a single id or # multiple ids are provided. If multiple ids are provided, then the number @@ -429,7 +467,7 @@ module ActiveRecord id = id.id ActiveSupport::Deprecation.warn(<<-MSG.squish) You are passing an instance of ActiveRecord::Base to `find`. - Please pass the id of the object by calling `.id` + Please pass the id of the object by calling `.id`. MSG end @@ -442,6 +480,8 @@ module ActiveRecord end def find_some(ids) + return find_some_ordered(ids) unless order_values.present? + result = where(primary_key => ids).to_a expected_size = @@ -463,49 +503,96 @@ module ActiveRecord end end + def find_some_ordered(ids) + ids = ids.slice(offset_value || 0, limit_value || ids.size) || [] + + result = except(:limit, :offset).where(primary_key => ids).records + + if result.size == ids.size + pk_type = @klass.type_for_attribute(primary_key) + + 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 if loaded? @records.first else - @take ||= limit(1).to_a.first + @take ||= limit(1).records.first end end - def find_nth(index, offset) + def find_nth(index, offset = nil) + # TODO: once the offset argument is removed we rely on offset_index + # within find_nth_with_limit, rather than pass it in via + # find_nth_with_limit_and_offset + if offset + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Passing an offset argument to find_nth is deprecated, + please use Relation#offset instead. + MSG + end if loaded? @records[index] else - offset += index - @offsets[offset] ||= find_nth_with_limit(offset, 1).first + offset ||= offset_index + @offsets[offset + index] ||= find_nth_with_limit_and_offset(index, 1, offset: offset).first end end def find_nth!(index) - find_nth(index, offset_index) or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql(@klass.arel_engine)}]") + find_nth(index) or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql(@klass.arel_engine)}]") end - def find_nth_with_limit(offset, limit) + def find_nth_with_limit(index, limit) + # TODO: once the offset argument is removed from find_nth, + # find_nth_with_limit_and_offset can be merged into this method relation = if order_values.empty? && primary_key - order(arel_table[primary_key].asc) + order(arel_attribute(primary_key).asc) else self end - relation = relation.offset(offset) unless offset.zero? + relation = relation.offset(index) unless index.zero? relation.limit(limit).to_a end - def find_last + 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 + 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 + end + end + + private + + def find_nth_with_limit_and_offset(index, limit, offset:) # :nodoc: if loaded? - @records.last + @records[index, limit] else - @last ||= - if limit_value - to_a.last - else - reverse_order.limit(1).to_a.first - end + index += offset + find_nth_with_limit(index, limit) end end + + def find_last(limit) + limit ? records.last(limit) : records.last + end end end diff --git a/activerecord/lib/active_record/relation/from_clause.rb b/activerecord/lib/active_record/relation/from_clause.rb index 92340216ed..8945cb0cc5 100644 --- a/activerecord/lib/active_record/relation/from_clause.rb +++ b/activerecord/lib/active_record/relation/from_clause.rb @@ -25,7 +25,7 @@ module ActiveRecord end def self.empty - new(nil, nil) + @empty ||= new(nil, nil) end end end diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index cb971eb255..396638d74d 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -141,6 +141,9 @@ module ActiveRecord end def merge_single_values + if relation.from_clause.empty? + relation.from_clause = other.from_clause + end relation.lock_value ||= other.lock_value unless other.create_with_value.blank? diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 39e7b42629..ecce949370 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -5,6 +5,7 @@ module ActiveRecord require 'active_record/relation/predicate_builder/base_handler' require 'active_record/relation/predicate_builder/basic_object_handler' require 'active_record/relation/predicate_builder/class_handler' + require 'active_record/relation/predicate_builder/polymorphic_array_handler' require 'active_record/relation/predicate_builder/range_handler' require 'active_record/relation/predicate_builder/relation_handler' @@ -18,9 +19,11 @@ module ActiveRecord register_handler(Class, ClassHandler.new(self)) register_handler(Base, BaseHandler.new(self)) register_handler(Range, RangeHandler.new(self)) + register_handler(RangeHandler::RangeWithBinds, RangeHandler.new(self)) register_handler(Relation, RelationHandler.new) register_handler(Array, ArrayHandler.new(self)) register_handler(AssociationQueryValue, AssociationQueryHandler.new(self)) + register_handler(PolymorphicArrayValue, PolymorphicArrayHandler.new(self)) end def build_from_hash(attributes) @@ -39,10 +42,7 @@ module ActiveRecord # # For polymorphic relationships, find the foreign key and type: # PriceEstimate.where(estimate_of: treasure) - if table.associated_with?(column) - value = AssociationQueryValue.new(table.associated_table(column), value) - end - + value = AssociationQueryHandler.value_for(table, column, value) if table.associated_with?(column) build(table.arel_attribute(column), value) end @@ -105,10 +105,23 @@ module ActiveRecord binds += bvs when Relation binds += value.bound_attributes + when Range + first = value.begin + last = value.end + unless first.respond_to?(:infinite?) && first.infinite? + binds << build_bind_param(column_name, first) + first = Arel::Nodes::BindParam.new + end + unless last.respond_to?(:infinite?) && last.infinite? + binds << build_bind_param(column_name, last) + last = Arel::Nodes::BindParam.new + end + + result[column_name] = RangeHandler::RangeWithBinds.new(first, last, value.exclude_end?) else if can_be_bound?(column_name, value) result[column_name] = Arel::Nodes::BindParam.new - binds << Relation::QueryAttribute.new(column_name.to_s, value, table.type(column_name)) + binds << build_bind_param(column_name, value) end end end @@ -123,9 +136,11 @@ module ActiveRecord end def convert_dot_notation_to_hash(attributes) - dot_notation = attributes.keys.select { |s| s.include?(".".freeze) } + dot_notation = attributes.select do |k, v| + k.include?(".".freeze) && !v.is_a?(Hash) + end - dot_notation.each do |key| + dot_notation.each_key do |key| table_name, column_name = key.split(".".freeze) value = attributes.delete(key) attributes[table_name] ||= {} @@ -145,5 +160,9 @@ module ActiveRecord handler_for(value).is_a?(BasicObjectHandler) && !table.associated_with?(column_name) end + + def build_bind_param(column_name, value) + Relation::QueryAttribute.new(column_name.to_s, value, table.type(column_name)) + end end end diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb index e81be63cd3..d7fd878265 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb @@ -1,6 +1,16 @@ module ActiveRecord class PredicateBuilder class AssociationQueryHandler # :nodoc: + def self.value_for(table, column, value) + klass = if table.associated_table(column).polymorphic_association? && ::Array === value && value.first.is_a?(Base) + PolymorphicArrayValue + else + AssociationQueryValue + end + + klass.new(table.associated_table(column), value) + end + def initialize(predicate_builder) @predicate_builder = predicate_builder end diff --git a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb new file mode 100644 index 0000000000..b6c6240343 --- /dev/null +++ b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb @@ -0,0 +1,57 @@ +module ActiveRecord + class PredicateBuilder + class PolymorphicArrayHandler # :nodoc: + def initialize(predicate_builder) + @predicate_builder = predicate_builder + end + + def call(attribute, value) + table = value.associated_table + queries = value.type_to_ids_mapping.map do |type, ids| + { table.association_foreign_type.to_s => type, table.association_foreign_key.to_s => ids } + end + + predicates = queries.map { |query| predicate_builder.build_from_hash(query) } + + if predicates.size > 1 + type_and_ids_predicates = predicates.map { |type_predicate, id_predicate| Arel::Nodes::Grouping.new(type_predicate.and(id_predicate)) } + type_and_ids_predicates.inject(&:or) + else + predicates.first + end + end + + protected + + attr_reader :predicate_builder + end + + class PolymorphicArrayValue # :nodoc: + attr_reader :associated_table, :values + + def initialize(associated_table, values) + @associated_table = associated_table + @values = values + end + + def type_to_ids_mapping + default_hash = Hash.new { |hsh, key| hsh[key] = [] } + values.each_with_object(default_hash) { |value, hash| hash[base_class(value).name] << convert_to_id(value) } + end + + private + + def primary_key(value) + associated_table.association_primary_key(base_class(value)) + end + + def base_class(value) + value.class.base_class + end + + def convert_to_id(value) + value._read_attribute(primary_key(value)) + end + end + end +end diff --git a/activerecord/lib/active_record/relation/predicate_builder/range_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/range_handler.rb index 1b3849e3ad..306d4694ae 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/range_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/range_handler.rb @@ -1,12 +1,28 @@ module ActiveRecord class PredicateBuilder class RangeHandler # :nodoc: + RangeWithBinds = Struct.new(:begin, :end, :exclude_end?) + def initialize(predicate_builder) @predicate_builder = predicate_builder end def call(attribute, value) - attribute.between(value) + if value.begin.respond_to?(:infinite?) && value.begin.infinite? + if value.end.respond_to?(:infinite?) && value.end.infinite? + attribute.not_in([]) + elsif value.exclude_end? + attribute.lt(value.end) + else + attribute.lteq(value.end) + end + elsif value.end.respond_to?(:infinite?) && value.end.infinite? + attribute.gteq(value.begin) + elsif value.exclude_end? + attribute.gteq(value.begin).and(attribute.lt(value.end)) + else + attribute.between(value) + end end protected diff --git a/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb index 063150958a..8a910a82fe 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb @@ -3,7 +3,7 @@ module ActiveRecord class RelationHandler # :nodoc: def call(attribute, value) if value.select_values.empty? - value = value.select(value.klass.arel_table[value.klass.primary_key]) + value = value.select(value.arel_attribute(value.klass.primary_key)) end attribute.in(value.arel) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 2dc52982c9..2a831c2017 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -3,6 +3,7 @@ require "active_record/relation/query_attribute" require "active_record/relation/where_clause" require "active_record/relation/where_clause_factory" require 'active_model/forbidden_attributes_protection' +require 'active_support/core_ext/string/filters' module ActiveRecord module QueryMethods @@ -13,6 +14,8 @@ module ActiveRecord # WhereChain objects act as placeholder for queries in which #where does not have any parameter. # In this case, #where must be chained with #not to return a new relation. class WhereChain + include ActiveModel::ForbiddenAttributesProtection + def initialize(scope) @scope = scope end @@ -41,6 +44,8 @@ module ActiveRecord # User.where.not(name: "Jon", role: "admin") # # SELECT * FROM users WHERE name != 'Jon' AND role != 'admin' def not(opts, *rest) + opts = sanitize_forbidden_attributes(opts) + where_clause = @scope.send(:where_clause_factory).build(opts, rest) @scope.references!(PredicateBuilder.references(opts)) if Hash === opts @@ -49,16 +54,17 @@ module ActiveRecord end end + FROZEN_EMPTY_ARRAY = [].freeze Relation::MULTI_VALUE_METHODS.each do |name| class_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{name}_values # def select_values - @values[:#{name}] || [] # @values[:select] || [] - end # end - # - def #{name}_values=(values) # def select_values=(values) - assert_mutability! # assert_mutability! - @values[:#{name}] = values # @values[:select] = values - end # end + def #{name}_values + @values[:#{name}] || FROZEN_EMPTY_ARRAY + end + + def #{name}_values=(values) + assert_mutability! + @values[:#{name}] = values + end CODE end @@ -93,11 +99,33 @@ module ActiveRecord end def bound_attributes - from_clause.binds + arel.bind_values + where_clause.binds + having_clause.binds + if limit_value && !string_containing_comma?(limit_value) + limit_bind = Attribute.with_cast_value( + "LIMIT".freeze, + connection.sanitize_limit(limit_value), + Type::Value.new, + ) + end + if offset_value + offset_bind = Attribute.with_cast_value( + "OFFSET".freeze, + offset_value.to_i, + Type::Value.new, + ) + end + connection.combine_bind_parameters( + from_clause: from_clause.binds, + join_clause: arel.bind_values, + where_clause: where_clause.binds, + having_clause: having_clause.binds, + limit: limit_bind, + offset: offset_bind, + ) end + FROZEN_EMPTY_HASH = {}.freeze def create_with_value # :nodoc: - @values[:create_with] || {} + @values[:create_with] || FROZEN_EMPTY_HASH end alias extensions extending_values @@ -407,10 +435,30 @@ module ActiveRecord self end - # Performs a joins on +args+: + # Performs a joins on +args+. The given symbol(s) should match the name of + # the association(s). # # User.joins(:posts) - # # SELECT "users".* FROM "users" INNER JOIN "posts" ON "posts"."user_id" = "users"."id" + # # SELECT "users".* + # # FROM "users" + # # INNER JOIN "posts" ON "posts"."user_id" = "users"."id" + # + # Multiple joins: + # + # User.joins(:posts, :account) + # # SELECT "users".* + # # FROM "users" + # # INNER JOIN "posts" ON "posts"."user_id" = "users"."id" + # # INNER JOIN "accounts" ON "accounts"."id" = "users"."account_id" + # + # Nested joins: + # + # User.joins(posts: [:comments]) + # # SELECT "users".* + # # FROM "users" + # # INNER JOIN "posts" ON "posts"."user_id" = "users"."id" + # # INNER JOIN "comments" "comments_posts" + # # ON "comments_posts"."post_id" = "posts"."id" # # You can use strings in order to customize your joins: # @@ -609,16 +657,22 @@ module ActiveRecord # they must differ only by #where (if no #group has been defined) or #having (if a #group is # present). Neither relation may have a #limit, #offset, or #distinct set. # - # Post.where("id = 1").or(Post.where("id = 2")) - # # SELECT `posts`.* FROM `posts` WHERE (('id = 1' OR 'id = 2')) + # Post.where("id = 1").or(Post.where("author_id = 3")) + # # SELECT `posts`.* FROM `posts` WHERE (('id = 1' OR 'author_id = 3')) # def or(other) + unless other.is_a? Relation + raise ArgumentError, "You have passed #{other.class.name} object to #or. Pass an ActiveRecord::Relation object instead." + end + spawn.or!(other) end def or!(other) # :nodoc: - unless structurally_compatible_for_or?(other) - raise ArgumentError, 'Relation passed to #or must be structurally compatible' + incompatible_values = structurally_incompatible_values_for_or(other) + + unless incompatible_values.empty? + raise ArgumentError, "Relation passed to #or must be structurally compatible. Incompatible values: #{incompatible_values}" end self.where_clause = self.where_clause.or(other.where_clause) @@ -653,6 +707,13 @@ module ActiveRecord end def limit!(value) # :nodoc: + if string_containing_comma?(value) + # Remove `string_containing_comma?` when removing this deprecation + ActiveSupport::Deprecation.warn(<<-WARNING.squish) + Passing a string to limit in the form "1,2" is deprecated and will be + removed in Rails 5.1. Please call `offset` explicitly instead. + WARNING + end self.limit_value = value self end @@ -903,8 +964,14 @@ module ActiveRecord arel.where(where_clause.ast) unless where_clause.empty? arel.having(having_clause.ast) unless having_clause.empty? - arel.take(connection.sanitize_limit(limit_value)) if limit_value - arel.skip(offset_value.to_i) if offset_value + if limit_value + if string_containing_comma?(limit_value) + arel.take(connection.sanitize_limit(limit_value)) + else + arel.take(Arel::Nodes::BindParam.new) + end + end + arel.skip(Arel::Nodes::BindParam.new) if offset_value arel.group(*arel_columns(group_values.uniq.reject(&:blank?))) unless group_values.empty? build_order(arel) @@ -941,12 +1008,6 @@ module ActiveRecord self.send(unscope_code, result) end - def association_for_table(table_name) - table_name = table_name.to_s - @klass._reflect_on_association(table_name) || - @klass._reflect_on_association(table_name.singularize) - end - def build_from opts = from_clause.value name = from_clause.name @@ -1036,8 +1097,8 @@ module ActiveRecord def arel_columns(columns) columns.map do |field| - if (Symbol === field || String === field) && columns_hash.key?(field.to_s) && !from_clause.value - arel_table[field] + if (Symbol === field || String === field) && (klass.has_attribute?(field) || klass.attribute_alias?(field)) && !from_clause.value + arel_attribute(field) elsif Symbol === field connection.quote_table_name(field.to_s) else @@ -1047,14 +1108,23 @@ module ActiveRecord end def reverse_sql_order(order_query) - order_query = ["#{quoted_table_name}.#{quoted_primary_key} ASC"] if order_query.empty? + if order_query.empty? + return [arel_attribute(primary_key).desc] if primary_key + raise IrreversibleOrderError, + "Relation has no current order and table has no primary key to be used as default order" + end order_query.flat_map do |o| case o + when Arel::Attribute + o.desc when Arel::Nodes::Ordering o.reverse when String - o.to_s.split(',').map! do |s| + if does_not_support_reverse?(o) + raise IrreversibleOrderError, "Order #{o.inspect} can not be reversed automatically" + end + o.split(',').map! do |s| s.strip! s.gsub!(/\sasc\Z/i, ' DESC') || s.gsub!(/\sdesc\Z/i, ' ASC') || s.concat(' DESC') end @@ -1064,6 +1134,13 @@ module ActiveRecord end end + def does_not_support_reverse?(order) + #uses sql function with multiple arguments + order =~ /\([^()]*,[^()]*\)/ || + # uses "nulls first" like construction + order =~ /nulls (first|last)\Z/i + end + def build_order(arel) orders = order_values.uniq orders.reject!(&:blank?) @@ -1099,12 +1176,10 @@ module ActiveRecord order_args.map! do |arg| case arg when Symbol - arg = klass.attribute_alias(arg) if klass.attribute_alias?(arg) - table[arg].asc + arel_attribute(arg).asc when Hash arg.map { |field, dir| - field = klass.attribute_alias(field) if klass.attribute_alias?(field) - table[field].send(dir.downcase) + arel_attribute(field).send(dir.downcase) } else arg @@ -1134,10 +1209,10 @@ module ActiveRecord end end - def structurally_compatible_for_or?(other) - Relation::SINGLE_VALUE_METHODS.all? { |m| send("#{m}_value") == other.send("#{m}_value") } && - (Relation::MULTI_VALUE_METHODS - [:extending]).all? { |m| send("#{m}_values") == other.send("#{m}_values") } && - (Relation::CLAUSE_METHODS - [:having, :where]).all? { |m| send("#{m}_clause") != other.send("#{m}_clause") } + def structurally_incompatible_values_for_or(other) + Relation::SINGLE_VALUE_METHODS.reject { |m| send("#{m}_value") == other.send("#{m}_value") } + + (Relation::MULTI_VALUE_METHODS - [:extending]).reject { |m| send("#{m}_values") == other.send("#{m}_values") } + + (Relation::CLAUSE_METHODS - [:having, :where]).reject { |m| send("#{m}_clause") == other.send("#{m}_clause") } end def new_where_clause @@ -1153,5 +1228,9 @@ module ActiveRecord def new_from_clause Relation::FromClause.empty end + + def string_containing_comma?(value) + ::String === value && value.include?(",") + end end end diff --git a/activerecord/lib/active_record/relation/record_fetch_warning.rb b/activerecord/lib/active_record/relation/record_fetch_warning.rb index 0a1814b3dd..dbd08811fa 100644 --- a/activerecord/lib/active_record/relation/record_fetch_warning.rb +++ b/activerecord/lib/active_record/relation/record_fetch_warning.rb @@ -24,9 +24,7 @@ module ActiveRecord end # :stopdoc: - ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| - payload = args.last - + ActiveSupport::Notifications.subscribe("sql.active_record") do |*, payload| QueryRegistry.queries << payload[:sql] end # :startdoc: @@ -34,14 +32,14 @@ module ActiveRecord class QueryRegistry # :nodoc: extend ActiveSupport::PerThreadRegistry - attr_accessor :queries + attr_reader :queries def initialize - reset + @queries = [] end def reset - @queries = [] + @queries.clear end end end diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 5c3318651a..d5c18a2a4a 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -12,6 +12,7 @@ module ActiveRecord # Merges in the conditions from <tt>other</tt>, if <tt>other</tt> is an ActiveRecord::Relation. # Returns an array representing the intersection of the resulting records with <tt>other</tt>, if <tt>other</tt> is an array. + # # Post.where(published: true).joins(:comments).merge( Comment.where(spam: false) ) # # Performs a single join query with both where conditions. # @@ -28,7 +29,7 @@ module ActiveRecord # This is mainly intended for sharing common conditions between multiple associations. def merge(other) if other.is_a?(Array) - to_a & other + records & other elsif other spawn.merge!(other) else @@ -37,11 +38,14 @@ module ActiveRecord end def merge!(other) # :nodoc: - if !other.is_a?(Relation) && other.respond_to?(:to_proc) + if other.is_a?(Hash) + Relation::HashMerger.new(self, other).merge + elsif other.is_a?(Relation) + Relation::Merger.new(self, other).merge + elsif other.respond_to?(:to_proc) instance_exec(&other) else - klass = other.is_a?(Hash) ? Relation::HashMerger : Relation::Merger - klass.new(self, other).merge + raise ArgumentError, "#{other.inspect} is not an ActiveRecord::Relation" end end diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index 1f000b3f0f..89396b518c 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -81,7 +81,7 @@ module ActiveRecord end def self.empty - new([], []) + @empty ||= new([], []) end protected @@ -158,8 +158,9 @@ module ActiveRecord end end + ARRAY_WITH_EMPTY_STRING = [''] def non_empty_predicates - predicates - [''] + predicates - ARRAY_WITH_EMPTY_STRING end def wrap_sql_literal(node) diff --git a/activerecord/lib/active_record/relation/where_clause_factory.rb b/activerecord/lib/active_record/relation/where_clause_factory.rb index a81ff98e49..dbf172a577 100644 --- a/activerecord/lib/active_record/relation/where_clause_factory.rb +++ b/activerecord/lib/active_record/relation/where_clause_factory.rb @@ -22,6 +22,7 @@ module ActiveRecord parts = predicate_builder.build_from_hash(attributes) when Arel::Nodes::Node parts = [opts] + binds = other else raise ArgumentError, "Unsupported argument type: #{opts} (#{opts.class})" end |