diff options
Diffstat (limited to 'activerecord/lib/active_record/relation')
15 files changed, 682 insertions, 320 deletions
diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index 49b01909c6..4f0502ae75 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -1,4 +1,3 @@ - module ActiveRecord module Batches # Looping through a collection of records from the database @@ -28,7 +27,7 @@ module ActiveRecord # # ==== Options # * <tt>:batch_size</tt> - Specifies the size of the batch. Default to 1000. - # * <tt>:start</tt> - Specifies the starting point for the batch processing. + # * <tt>:start</tt> - Specifies the primary key value to start from. # 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 @@ -41,8 +40,8 @@ module ActiveRecord # # NOTE: It's not possible to set the order. That is automatically set to # ascending on the primary key ("id ASC") to make the batch ordering - # work. This also means that this method only works with integer-based - # primary keys. + # work. This also means that this method only works when the primary key is + # orderable (e.g. an integer or string). # # NOTE: You can't set the limit either, that's used to control # the batch sizes. @@ -52,7 +51,9 @@ module ActiveRecord records.each { |record| yield record } end else - enum_for :find_each, options + enum_for :find_each, options do + options[:start] ? where(table[primary_key].gteq(options[:start])).size : size + end end end @@ -64,9 +65,19 @@ module ActiveRecord # group.each { |person| person.party_all_night! } # end # + # If you do not provide a block to #find_in_batches, it will return an Enumerator + # for chaining with other methods: + # + # Person.find_in_batches.with_index do |group, batch| + # puts "Processing group ##{batch}" + # group.each(&:recover_from_last_night!) + # end + # + # To be yielded each record one by one, use #find_each instead. + # # ==== Options # * <tt>:batch_size</tt> - Specifies the size of the batch. Default to 1000. - # * <tt>:start</tt> - Specifies the starting point for the batch processing. + # * <tt>:start</tt> - Specifies the primary key value to start from. # 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 @@ -79,8 +90,8 @@ module ActiveRecord # # NOTE: It's not possible to set the order. That is automatically set to # ascending on the primary key ("id ASC") to make the batch ordering - # work. This also means that this method only works with integer-based - # primary keys. + # work. This also means that this method only works when the primary key is + # orderable (e.g. an integer or string). # # NOTE: You can't set the limit either, that's used to control # the batch sizes. @@ -88,30 +99,33 @@ module ActiveRecord options.assert_valid_keys(:start, :batch_size) relation = self + start = options[:start] + batch_size = options[:batch_size] || 1000 + + unless block_given? + return to_enum(:find_in_batches, options) do + total = start ? where(table[primary_key].gteq(start)).size : size + (total - 1).div(batch_size) + 1 + end + 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") end - start = options.delete(:start) - batch_size = options.delete(:batch_size) || 1000 - relation = relation.reorder(batch_order).limit(batch_size) records = start ? relation.where(table[primary_key].gteq(start)).to_a : relation.to_a while records.any? records_size = records.size primary_key_offset = records.last.id + raise "Primary key not included in the custom select clause" unless primary_key_offset yield records break if records_size < batch_size - if primary_key_offset - records = relation.where(table[primary_key].gt(primary_key_offset)).to_a - else - raise "Primary key not included in the custom select clause" - end + records = relation.where(table[primary_key].gt(primary_key_offset)).to_a end end diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 45ffb99868..1d4cb1a83b 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -19,21 +19,32 @@ module ActiveRecord # # Person.group(:city).count # # => { 'Rome' => 5, 'Paris' => 3 } - def count(column_name = nil, options = {}) - # TODO: Remove options argument as soon we remove support to - # activerecord-deprecated_finders. - column_name, options = nil, column_name if column_name.is_a?(Hash) - calculate(:count, column_name, options) + # + # If +count+ is used with +group+ for multiple columns, it returns a Hash whose + # keys are an array containing the individual values of each column and the value + # of each key would be the +count+. + # + # Article.group(:status, :category).count + # # => {["draft", "business"]=>10, ["draft", "technology"]=>4, + # ["published", "business"]=>0, ["published", "technology"]=>2} + # + # If +count+ is used with +select+, it will count the selected columns: + # + # Person.select(:age).count + # # => counts the number of different age values + # + # Note: not all valid +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) end # Calculates the average value on a given column. Returns +nil+ if there's # no row. See +calculate+ for examples with options. # # Person.average(:age) # => 35.8 - def average(column_name, options = {}) - # TODO: Remove options argument as soon we remove support to - # activerecord-deprecated_finders. - calculate(:average, column_name, options) + def average(column_name) + calculate(:average, column_name) end # Calculates the minimum value on a given column. The value is returned @@ -41,10 +52,8 @@ module ActiveRecord # +calculate+ for examples with options. # # Person.minimum(:age) # => 7 - def minimum(column_name, options = {}) - # TODO: Remove options argument as soon we remove support to - # activerecord-deprecated_finders. - calculate(:minimum, column_name, options) + def minimum(column_name) + calculate(:minimum, column_name) end # Calculates the maximum value on a given column. The value is returned @@ -52,10 +61,8 @@ module ActiveRecord # +calculate+ for examples with options. # # Person.maximum(:age) # => 93 - def maximum(column_name, options = {}) - # TODO: Remove options argument as soon we remove support to - # activerecord-deprecated_finders. - calculate(:maximum, column_name, options) + def maximum(column_name) + calculate(:maximum, column_name) end # Calculates the sum of values on a given column. The value is returned @@ -98,17 +105,15 @@ module ActiveRecord # Person.group(:last_name).having("min(age) > 17").minimum(:age) # # Person.sum("2 * age") - def calculate(operation, column_name, options = {}) - # TODO: Remove options argument as soon we remove support to - # activerecord-deprecated_finders. + def calculate(operation, column_name) if column_name.is_a?(Symbol) && attribute_alias?(column_name) column_name = attribute_alias(column_name) end if has_include?(column_name) - construct_relation_for_association_calculations.calculate(operation, column_name, options) + construct_relation_for_association_calculations.calculate(operation, column_name) else - perform_calculation(operation, column_name, options) + perform_calculation(operation, column_name) end end @@ -161,19 +166,8 @@ module ActiveRecord relation.select_values = column_names.map { |cn| columns_hash.key?(cn) ? arel_table[cn] : cn } - result = klass.connection.select_all(relation.arel, nil, bind_values) - columns = result.columns.map do |key| - klass.column_types.fetch(key) { - result.column_types.fetch(key) { result.identity_type } - } - end - - result = result.map do |attributes| - values = klass.initialize_attributes(attributes).values - - columns.zip(values).map { |column, value| column.type_cast value } - end - columns.one? ? result.map!(&:first) : result + result = klass.connection.select_all(relation.arel, nil, relation.arel.bind_values + bind_values) + result.cast_values(klass.column_types) end end @@ -188,12 +182,10 @@ module ActiveRecord private def has_include?(column_name) - eager_loading? || (includes_values.present? && (column_name || references_eager_loaded_tables?)) + eager_loading? || (includes_values.present? && ((column_name && column_name != :all) || references_eager_loaded_tables?)) end - def perform_calculation(operation, column_name, options = {}) - # TODO: Remove options argument as soon we remove support to - # activerecord-deprecated_finders. + def perform_calculation(operation, column_name) operation = operation.to_s.downcase # If #count is used with #distinct / #uniq it is considered distinct. (eg. relation.distinct.count) @@ -231,31 +223,36 @@ module ActiveRecord def execute_simple_calculation(operation, column_name, distinct) #:nodoc: # Postgresql doesn't like ORDER BY when there are no GROUP BY - relation = reorder(nil) + relation = unscope(:order) column_alias = column_name + bind_values = nil + if operation == "count" && (relation.limit_value || relation.offset_value) # Shortcut when limit is zero. return 0 if relation.limit_value == 0 query_builder = build_count_subquery(relation, column_name, distinct) + bind_values = query_builder.bind_values + relation.bind_values else column = aggregate_column(column_name) select_value = operation_over_aggregate_column(column, operation, distinct) column_alias = select_value.alias + column_alias ||= @klass.connection.column_name_for_operation(operation, select_value) relation.select_values = [select_value] query_builder = relation.arel + bind_values = query_builder.bind_values + relation.bind_values end - result = @klass.connection.select_all(query_builder, nil, relation.bind_values) + result = @klass.connection.select_all(query_builder, nil, bind_values) row = result.first value = row && row.values.first column = result.column_types.fetch(column_alias) do - column_for(column_name) + type_for(column_name) end type_cast_calculated_value(value, column, operation) @@ -265,8 +262,8 @@ module ActiveRecord group_attrs = group_values if group_attrs.first.respond_to?(:to_sym) - association = @klass.reflect_on_association(group_attrs.first.to_sym) - associated = group_attrs.size == 1 && association && association.macro == :belongs_to # only count belongs_to associations + association = @klass._reflect_on_association(group_attrs.first) + associated = group_attrs.size == 1 && association && association.belongs_to? # only count belongs_to associations group_fields = Array(associated ? association.foreign_key : group_attrs) else group_fields = group_attrs @@ -307,7 +304,7 @@ module ActiveRecord relation.group_values = group relation.select_values = select_values - calculated_data = @klass.connection.select_all(relation, nil, bind_values) + calculated_data = @klass.connection.select_all(relation, nil, relation.arel.bind_values + bind_values) if association key_ids = calculated_data.collect { |row| row[group_aliases.first] } @@ -318,14 +315,14 @@ module ActiveRecord Hash[calculated_data.map do |row| key = group_columns.map { |aliaz, col_name| column = calculated_data.column_types.fetch(aliaz) do - column_for(col_name) + type_for(col_name) end type_cast_calculated_value(row[aliaz], column) } key = key.first if key.size == 1 key = key_records[key] if associated - column_type = calculated_data.column_types.fetch(aggregate_alias) { column_for(column_name) } + column_type = calculated_data.column_types.fetch(aggregate_alias) { type_for(column_name) } [key, type_cast_calculated_value(row[aggregate_alias], column_type, operation)] end] end @@ -352,24 +349,20 @@ module ActiveRecord @klass.connection.table_alias_for(table_name) end - def column_for(field) + def type_for(field) field_name = field.respond_to?(:name) ? field.name.to_s : field.to_s.split('.').last - @klass.columns_hash[field_name] + @klass.type_for_attribute(field_name) end - def type_cast_calculated_value(value, column, operation = nil) + def type_cast_calculated_value(value, type, operation = nil) case operation when 'count' then value.to_i - when 'sum' then type_cast_using_column(value || 0, column) + when 'sum' then type.type_cast_from_database(value || 0) when 'average' then value.respond_to?(:to_d) ? value.to_d : value - else type_cast_using_column(value, column) + else type.type_cast_from_database(value) end end - def type_cast_using_column(value, column) - column ? column.type_cast(value) : value - end - # TODO: refactor to allow non-string `select_values` (eg. Arel nodes). def select_for_count if select_values.present? @@ -385,9 +378,11 @@ module ActiveRecord aliased_column = aggregate_column(column_name == :all ? 1 : column_name).as(column_alias) relation.select_values = [aliased_column] - subquery = relation.arel.as(subquery_alias) + arel = relation.arel + subquery = arel.as(subquery_alias) sm = Arel::SelectManager.new relation.engine + sm.bind_values = arel.bind_values select_value = operation_over_aggregate_column(column_alias, 'count', distinct) sm.project(select_value).from(subquery) end diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index 21beed332f..d4a8823cfe 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -1,6 +1,5 @@ require 'set' require 'active_support/concern' -require 'active_support/deprecation' module ActiveRecord module Delegation # :nodoc: @@ -40,10 +39,10 @@ module ActiveRecord BLACKLISTED_ARRAY_METHODS = [ :compact!, :flatten!, :reject!, :reverse!, :rotate!, :map!, :shuffle!, :slice!, :sort!, :sort_by!, :delete_if, - :keep_if, :pop, :shift, :delete_at, :compact + :keep_if, :pop, :shift, :delete_at, :compact, :select! ].to_set # :nodoc: - delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, to: :to_a + delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, :join, to: :to_a delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :connection, :columns_hash, :to => :klass diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 4984dbd277..088bc203b7 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -1,3 +1,6 @@ +require 'active_support/deprecation' +require 'active_support/core_ext/string/filters' + module ActiveRecord module FinderMethods ONE_AS_ONE = '1 AS one' @@ -63,7 +66,7 @@ module ActiveRecord # # returns an Array of the required fields, available since Rails 3.1. def find(*args) if block_given? - to_a.find { |*block_args| yield(*block_args) } + to_a.find(*args) { |*block_args| yield(*block_args) } else find_with_ids(*args) end @@ -79,12 +82,16 @@ module ActiveRecord # Post.find_by "published_at < ?", 2.weeks.ago def find_by(*args) where(*args).take + rescue RangeError + nil end # Like <tt>find_by</tt>, except that if no record is found, raises # an <tt>ActiveRecord::RecordNotFound</tt> error. def find_by!(*args) where(*args).take! + rescue RangeError + raise RecordNotFound, "Couldn't find #{@klass.name} with an out of range value" end # Gives a record (or N records if a parameter is supplied) without any implied @@ -101,7 +108,7 @@ module ActiveRecord # Same as +take+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record # is found. Note that <tt>take!</tt> accepts no arguments. def take! - take or raise RecordNotFound + take or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql(@klass.arel_engine)}]") end # Find the first record (or first N records if a parameter is supplied). @@ -127,16 +134,16 @@ module ActiveRecord # def first(limit = nil) if limit - find_first_with_limit(limit) + find_nth_with_limit(offset_index, limit) else - find_first + find_nth(0, offset_index) end end # Same as +first+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record # is found. Note that <tt>first!</tt> accepts no arguments. def first! - first or raise RecordNotFound + find_nth! 0 end # Find the last record (or last N records if a parameter is supplied). @@ -169,7 +176,87 @@ module ActiveRecord # Same as +last+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record # is found. Note that <tt>last!</tt> accepts no arguments. def last! - last or raise RecordNotFound + last or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql(@klass.arel_engine)}]") + end + + # Find the second record. + # If no order is defined it will order by primary key. + # + # Person.second # returns the second object fetched by SELECT * FROM people + # 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) + end + + # Same as +second+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record + # is found. + def second! + find_nth! 1 + end + + # Find the third record. + # If no order is defined it will order by primary key. + # + # Person.third # returns the third object fetched by SELECT * FROM people + # 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) + end + + # Same as +third+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record + # is found. + def third! + find_nth! 2 + end + + # Find the fourth record. + # If no order is defined it will order by primary key. + # + # Person.fourth # returns the fourth object fetched by SELECT * FROM people + # 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) + end + + # Same as +fourth+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record + # is found. + def fourth! + find_nth! 3 + end + + # Find the fifth record. + # If no order is defined it will order by primary key. + # + # Person.fifth # returns the fifth object fetched by SELECT * FROM people + # 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) + end + + # Same as +fifth+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record + # is found. + def fifth! + find_nth! 4 + end + + # Find the forty-second record. Also known as accessing "the reddit". + # If no order is defined it will order by primary key. + # + # Person.forty_two # returns the forty-second object fetched by SELECT * FROM people + # 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) + end + + # Same as +forty_two+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record + # is found. + def forty_two! + find_nth! 41 end # Returns +true+ if a record exists in the table that matches the +id+ or @@ -200,7 +287,14 @@ module ActiveRecord # Person.exists?(false) # Person.exists? def exists?(conditions = :none) - conditions = conditions.id if Base === conditions + if Base === conditions + 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` + MSG + end + return false if !conditions relation = apply_join_dependency(self, construct_join_dependency) @@ -212,10 +306,12 @@ module ActiveRecord when Array, Hash relation = relation.where(conditions) else - relation = relation.where(table[primary_key].eq(conditions)) if conditions != :none + unless conditions == :none + relation = where(primary_key => conditions) + end end - connection.select_value(relation, "#{name} Exists", relation.bind_values) ? true : false + connection.select_value(relation, "#{name} Exists", relation.arel.bind_values + relation.bind_values) ? true : false end # This method is called whenever no records are found with either a single @@ -227,13 +323,13 @@ module ActiveRecord # the expected number of results should be provided in the +expected_size+ # argument. def raise_record_not_found_exception!(ids, result_size, expected_size) #:nodoc: - conditions = arel.where_sql + conditions = arel.where_sql(@klass.arel_engine) conditions = " [#{conditions}]" if conditions if Array(ids).size == 1 - error = "Couldn't find #{@klass.name} with #{primary_key}=#{ids}#{conditions}" + error = "Couldn't find #{@klass.name} with '#{primary_key}'=#{ids}#{conditions}" else - error = "Couldn't find all #{@klass.name.pluralize} with IDs " + error = "Couldn't find all #{@klass.name.pluralize} with '#{primary_key}': " error << "(#{ids.join(", ")})#{conditions} (found #{result_size} results, but was looking for #{expected_size})" end @@ -242,8 +338,21 @@ module ActiveRecord private + def offset_index + offset_value || 0 + end + def find_with_associations - join_dependency = construct_join_dependency + # NOTE: the JoinDependency constructed here needs to know about + # any joins already present in `self`, so pass them in + # + # failing to do so means that in cases like activerecord/test/cases/associations/inner_join_association_test.rb:136 + # incorrect SQL is generated. In that case, the join dependency for + # SpecialCategorizations is constructed without knowledge of the + # preexisting join in joins_values to categorizations (by way of + # the `has_many :through` for categories). + # + join_dependency = construct_join_dependency(joins_values) aliases = join_dependency.aliases relation = select aliases.columns @@ -255,7 +364,8 @@ module ActiveRecord if ActiveRecord::NullRelation === relation [] else - rows = connection.select_all(relation.arel, 'SQL', relation.bind_values.dup) + arel = relation.arel + rows = connection.select_all(arel, 'SQL', arel.bind_values + relation.bind_values) join_dependency.instantiate(rows, aliases) end end @@ -267,7 +377,15 @@ module ActiveRecord end def construct_relation_for_association_calculations - apply_join_dependency(self, construct_join_dependency(arel.froms.first)) + from = arel.froms.first + if Arel::Table === from + apply_join_dependency(self, construct_join_dependency) + else + # FIXME: as far as I can tell, `from` will always be an Arel::Table. + # There are no tests that test this branch, but presumably it's + # possible for `from` to be a list? + apply_join_dependency(self, construct_join_dependency(from)) + end end def apply_join_dependency(relation, join_dependency) @@ -279,7 +397,7 @@ module ActiveRecord else if relation.limit_value limited_ids = limited_ids_for(relation) - limited_ids.empty? ? relation.none! : relation.where!(table[primary_key].in(limited_ids)) + limited_ids.empty? ? relation.none! : relation.where!(primary_key => limited_ids) end relation.except(:limit, :offset) end @@ -290,13 +408,14 @@ module ActiveRecord "#{quoted_table_name}.#{quoted_primary_key}", relation.order_values) relation = relation.except(:select).select(values).distinct! + arel = relation.arel - id_rows = @klass.connection.select_all(relation.arel, 'SQL', relation.bind_values) + id_rows = @klass.connection.select_all(arel, 'SQL', arel.bind_values + relation.bind_values) id_rows.map {|row| row[primary_key]} end def using_limitable_reflections?(reflections) - reflections.none? { |r| r.collection? } + reflections.none?(&:collection?) end protected @@ -318,15 +437,20 @@ module ActiveRecord 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) - id = id.id if ActiveRecord::Base === id + if ActiveRecord::Base === id + 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` + MSG + end - column = columns_hash[primary_key] - substitute = connection.substitute_at(column, bind_values.length) - relation = where(table[primary_key].eq(substitute)) - relation.bind_values += [[column, id]] + relation = where(primary_key => id) record = relation.take raise_record_not_found_exception!(id, 0, 1) unless record @@ -335,7 +459,7 @@ module ActiveRecord end def find_some(ids) - result = where(table[primary_key].in(ids)).to_a + result = where(primary_key => ids).to_a expected_size = if limit_value && ids.size > limit_value @@ -364,20 +488,28 @@ module ActiveRecord end end - def find_first + def find_nth(index, offset) if loaded? - @records.first + @records[index] else - @first ||= find_first_with_limit(1).first + offset += index + @offsets[offset] ||= find_nth_with_limit(offset, 1).first end end - def find_first_with_limit(limit) - if order_values.empty? && primary_key - order(arel_table[primary_key].asc).limit(limit).to_a - else - limit(limit).to_a - 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)}]") + end + + def find_nth_with_limit(offset, limit) + relation = if order_values.empty? && primary_key + order(arel_table[primary_key].asc) + else + self + end + + relation = relation.offset(offset) unless offset.zero? + relation.limit(limit).to_a end def find_last diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index 182b9ed89c..afb0b208c3 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -13,7 +13,7 @@ module ActiveRecord @hash = hash end - def merge + def merge #:nodoc: Merger.new(relation, other).merge end @@ -22,7 +22,7 @@ module ActiveRecord # build a relation to merge in rather than directly merging # the values. def other - other = Relation.create(relation.klass, relation.table) + other = Relation.create(relation.klass, relation.table, relation.predicate_builder) hash.each { |k, v| if k == :joins if Hash === v @@ -30,6 +30,8 @@ module ActiveRecord else other.joins!(*v) end + elsif k == :select + other._select!(v) else other.send("#{k}!", v) end @@ -62,7 +64,13 @@ module ActiveRecord # expensive), most of the time the value is going to be `nil` or `.blank?`, the only catch is that # `false.blank?` returns `true`, so there needs to be an extra check so that explicit `false` values # don't fall through the cracks. - relation.send("#{name}!", *value) unless value.nil? || (value.blank? && false != value) + unless value.nil? || (value.blank? && false != value) + if name == :select + relation._select!(*value) + else + relation.send("#{name}!", *value) + end + end end merge_multi_values @@ -75,12 +83,12 @@ module ActiveRecord private def merge_joins - return if values[:joins].blank? + return if other.joins_values.blank? if other.klass == relation.klass - relation.joins!(*values[:joins]) + relation.joins!(*other.joins_values) else - joins_dependency, rest = values[:joins].partition do |join| + joins_dependency, rest = other.joins_values.partition do |join| case join when Hash, Symbol, Array true @@ -100,56 +108,43 @@ module ActiveRecord def merge_multi_values lhs_wheres = relation.where_values - rhs_wheres = values[:where] || [] + rhs_wheres = other.where_values lhs_binds = relation.bind_values - rhs_binds = values[:bind] || [] + rhs_binds = other.bind_values removed, kept = partition_overwrites(lhs_wheres, rhs_wheres) where_values = kept + rhs_wheres bind_values = filter_binds(lhs_binds, removed) + rhs_binds - conn = relation.klass.connection - bv_index = 0 - where_values.map! do |node| - if Arel::Nodes::Equality === node && Arel::Nodes::BindParam === node.right - substitute = conn.substitute_at(bind_values[bv_index].first, bv_index) - bv_index += 1 - Arel::Nodes::Equality.new(node.left, substitute) - else - node - end - end - relation.where_values = where_values relation.bind_values = bind_values - if values[:reordering] + if other.reordering_value # override any order specified in the original relation - relation.reorder! values[:order] - elsif values[:order] + relation.reorder! other.order_values + elsif other.order_values # merge in order_values from relation - relation.order! values[:order] + relation.order! other.order_values end - relation.extend(*values[:extending]) unless values[:extending].blank? + relation.extend(*other.extending_values) unless other.extending_values.blank? end def merge_single_values - relation.from_value = values[:from] unless relation.from_value - relation.lock_value = values[:lock] unless relation.lock_value - relation.reverse_order_value = values[:reverse_order] + relation.from_value = other.from_value unless relation.from_value + relation.lock_value = other.lock_value unless relation.lock_value - unless values[:create_with].blank? - relation.create_with_value = (relation.create_with_value || {}).merge(values[:create_with]) + unless other.create_with_value.blank? + relation.create_with_value = (relation.create_with_value || {}).merge(other.create_with_value) end end def filter_binds(lhs_binds, removed_wheres) return lhs_binds if removed_wheres.empty? - set = Set.new removed_wheres.map { |x| x.left.name } + set = Set.new removed_wheres.map { |x| x.left.name.to_s } lhs_binds.dup.delete_if { |col,_| set.include? col.name } end diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 1252af7635..567efce8ae 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -1,82 +1,44 @@ module ActiveRecord class PredicateBuilder # :nodoc: - @handlers = [] - - autoload :RelationHandler, 'active_record/relation/predicate_builder/relation_handler' - autoload :ArrayHandler, 'active_record/relation/predicate_builder/array_handler' - - def self.resolve_column_aliases(klass, hash) - hash = hash.dup - hash.keys.grep(Symbol) do |key| - if klass.attribute_alias? key - hash[klass.attribute_alias(key)] = hash.delete key - end - end - hash + require 'active_record/relation/predicate_builder/array_handler' + require 'active_record/relation/predicate_builder/association_query_handler' + 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/range_handler' + require 'active_record/relation/predicate_builder/relation_handler' + + delegate :resolve_column_aliases, to: :table + + def initialize(table) + @table = table + @handlers = [] + + register_handler(BasicObject, BasicObjectHandler.new(self)) + register_handler(Class, ClassHandler.new(self)) + register_handler(Base, BaseHandler.new(self)) + register_handler(Range, RangeHandler.new(self)) + register_handler(Relation, RelationHandler.new) + register_handler(Array, ArrayHandler.new(self)) + register_handler(AssociationQueryValue, AssociationQueryHandler.new(self)) end - def self.build_from_hash(klass, attributes, default_table) - queries = [] - - attributes.each do |column, value| - table = default_table - - if value.is_a?(Hash) - if value.empty? - queries << '1=0' - else - table = Arel::Table.new(column, default_table.engine) - association = klass.reflect_on_association(column.to_sym) - - value.each do |k, v| - queries.concat expand(association && association.klass, table, k, v) - end - end - else - column = column.to_s - - if column.include?('.') - table_name, column = column.split('.', 2) - table = Arel::Table.new(table_name, default_table.engine) - end - - queries.concat expand(klass, table, column, value) - end - end - - queries + def build_from_hash(attributes) + attributes = convert_dot_notation_to_hash(attributes.stringify_keys) + expand_from_hash(attributes) end - def self.expand(klass, table, column, value) - queries = [] - + def expand(column, value) # 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 klass && reflection = klass.reflect_on_association(column.to_sym) - if reflection.polymorphic? && base_class = polymorphic_base_class_from_value(value) - queries << build(table[reflection.foreign_type], base_class) - end - - column = reflection.foreign_key + if table.associated_with?(column) + value = AssociationQueryValue.new(table.associated_table(column), value) end - queries << build(table[column], value) - queries - end - - def self.polymorphic_base_class_from_value(value) - case value - when Relation - value.klass.base_class - when Array - val = value.compact.first - val.class.base_class if val.is_a?(Base) - when Base - value.class.base_class - end + build(table.arel_attribute(column), value) end def self.references(attributes) @@ -101,25 +63,49 @@ module ActiveRecord # ) # end # ActiveRecord::PredicateBuilder.register_handler(MyCustomDateRange, handler) - def self.register_handler(klass, handler) + def register_handler(klass, handler) @handlers.unshift([klass, handler]) end - register_handler(BasicObject, ->(attribute, value) { attribute.eq(value) }) - # FIXME: I think we need to deprecate this behavior - register_handler(Class, ->(attribute, value) { attribute.eq(value.name) }) - register_handler(Base, ->(attribute, value) { attribute.eq(value.id) }) - register_handler(Range, ->(attribute, value) { attribute.in(value) }) - register_handler(Relation, RelationHandler.new) - register_handler(Array, ArrayHandler.new) + def build(attribute, value) + handler_for(value).call(attribute, value) + end - private - def self.build(attribute, value) - handler_for(value).call(attribute, value) + protected + + attr_reader :table + + def expand_from_hash(attributes) + return ["1=0"] if attributes.empty? + + attributes.flat_map do |key, value| + if value.is_a?(Hash) + builder = self.class.new(table.associated_table(key)) + builder.expand_from_hash(value) + else + expand(key, value) + end end + end + + private + + def convert_dot_notation_to_hash(attributes) + dot_notation = attributes.keys.select { |s| s.include?(".") } + + dot_notation.each do |key| + table_name, column_name = key.split(".") + value = attributes.delete(key) + attributes[table_name] ||= {} - def self.handler_for(object) - @handlers.detect { |klass, _| klass === object }.last + attributes[table_name] = attributes[table_name].merge(column_name => value) end + + attributes + end + + def handler_for(object) + @handlers.detect { |klass, _| klass === object }.last + end end end diff --git a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb index 2f6c34ac08..95dbd6a77f 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb @@ -1,29 +1,43 @@ module ActiveRecord class PredicateBuilder class ArrayHandler # :nodoc: + def initialize(predicate_builder) + @predicate_builder = predicate_builder + end + def call(attribute, value) values = value.map { |x| x.is_a?(Base) ? x.id : x } - ranges, values = values.partition { |v| v.is_a?(Range) } + nils, values = values.partition(&:nil?) - values_predicate = if values.include?(nil) - values = values.compact + return attribute.in([]) if values.empty? && nils.empty? + ranges, values = values.partition { |v| v.is_a?(Range) } + + values_predicate = case values.length - when 0 - attribute.eq(nil) - when 1 - attribute.eq(values.first).or(attribute.eq(nil)) - else - attribute.in(values).or(attribute.eq(nil)) + when 0 then NullPredicate + when 1 then predicate_builder.build(attribute, values.first) + else attribute.in(values) end - else - attribute.in(values) + + unless nils.empty? + values_predicate = values_predicate.or(predicate_builder.build(attribute, nil)) end - array_predicates = ranges.map { |range| attribute.in(range) } - array_predicates << values_predicate + array_predicates = ranges.map { |range| predicate_builder.build(attribute, range) } + array_predicates.unshift(values_predicate) array_predicates.inject { |composite, predicate| composite.or(predicate) } end + + protected + + attr_reader :predicate_builder + + module NullPredicate # :nodoc: + def self.or(other) + other + end + end 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 new file mode 100644 index 0000000000..aabcf20c1d --- /dev/null +++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb @@ -0,0 +1,58 @@ +module ActiveRecord + class PredicateBuilder + class AssociationQueryHandler # :nodoc: + def initialize(predicate_builder) + @predicate_builder = predicate_builder + end + + def call(attribute, value) + queries = {} + + table = value.associated_table + if value.base_class + queries[table.association_foreign_type] = value.base_class.name + end + + queries[table.association_foreign_key] = value.ids + predicate_builder.build_from_hash(queries) + end + + protected + + attr_reader :predicate_builder + end + + class AssociationQueryValue # :nodoc: + attr_reader :associated_table, :value + + def initialize(associated_table, value) + @associated_table = associated_table + @value = value + end + + def ids + value + end + + def base_class + if associated_table.polymorphic_association? + @base_class ||= polymorphic_base_class_from_value + end + end + + private + + def polymorphic_base_class_from_value + case value + when Relation + value.klass.base_class + when Array + val = value.compact.first + val.class.base_class if val.is_a?(Base) + when Base + value.class.base_class + end + end + end + end +end diff --git a/activerecord/lib/active_record/relation/predicate_builder/base_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/base_handler.rb new file mode 100644 index 0000000000..6fa5b16f73 --- /dev/null +++ b/activerecord/lib/active_record/relation/predicate_builder/base_handler.rb @@ -0,0 +1,17 @@ +module ActiveRecord + class PredicateBuilder + class BaseHandler # :nodoc: + def initialize(predicate_builder) + @predicate_builder = predicate_builder + end + + def call(attribute, value) + predicate_builder.build(attribute, value.id) + end + + protected + + attr_reader :predicate_builder + end + end +end diff --git a/activerecord/lib/active_record/relation/predicate_builder/basic_object_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/basic_object_handler.rb new file mode 100644 index 0000000000..6cec75dc0a --- /dev/null +++ b/activerecord/lib/active_record/relation/predicate_builder/basic_object_handler.rb @@ -0,0 +1,17 @@ +module ActiveRecord + class PredicateBuilder + class BasicObjectHandler # :nodoc: + def initialize(predicate_builder) + @predicate_builder = predicate_builder + end + + def call(attribute, value) + attribute.eq(value) + end + + protected + + attr_reader :predicate_builder + end + end +end diff --git a/activerecord/lib/active_record/relation/predicate_builder/class_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/class_handler.rb new file mode 100644 index 0000000000..ed313fc9d4 --- /dev/null +++ b/activerecord/lib/active_record/relation/predicate_builder/class_handler.rb @@ -0,0 +1,27 @@ +module ActiveRecord + class PredicateBuilder + class ClassHandler # :nodoc: + def initialize(predicate_builder) + @predicate_builder = predicate_builder + end + + def call(attribute, value) + print_deprecation_warning + predicate_builder.build(attribute, value.name) + end + + protected + + attr_reader :predicate_builder + + private + + def print_deprecation_warning + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Passing a class as a value in an Active Record query is deprecated and + will be removed. Pass a string instead. + MSG + 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 new file mode 100644 index 0000000000..1b3849e3ad --- /dev/null +++ b/activerecord/lib/active_record/relation/predicate_builder/range_handler.rb @@ -0,0 +1,17 @@ +module ActiveRecord + class PredicateBuilder + class RangeHandler # :nodoc: + def initialize(predicate_builder) + @predicate_builder = predicate_builder + end + + def call(attribute, value) + attribute.between(value) + end + + protected + + attr_reader :predicate_builder + end + end +end 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 618fa3cdd9..063150958a 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb @@ -6,7 +6,7 @@ module ActiveRecord value = value.select(value.klass.arel_table[value.klass.primary_key]) end - attribute.in(value.arel.ast) + attribute.in(value.arel) end end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 979216bee7..d6e6cb4d05 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1,9 +1,13 @@ require 'active_support/core_ext/array/wrap' +require 'active_support/core_ext/string/filters' +require 'active_model/forbidden_attributes_protection' module ActiveRecord module QueryMethods extend ActiveSupport::Concern + include ActiveModel::ForbiddenAttributesProtection + # 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 @@ -49,6 +53,8 @@ module ActiveRecord Arel::Nodes::Not.new(rel) end end + + @scope.references!(PredicateBuilder.references(opts)) if Hash === opts @scope.where_values += where_value @scope end @@ -56,14 +62,14 @@ module ActiveRecord 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) - raise ImmutableRelation if @loaded # raise ImmutableRelation if @loaded - @values[:#{name}] = values # @values[:select] = values - end # end + 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 CODE end @@ -78,7 +84,7 @@ module ActiveRecord Relation::SINGLE_VALUE_METHODS.each do |name| class_eval <<-CODE, __FILE__, __LINE__ + 1 def #{name}_value=(value) # def readonly_value=(value) - raise ImmutableRelation if @loaded # raise ImmutableRelation if @loaded + assert_mutability! # assert_mutability! @values[:#{name}] = value # @values[:readonly] = value end # end CODE @@ -120,6 +126,9 @@ module ActiveRecord # Will throw an error, but this will work: # # User.includes(:posts).where('posts.name = ?', 'example').references(:posts) + # + # Note that +includes+ works with association names while +references+ needs + # the actual table name. def includes(*args) check_if_method_has_arguments!(:includes, args) spawn.includes!(*args) @@ -163,24 +172,26 @@ module ActiveRecord self end - # Used to indicate that an association is referenced by an SQL string, and should - # therefore be JOINed in any query rather than loaded separately. + # Use to indicate that the given +table_names+ are referenced by an SQL string, + # and should therefore be JOINed in any query rather than loaded separately. + # This method only works in conjunction with +includes+. + # See #includes for more details. # # User.includes(:posts).where("posts.name = 'foo'") # # => Doesn't JOIN the posts table, resulting in an error. # # User.includes(:posts).where("posts.name = 'foo'").references(:posts) # # => Query now knows the string references posts, so adds a JOIN - def references(*args) - check_if_method_has_arguments!(:references, args) - spawn.references!(*args) + def references(*table_names) + check_if_method_has_arguments!(:references, table_names) + spawn.references!(*table_names) end - def references!(*args) # :nodoc: - args.flatten! - args.map!(&:to_s) + def references!(*table_names) # :nodoc: + table_names.flatten! + table_names.map!(&:to_s) - self.references_values |= args + self.references_values |= table_names self end @@ -197,7 +208,7 @@ module ActiveRecord # fields are retrieved: # # Model.select(:field) - # # => [#<Model field:value>] + # # => [#<Model id: nil, field: "value">] # # Although in the above example it looks as though this method returns an # array, it actually returns a relation object and can have other query @@ -206,12 +217,12 @@ module ActiveRecord # The argument to the method can also be an array of fields. # # Model.select(:field, :other_field, :and_one_more) - # # => [#<Model field: "value", other_field: "value", and_one_more: "value">] + # # => [#<Model id: nil, field: "value", other_field: "value", and_one_more: "value">] # # You can also use one or more strings, which will be used unchanged as SELECT fields. # # Model.select('field AS field_one', 'other_field AS field_two') - # # => [#<Model field: "value", other_field: "value">] + # # => [#<Model id: nil, field: "value", other_field: "value">] # # If an alias was specified, it will be accessible from the resulting objects: # @@ -219,7 +230,7 @@ module ActiveRecord # # => "value" # # Accessing attributes of an object that do not have fields retrieved by a select - # will throw <tt>ActiveModel::MissingAttributeError</tt>: + # except +id+ will throw <tt>ActiveModel::MissingAttributeError</tt>: # # Model.select(:field).first.other_field # # => ActiveModel::MissingAttributeError: missing attribute: other_field @@ -228,13 +239,15 @@ module ActiveRecord to_a.select { |*block_args| yield(*block_args) } else raise ArgumentError, 'Call this with at least one field' if fields.empty? - spawn.select!(*fields) + spawn._select!(*fields) end end - def select!(*fields) # :nodoc: + def _select!(*fields) # :nodoc: fields.flatten! - + fields.map! do |field| + klass.attribute_alias?(field) ? klass.attribute_alias(field) : field + end self.select_values += fields self end @@ -254,6 +267,10 @@ module ActiveRecord # # User.group('name AS grouped_name, age') # => [#<User id: 3, name: "Foo", age: 21, ...>, #<User id: 2, name: "Oscar", age: 21, ...>, #<User id: 5, name: "Foo", age: 23, ...>] + # + # Passing in an array of attributes to group by is also supported. + # User.select([:id, :first_name]).group(:id, :first_name).first(3) + # => [#<User id: 1, first_name: "Bill">, #<User id: 2, first_name: "Earl">, #<User id: 3, first_name: "Beto">] def group(*args) check_if_method_has_arguments!(:group, args) spawn.group!(*args) @@ -268,15 +285,6 @@ module ActiveRecord # Allows to specify an order attribute: # - # User.order('name') - # => SELECT "users".* FROM "users" ORDER BY name - # - # User.order('name DESC') - # => SELECT "users".* FROM "users" ORDER BY name DESC - # - # User.order('name DESC, email') - # => SELECT "users".* FROM "users" ORDER BY name DESC, email - # # User.order(:name) # => SELECT "users".* FROM "users" ORDER BY "users"."name" ASC # @@ -285,6 +293,15 @@ module ActiveRecord # # User.order(:name, email: :desc) # => SELECT "users".* FROM "users" ORDER BY "users"."name" ASC, "users"."email" DESC + # + # User.order('name') + # => SELECT "users".* FROM "users" ORDER BY name + # + # User.order('name DESC') + # => SELECT "users".* FROM "users" ORDER BY name DESC + # + # User.order('name DESC, email') + # => SELECT "users".* FROM "users" ORDER BY name DESC, email def order(*args) check_if_method_has_arguments!(:order, args) spawn.order!(*args) @@ -398,19 +415,17 @@ module ActiveRecord # => SELECT "users".* FROM "users" LEFT JOIN bookmarks ON bookmarks.bookmarkable_type = 'Post' AND bookmarks.user_id = users.id def joins(*args) check_if_method_has_arguments!(:joins, args) - - args.compact! - args.flatten! - spawn.joins!(*args) end def joins!(*args) # :nodoc: + args.compact! + args.flatten! self.joins_values += args self end - def bind(value) + def bind(value) # :nodoc: spawn.bind!(value) end @@ -548,15 +563,14 @@ module ActiveRecord end end - def where!(opts = :chain, *rest) # :nodoc: - if opts == :chain - WhereChain.new(self) - else - references!(PredicateBuilder.references(opts)) if Hash === opts - - self.where_values += build_where(opts, rest) - self + def where!(opts, *rest) # :nodoc: + if Hash === opts + opts = sanitize_forbidden_attributes(opts) + references!(PredicateBuilder.references(opts)) end + + self.where_values += build_where(opts, rest) + self end # Allows you to change a previously set where condition for a given attribute, instead of appending to that condition. @@ -662,11 +676,11 @@ module ActiveRecord # end # def none - extending(NullRelation) + where("1=0").extending!(NullRelation) end def none! # :nodoc: - extending!(NullRelation) + where!("1=0").extending!(NullRelation) end # Sets readonly attributes for the returned relation. If value is @@ -702,7 +716,13 @@ module ActiveRecord end def create_with!(value) # :nodoc: - self.create_with_value = value ? create_with_value.merge(value) : {} + if value + value = sanitize_forbidden_attributes(value) + self.create_with_value = create_with_value.merge(value) + else + self.create_with_value = {} + end + self end @@ -725,6 +745,9 @@ module ActiveRecord def from!(value, subquery_name = nil) # :nodoc: self.from_value = [value, subquery_name] + if value.is_a? Relation + self.bind_values = value.arel.bind_values + value.bind_values + bind_values + end self end @@ -812,22 +835,30 @@ module ActiveRecord end def reverse_order! # :nodoc: - self.reverse_order_value = !reverse_order_value + orders = order_values.uniq + orders.reject!(&:blank?) + self.order_values = reverse_sql_order(orders) self end # Returns the Arel object associated with the relation. - def arel + def arel # :nodoc: @arel ||= build_arel end - # Like #arel, but ignores the default scope of the model. + private + + def assert_mutability! + raise ImmutableRelation if @loaded + raise ImmutableRelation if defined?(@arel) && @arel + end + def build_arel - arel = Arel::SelectManager.new(table.engine, table) + arel = Arel::SelectManager.new(table) build_joins(arel, joins_values.flatten) unless joins_values.empty? - collapse_wheres(arel, (where_values - ['']).uniq) + collapse_wheres(arel, (where_values - [''])) #TODO: Add uniq with real value comparison / ignore uniqs that have binds arel.having(*having_values.uniq.reject(&:blank?)) unless having_values.empty? @@ -847,8 +878,6 @@ module ActiveRecord arel end - private - def symbol_unscoping(scope) if !VALID_UNSCOPING_VALUES.include?(scope) raise ArgumentError, "Called unscope() with invalid unscoping argument ':#{scope}'. Valid arguments are :#{VALID_UNSCOPING_VALUES.to_a.join(", :")}." @@ -859,8 +888,9 @@ module ActiveRecord case scope when :order - self.reverse_order_value = false result = [] + when :where + self.bind_values = [] else result = [] unless single_val_method end @@ -873,11 +903,9 @@ module ActiveRecord where_values.reject! do |rel| case rel - when Arel::Nodes::In, Arel::Nodes::NotIn, Arel::Nodes::Equality, Arel::Nodes::NotEqual + when Arel::Nodes::Between, Arel::Nodes::In, Arel::Nodes::NotIn, Arel::Nodes::Equality, Arel::Nodes::NotEqual, Arel::Nodes::LessThanOrEqual, Arel::Nodes::GreaterThanOrEqual subrelation = (rel.left.kind_of?(Arel::Attributes::Attribute) ? rel.left : rel.right) - subrelation.name == target_value - else - raise "unscope(where: #{target_value.inspect}) failed: unscoping #{rel.class} is unimplemented." + subrelation.name.to_s == target_value end end @@ -913,34 +941,67 @@ module ActiveRecord def build_where(opts, other = []) case opts when String, Array - #TODO: Remove duplication with: /activerecord/lib/active_record/sanitization.rb:113 - values = Hash === other.first ? other.first.values : other - - values.grep(ActiveRecord::Relation) do |rel| - self.bind_values += rel.bind_values - end - [@klass.send(:sanitize_sql, other.empty? ? opts : ([opts] + other))] when Hash - opts = PredicateBuilder.resolve_column_aliases(klass, opts) - attributes = @klass.send(:expand_hash_conditions_for_aggregates, opts) + opts = predicate_builder.resolve_column_aliases(opts) - attributes.values.grep(ActiveRecord::Relation) do |rel| - self.bind_values += rel.bind_values - end + tmp_opts, bind_values = create_binds(opts) + self.bind_values += bind_values + + attributes = @klass.send(:expand_hash_conditions_for_aggregates, tmp_opts) + add_relations_to_bind_values(attributes) - PredicateBuilder.build_from_hash(klass, attributes, table) + predicate_builder.build_from_hash(attributes) else [opts] end end + def create_binds(opts) + bindable, non_binds = opts.partition do |column, value| + case value + when String, Integer, ActiveRecord::StatementCache::Substitute + @klass.columns_hash.include? column.to_s + else + false + end + end + + association_binds, non_binds = non_binds.partition do |column, value| + value.is_a?(Hash) && association_for_table(column) + end + + new_opts = {} + binds = [] + + bindable.each do |(column,value)| + binds.push [@klass.columns_hash[column.to_s], value] + new_opts[column] = connection.substitute_at(column) + end + + association_binds.each do |(column, value)| + association_relation = association_for_table(column).klass.send(:relation) + association_new_opts, association_bind = association_relation.send(:create_binds, value) + new_opts[column] = association_new_opts + binds += association_bind + end + + non_binds.each { |column,value| new_opts[column] = value } + + [new_opts, binds] + 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, name = from_value case opts when Relation name ||= 'subquery' - self.bind_values = opts.bind_values + self.bind_values opts.arel.as(name.to_s) else opts @@ -976,9 +1037,12 @@ module ActiveRecord join_list ) - joins = join_dependency.join_constraints stashed_association_joins + join_infos = join_dependency.join_constraints stashed_association_joins - joins.each { |join| manager.from(join) } + join_infos.each do |info| + info.joins.each { |join| manager.from(join) } + manager.bind_values.concat info.binds + end manager.join_sources.concat(join_list) @@ -987,9 +1051,15 @@ module ActiveRecord def build_select(arel, selects) if !selects.empty? - arel.project(*selects) - elsif from_value - arel.project(Arel.star) + expanded_select = selects.map do |field| + if (Symbol === field || String === field) && columns_hash.key?(field.to_s) + arel_table[field] + else + field + end + end + + arel.project(*expanded_select) else arel.project(@klass.arel_table[Arel.star]) end @@ -1020,15 +1090,19 @@ module ActiveRecord def build_order(arel) orders = order_values.uniq orders.reject!(&:blank?) - orders = reverse_sql_order(orders) if reverse_order_value arel.order(*orders) unless orders.empty? end + VALID_DIRECTIONS = [:asc, :desc, :ASC, :DESC, + 'asc', 'desc', 'ASC', 'DESC'] # :nodoc: + def validate_order_args(args) - args.grep(Hash) do |h| - unless (h.values - [:asc, :desc]).empty? - raise ArgumentError, 'Direction should be :asc or :desc' + args.each do |arg| + next unless arg.is_a?(Hash) + arg.each do |_key, value| + raise ArgumentError, "Direction \"#{value}\" is invalid. Valid " \ + "directions are: #{VALID_DIRECTIONS.inspect}" unless VALID_DIRECTIONS.include?(value) end end end @@ -1045,10 +1119,12 @@ module ActiveRecord order_args.map! do |arg| case arg when Symbol + arg = klass.attribute_alias(arg) if klass.attribute_alias?(arg) table[arg].asc when Hash arg.map { |field, dir| - table[field].send(dir) + field = klass.attribute_alias(field) if klass.attribute_alias?(field) + table[field].send(dir.downcase) } else arg @@ -1077,5 +1153,17 @@ module ActiveRecord raise ArgumentError, "The method .#{method_name}() must contain arguments." end end + + def add_relations_to_bind_values(attributes) + if attributes.is_a?(Hash) + attributes.each_value do |value| + if value.is_a?(ActiveRecord::Relation) + self.bind_values += value.arel.bind_values + value.bind_values + else + add_relations_to_bind_values(value) + end + end + end + end end end diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 2552cbd234..01bddea6c9 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -58,13 +58,16 @@ module ActiveRecord # Post.order('id asc').only(:where) # discards the order condition # Post.order('id asc').only(:where, :order) # uses the specified order def only(*onlies) + if onlies.any? { |o| o == :where } + onlies << :bind + end relation_with values.slice(*onlies) end private def relation_with(values) # :nodoc: - result = Relation.create(klass, table, values) + result = Relation.create(klass, table, predicate_builder, values) result.extend(*extending_values) if extending_values.any? result end |