diff options
Diffstat (limited to 'activerecord/lib/active_record/relation')
14 files changed, 206 insertions, 151 deletions
diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index fa19c679cf..561869017a 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative "batches/batch_enumerator" +require "active_record/relation/batches/batch_enumerator" module ActiveRecord module Batches @@ -251,20 +251,27 @@ module ActiveRecord end end - batch_relation = relation.where(arel_attribute(primary_key).gt(primary_key_offset)) + attr = Relation::QueryAttribute.new(primary_key, primary_key_offset, klass.type_for_attribute(primary_key)) + batch_relation = relation.where(arel_attribute(primary_key).gt(Arel::Nodes::BindParam.new(attr))) end end private 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 + if start + attr = Relation::QueryAttribute.new(primary_key, start, klass.type_for_attribute(primary_key)) + relation = relation.where(arel_attribute(primary_key).gteq(Arel::Nodes::BindParam.new(attr))) + end + if finish + attr = Relation::QueryAttribute.new(primary_key, finish, klass.type_for_attribute(primary_key)) + relation = relation.where(arel_attribute(primary_key).lteq(Arel::Nodes::BindParam.new(attr))) + end relation end def batch_order - "#{quoted_table_name}.#{quoted_primary_key} ASC" + arel_attribute(primary_key).asc end def act_on_ignored_order(error_on_ignore) diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 42d43224fa..cb0b06cfdc 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -130,7 +130,7 @@ module ActiveRecord # end def calculate(operation, column_name) if has_include?(column_name) - relation = construct_relation_for_association_calculations + relation = apply_join_dependency relation.distinct! if operation.to_s.downcase == "count" relation.calculate(operation, column_name) @@ -180,8 +180,10 @@ module ActiveRecord end if has_include?(column_names.first) - construct_relation_for_association_calculations.pluck(*column_names) + relation = apply_join_dependency + relation.pluck(*column_names) else + enforce_raw_sql_whitelist(column_names) relation = spawn relation.select_values = column_names.map { |cn| @klass.has_attribute?(cn) || @klass.attribute_alias?(cn) ? arel_attribute(cn) : cn @@ -215,7 +217,7 @@ module ActiveRecord if operation == "count" column_name ||= select_for_count if column_name == :all - if distinct && !(has_limit_or_offset? && order_values.any?) + if distinct && (group_values.any? || select_values.empty? && order_values.empty?) column_name = primary_key end elsif column_name =~ /\s*DISTINCT[\s(]+/i @@ -247,7 +249,7 @@ module ActiveRecord def execute_simple_calculation(operation, column_name, distinct) #:nodoc: column_alias = column_name - if operation == "count" && has_limit_or_offset? + if operation == "count" && (column_name == :all && distinct || has_limit_or_offset?) # Shortcut when limit is zero. return 0 if limit_value == 0 @@ -389,14 +391,12 @@ module ActiveRecord end def build_count_subquery(relation, column_name, distinct) - relation.select_values = [ - if column_name == :all - distinct ? table[Arel.star] : Arel.sql("1") - else - column_alias = Arel.sql("count_column") - aggregate_column(column_name).as(column_alias) - end - ] + if column_name == :all + relation.select_values = [ Arel.sql(FinderMethods::ONE_AS_ONE) ] unless distinct + else + column_alias = Arel.sql("count_column") + relation.select_values = [ aggregate_column(column_name).as(column_alias) ] + end subquery = relation.arel.as(Arel.sql("subquery_for_count")) select_value = operation_over_aggregate_column(column_alias || Arel.star, "count", false) diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index 1aa85993ca..4863befec8 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -38,13 +38,12 @@ module ActiveRecord # may vary depending on the klass of a relation, so we create a subclass of Relation # for each different klass, and the delegations are compiled into that subclass only. - delegate :to_xml, :encode_with, :length, :each, :uniq, :to_ary, :join, - :[], :&, :|, :+, :-, :sample, :reverse, :compact, :in_groups, :in_groups_of, + delegate :to_xml, :encode_with, :length, :each, :uniq, :join, + :[], :&, :|, :+, :-, :sample, :reverse, :rotate, :compact, :in_groups, :in_groups_of, :to_sentence, :to_formatted_s, :as_json, - :shuffle, :split, :index, to: :records + :shuffle, :split, :slice, :index, :rindex, to: :records - delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, - :connection, :columns_hash, to: :klass + delegate :primary_key, :connection, to: :klass module ClassSpecificRelation # :nodoc: extend ActiveSupport::Concern diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 2aed941916..5f959af5dc 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -18,9 +18,10 @@ module ActiveRecord # Person.find([1]) # returns an array for the object with ID = 1 # Person.where("administrator = 1").order("created_on DESC").find(1) # - # NOTE: The returned records may not be in the same order as the ids you - # provide since database rows are unordered. You will need to provide an explicit QueryMethods#order - # option if you want the results to be sorted. + # NOTE: The returned records are in the same order as the ids you provide. + # If you want the results to be sorted by database, you can use ActiveRecord::QueryMethods#where + # method and provide an explicit ActiveRecord::QueryMethods#order option. + # But ActiveRecord::QueryMethods#where method doesn't raise ActiveRecord::RecordNotFound. # # ==== Find with lock # @@ -88,7 +89,7 @@ module ActiveRecord where(arg, *args).take! rescue ::RangeError raise RecordNotFound.new("Couldn't find #{@klass.name} with an out of range value", - @klass.name) + @klass.name, @klass.primary_key) end # Gives a record (or N records if a parameter is supplied) without any implied @@ -147,7 +148,7 @@ module ActiveRecord # # [#<Person id:4>, #<Person id:3>, #<Person id:2>] def last(limit = nil) - return find_last(limit) if loaded? || limit_value + return find_last(limit) if loaded? || has_limit_or_offset? result = ordered_relation.limit(limit) result = result.reverse_order! @@ -284,7 +285,7 @@ module ActiveRecord # * Hash - Finds the record that matches these +find+-style conditions # (such as <tt>{name: 'David'}</tt>). # * +false+ - Returns always +false+. - # * No args - Returns +false+ if the table is empty, +true+ otherwise. + # * No args - Returns +false+ if the relation is empty, +true+ otherwise. # # For more information about specifying conditions as a hash or array, # see the Conditions section in the introduction to ActiveRecord::Base. @@ -300,6 +301,7 @@ module ActiveRecord # Person.exists?(name: 'David') # Person.exists?(false) # Person.exists? + # Person.where(name: 'Spartacus', rating: 4).exists? def exists?(conditions = :none) if Base === conditions raise ArgumentError, <<-MSG.squish @@ -310,12 +312,12 @@ module ActiveRecord return false if !conditions || limit_value == 0 - relation = self unless eager_loading? - relation ||= apply_join_dependency(self, construct_join_dependency(eager_loading: false)) - - return false if ActiveRecord::NullRelation === relation + if eager_loading? + relation = apply_join_dependency(eager_loading: false) + return relation.exists?(conditions) + end - relation = construct_relation_for_exists(relation, conditions) + relation = construct_relation_for_exists(conditions) skip_query_cache_if_necessary { connection.select_value(relation.arel, "#{name} Exists") } ? true : false rescue ::RangeError @@ -338,7 +340,7 @@ module ActiveRecord if ids.nil? error = "Couldn't find #{name}".dup error << " with#{conditions}" if conditions - raise RecordNotFound.new(error, name) + raise RecordNotFound.new(error, name, key) elsif Array(ids).size == 1 error = "Couldn't find #{name} with '#{key}'=#{ids}#{conditions}" raise RecordNotFound.new(error, name, key, ids) @@ -346,7 +348,7 @@ module ActiveRecord error = "Couldn't find all #{name.pluralize} with '#{key}': ".dup error << "(#{ids.join(", ")})#{conditions} (found #{result_size} results, but was looking for #{expected_size})." error << " Couldn't find #{name.pluralize(not_found_ids.size)} with #{key.to_s.pluralize(not_found_ids.size)} #{not_found_ids.join(', ')}." if not_found_ids - raise RecordNotFound.new(error, name, primary_key, ids) + raise RecordNotFound.new(error, name, key, ids) end end @@ -356,27 +358,8 @@ module ActiveRecord offset_value || 0 end - def find_with_associations - # 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 - relation = apply_join_dependency(relation, join_dependency) - - yield relation, join_dependency - end - - def construct_relation_for_exists(relation, conditions) - relation = relation.except(:select, :distinct, :order)._select!(ONE_AS_ONE).limit!(1) + def construct_relation_for_exists(conditions) + relation = except(:select, :distinct, :order)._select!(ONE_AS_ONE).limit!(1) case conditions when Array, Hash @@ -388,32 +371,39 @@ module ActiveRecord relation end - def construct_join_dependency(joins = [], eager_loading: true) + def construct_join_dependency(eager_loading: true) including = eager_load_values + includes_values - ActiveRecord::Associations::JoinDependency.new(klass, table, including, joins, eager_loading: eager_loading) + joins = joins_values.select { |join| join.is_a?(Arel::Nodes::Join) } + ActiveRecord::Associations::JoinDependency.new( + klass, table, including, alias_tracker(joins), eager_loading: eager_loading + ) end - def construct_relation_for_association_calculations - apply_join_dependency(self, construct_join_dependency(joins_values)) - end - - def apply_join_dependency(relation, join_dependency) - relation = relation.except(:includes, :eager_load, :preload).joins!(join_dependency) + def apply_join_dependency(eager_loading: true) + join_dependency = construct_join_dependency(eager_loading: eager_loading) + relation = except(:includes, :eager_load, :preload).joins!(join_dependency) - if using_limitable_reflections?(join_dependency.reflections) - relation - else - if relation.limit_value + if eager_loading && !using_limitable_reflections?(join_dependency.reflections) + if has_limit_or_offset? limited_ids = limited_ids_for(relation) limited_ids.empty? ? relation.none! : relation.where!(primary_key => limited_ids) end - relation.except(:limit, :offset) + relation.limit_value = relation.offset_value = nil + end + + if block_given? + relation._select!(join_dependency.aliases.columns) + yield relation, join_dependency + else + relation end end def limited_ids_for(relation) values = @klass.connection.columns_for_distinct( - "#{quoted_table_name}.#{quoted_primary_key}", relation.order_values) + connection.column_name_from_arel_node(arel_attribute(primary_key)), + relation.order_values + ) relation = relation.except(:select).select(values).distinct! @@ -433,9 +423,12 @@ module ActiveRecord ids = ids.flatten.compact.uniq + model_name = @klass.name + case ids.size when 0 - raise RecordNotFound, "Couldn't find #{@klass.name} without an ID" + error_message = "Couldn't find #{model_name} without an ID" + raise RecordNotFound.new(error_message, model_name, primary_key) when 1 result = find_one(ids.first) expects_array ? [ result ] : result @@ -443,7 +436,8 @@ module ActiveRecord find_some(ids) end rescue ::RangeError - raise RecordNotFound, "Couldn't find #{@klass.name} with an out of range ID" + error_message = "Couldn't find #{model_name} with an out of range ID" + raise RecordNotFound.new(error_message, model_name, primary_key, ids) end def find_one(id) @@ -527,7 +521,11 @@ module ActiveRecord else relation = ordered_relation - if limit_value.nil? || index < limit_value + if limit_value + limit = [limit_value - index, limit].min + end + + if limit > 0 relation = relation.offset(offset_index + index) unless index.zero? relation.limit(limit).to_a else @@ -542,12 +540,11 @@ module ActiveRecord else relation = ordered_relation - 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 + if equal?(relation) || has_limit_or_offset? + relation.records[-index] + else + relation.last(index)[-index] + end end end diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index 03824ffff9..ebdd4144bb 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -52,7 +52,7 @@ module ActiveRecord NORMAL_VALUES = Relation::VALUE_METHODS - Relation::CLAUSE_METHODS - - [:includes, :preload, :joins, :order, :reverse_order, :lock, :create_with, :reordering] # :nodoc: + [:includes, :preload, :joins, :left_outer_joins, :order, :reverse_order, :lock, :create_with, :reordering] # :nodoc: def normal_values NORMAL_VALUES @@ -79,6 +79,7 @@ module ActiveRecord merge_clauses merge_preloads merge_joins + merge_outer_joins relation end @@ -112,22 +113,43 @@ module ActiveRecord if other.klass == relation.klass relation.joins!(*other.joins_values) else - joins_dependency, rest = other.joins_values.partition do |join| + alias_tracker = nil + joins_dependency = other.joins_values.map do |join| case join when Hash, Symbol, Array - true + alias_tracker ||= other.alias_tracker + ActiveRecord::Associations::JoinDependency.new( + other.klass, other.table, join, alias_tracker + ) else - false + join end end - join_dependency = ActiveRecord::Associations::JoinDependency.new( - other.klass, other.table, joins_dependency, [] - ) + relation.joins!(*joins_dependency) + end + end + + def merge_outer_joins + return if other.left_outer_joins_values.blank? - relation.joins! rest + if other.klass == relation.klass + relation.left_outer_joins!(*other.left_outer_joins_values) + else + alias_tracker = nil + joins_dependency = other.left_outer_joins_values.map do |join| + case join + when Hash, Symbol, Array + alias_tracker ||= other.alias_tracker + ActiveRecord::Associations::JoinDependency.new( + other.klass, other.table, join, alias_tracker + ) + else + join + end + end - @relation = relation.joins join_dependency + relation.left_outer_joins!(*joins_dependency) end end diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 5c42414072..885c26d7aa 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -13,6 +13,7 @@ module ActiveRecord register_handler(Range, RangeHandler.new(self)) register_handler(Relation, RelationHandler.new) register_handler(Array, ArrayHandler.new(self)) + register_handler(Set, ArrayHandler.new(self)) end def build_from_hash(attributes) @@ -123,11 +124,11 @@ module ActiveRecord end end -require_relative "predicate_builder/array_handler" -require_relative "predicate_builder/base_handler" -require_relative "predicate_builder/basic_object_handler" -require_relative "predicate_builder/range_handler" -require_relative "predicate_builder/relation_handler" +require "active_record/relation/predicate_builder/array_handler" +require "active_record/relation/predicate_builder/base_handler" +require "active_record/relation/predicate_builder/basic_object_handler" +require "active_record/relation/predicate_builder/range_handler" +require "active_record/relation/predicate_builder/relation_handler" -require_relative "predicate_builder/association_query_value" -require_relative "predicate_builder/polymorphic_array_value" +require "active_record/relation/predicate_builder/association_query_value" +require "active_record/relation/predicate_builder/polymorphic_array_value" diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb index 0255a65bfe..28c7483c95 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb @@ -30,7 +30,7 @@ module ActiveRecord end def primary_key - associated_table.association_join_keys.key + associated_table.association_join_primary_key end def convert_to_id(value) diff --git a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_value.rb b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_value.rb index b87b5c36dd..e8e2f2c626 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_value.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_value.rb @@ -29,7 +29,7 @@ module ActiveRecord end def primary_key(value) - associated_table.association_primary_key(base_class(value)) + associated_table.association_join_primary_key(base_class(value)) end def base_class(value) 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 f51ea4fde0..c8bbfa5051 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb @@ -4,6 +4,10 @@ module ActiveRecord class PredicateBuilder class RelationHandler # :nodoc: def call(attribute, value) + if value.eager_loading? + value = value.send(:apply_join_dependency) + end + if value.select_values.empty? value = value.select(value.arel_attribute(value.klass.primary_key)) end diff --git a/activerecord/lib/active_record/relation/query_attribute.rb b/activerecord/lib/active_record/relation/query_attribute.rb index 5a9a7fd432..3532f28858 100644 --- a/activerecord/lib/active_record/relation/query_attribute.rb +++ b/activerecord/lib/active_record/relation/query_attribute.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true -require_relative "../attribute" +require "active_model/attribute" module ActiveRecord class Relation - class QueryAttribute < Attribute # :nodoc: + class QueryAttribute < ActiveModel::Attribute # :nodoc: def type_cast(value) value end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index bdc5c27328..86882c7ce7 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true -require_relative "from_clause" -require_relative "query_attribute" -require_relative "where_clause" -require_relative "where_clause_factory" +require "active_record/relation/from_clause" +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" module ActiveRecord @@ -295,6 +295,7 @@ module ActiveRecord spawn.order!(*args) end + # Same as #order but operates on relation in-place instead of copying. def order!(*args) # :nodoc: preprocess_order_args(args) @@ -316,6 +317,7 @@ module ActiveRecord spawn.reorder!(*args) end + # Same as #reorder but operates on relation in-place instead of copying. def reorder!(*args) # :nodoc: preprocess_order_args(args) @@ -325,8 +327,8 @@ module ActiveRecord end VALID_UNSCOPING_VALUES = Set.new([:where, :select, :group, :order, :lock, - :limit, :offset, :joins, :includes, :from, - :readonly, :having]) + :limit, :offset, :joins, :left_outer_joins, + :includes, :from, :readonly, :having]) # Removes an unwanted relation that is already defined on a chain of relations. # This is useful when passing around chains of relations and would like to @@ -373,10 +375,11 @@ module ActiveRecord args.each do |scope| case scope when Symbol + scope = :left_outer_joins if scope == :left_joins if !VALID_UNSCOPING_VALUES.include?(scope) raise ArgumentError, "Called unscope() with invalid unscoping argument ':#{scope}'. Valid arguments are :#{VALID_UNSCOPING_VALUES.to_a.join(", :")}." end - set_value(scope, nil) + set_value(scope, DEFAULT_VALUES[scope]) when Hash scope.each do |key, target_value| if key != :where @@ -441,16 +444,14 @@ module ActiveRecord # => SELECT "users".* FROM "users" LEFT OUTER JOIN "posts" ON "posts"."user_id" = "users"."id" # def left_outer_joins(*args) - check_if_method_has_arguments!(:left_outer_joins, args) - - args.compact! - args.flatten! - + check_if_method_has_arguments!(__callee__, args) spawn.left_outer_joins!(*args) end alias :left_joins :left_outer_joins def left_outer_joins!(*args) # :nodoc: + args.compact! + args.flatten! self.left_outer_joins_values += args self end @@ -898,14 +899,14 @@ module ActiveRecord end # Returns the Arel object associated with the relation. - def arel # :nodoc: - @arel ||= build_arel + def arel(aliases = nil) # :nodoc: + @arel ||= build_arel(aliases) end protected # Returns a relation value with a given name def get_value(name) # :nodoc: - @values[name] || default_value_for(name) + @values.fetch(name, DEFAULT_VALUES[name]) end # Sets the relation value with the given name @@ -921,16 +922,16 @@ module ActiveRecord raise ImmutableRelation if defined?(@arel) && @arel end - def build_arel + def build_arel(aliases) arel = Arel::SelectManager.new(table) - build_joins(arel, joins_values.flatten) unless joins_values.empty? - build_left_outer_joins(arel, left_outer_joins_values.flatten) unless left_outer_joins_values.empty? + aliases = build_joins(arel, joins_values.flatten, aliases) unless joins_values.empty? + build_left_outer_joins(arel, left_outer_joins_values.flatten, aliases) unless left_outer_joins_values.empty? arel.where(where_clause.ast) unless where_clause.empty? arel.having(having_clause.ast) unless having_clause.empty? if limit_value - limit_attribute = Attribute.with_cast_value( + limit_attribute = ActiveModel::Attribute.with_cast_value( "LIMIT".freeze, connection.sanitize_limit(limit_value), Type.default_value, @@ -938,7 +939,7 @@ module ActiveRecord arel.take(Arel::Nodes::BindParam.new(limit_attribute)) end if offset_value - offset_attribute = Attribute.with_cast_value( + offset_attribute = ActiveModel::Attribute.with_cast_value( "OFFSET".freeze, offset_value.to_i, Type.default_value, @@ -963,6 +964,9 @@ module ActiveRecord name = from_clause.name case opts when Relation + if opts.eager_loading? + opts = opts.send(:apply_join_dependency) + end name ||= "subquery" opts.arel.as(name.to_s) else @@ -970,20 +974,22 @@ module ActiveRecord end end - def build_left_outer_joins(manager, outer_joins) + def build_left_outer_joins(manager, outer_joins, aliases) buckets = outer_joins.group_by do |join| case join when Hash, Symbol, Array :association_join + when ActiveRecord::Associations::JoinDependency + :stashed_join else raise ArgumentError, "only Hash, Symbol and Array are allowed" end end - build_join_query(manager, buckets, Arel::Nodes::OuterJoin) + build_join_query(manager, buckets, Arel::Nodes::OuterJoin, aliases) end - def build_joins(manager, joins) + def build_joins(manager, joins, aliases) buckets = joins.group_by do |join| case join when String @@ -999,10 +1005,10 @@ module ActiveRecord end end - build_join_query(manager, buckets, Arel::Nodes::InnerJoin) + build_join_query(manager, buckets, Arel::Nodes::InnerJoin, aliases) end - def build_join_query(manager, buckets, join_type) + def build_join_query(manager, buckets, join_type, aliases) buckets.default = [] association_joins = buckets[:association_join] @@ -1010,10 +1016,11 @@ module ActiveRecord join_nodes = buckets[:join_node].uniq string_joins = buckets[:string_join].map(&:strip).uniq - join_list = join_nodes + convert_join_strings_to_ast(manager, string_joins) + join_list = join_nodes + convert_join_strings_to_ast(string_joins) + alias_tracker = alias_tracker(join_list, aliases) join_dependency = ActiveRecord::Associations::JoinDependency.new( - klass, table, association_joins, join_list + klass, table, association_joins, alias_tracker ) joins = join_dependency.join_constraints(stashed_association_joins, join_type) @@ -1021,10 +1028,10 @@ module ActiveRecord manager.join_sources.concat(join_list) - manager + alias_tracker.aliases end - def convert_join_strings_to_ast(table, joins) + def convert_join_strings_to_ast(joins) joins .flatten .reject(&:blank?) @@ -1034,6 +1041,8 @@ module ActiveRecord def build_select(arel) if select_values.any? arel.project(*arel_columns(select_values.uniq)) + elsif klass.ignored_columns.any? + arel.project(*klass.column_names.map { |field| arel_attribute(field) }) else arel.project(table[Arel.star]) end @@ -1070,7 +1079,7 @@ module ActiveRecord end o.split(",").map! do |s| s.strip! - s.gsub!(/\sasc\Z/i, " DESC") || s.gsub!(/\sdesc\Z/i, " ASC") || s.concat(" DESC") + s.gsub!(/\sasc\Z/i, " DESC") || s.gsub!(/\sdesc\Z/i, " ASC") || (s << " DESC") end else o @@ -1079,6 +1088,10 @@ module ActiveRecord end def does_not_support_reverse?(order) + # Account for String subclasses like Arel::Nodes::SqlLiteral that + # override methods like #count. + order = String.new(order) unless order.instance_of?(String) + # Uses SQL function with multiple arguments. (order.include?(",") && order.split(",").find { |section| section.count("(") != section.count(")") }) || # Uses "nulls first" like construction. @@ -1109,9 +1122,15 @@ module ActiveRecord def preprocess_order_args(order_args) order_args.map! do |arg| - klass.send(:sanitize_sql_for_order, arg) + klass.sanitize_sql_for_order(arg) end order_args.flatten! + + @klass.enforce_raw_sql_whitelist( + order_args.flat_map { |a| a.is_a?(Hash) ? a.keys : a }, + whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST + ) + validate_order_args(order_args) references = order_args.grep(String) @@ -1172,23 +1191,15 @@ module ActiveRecord end alias having_clause_factory where_clause_factory - def default_value_for(name) - case name - when :create_with - FROZEN_EMPTY_HASH - when :readonly - false - when :where, :having - Relation::WhereClause.empty - when :from - Relation::FromClause.empty - when *Relation::MULTI_VALUE_METHODS - FROZEN_EMPTY_ARRAY - when *Relation::SINGLE_VALUE_METHODS - nil - else - raise ArgumentError, "unknown relation value #{name.inspect}" - end + DEFAULT_VALUES = { + create_with: FROZEN_EMPTY_HASH, + where: Relation::WhereClause.empty, + having: Relation::WhereClause.empty, + from: Relation::FromClause.empty + } + + Relation::MULTI_VALUE_METHODS.each do |value| + DEFAULT_VALUES[value] ||= FROZEN_EMPTY_ARRAY end end end diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 424894f835..617d8de8b2 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -2,7 +2,7 @@ require "active_support/core_ext/hash/except" require "active_support/core_ext/hash/slice" -require_relative "merger" +require "active_record/relation/merger" module ActiveRecord module SpawnMethods diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index 752bb38481..a502713e56 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -47,7 +47,7 @@ module ActiveRecord end def to_h(table_name = nil) - equalities = predicates.grep(Arel::Nodes::Equality) + equalities = equalities(predicates) if table_name equalities = equalities.select do |node| node.left.relation.name == table_name @@ -90,6 +90,20 @@ module ActiveRecord end private + def equalities(predicates) + equalities = [] + + predicates.each do |node| + case node + when Arel::Nodes::Equality + equalities << node + when Arel::Nodes::And + equalities.concat equalities(node.children) + end + end + + equalities + end def predicates_unreferenced_by(other) predicates.reject do |n| @@ -121,7 +135,7 @@ module ActiveRecord end def except_predicates(columns) - self.predicates.reject do |node| + predicates.reject do |node| case node when Arel::Nodes::Between, Arel::Nodes::In, Arel::Nodes::NotIn, Arel::Nodes::Equality, Arel::Nodes::NotEqual, Arel::Nodes::LessThan, Arel::Nodes::LessThanOrEqual, Arel::Nodes::GreaterThan, Arel::Nodes::GreaterThanOrEqual subrelation = (node.left.kind_of?(Arel::Attributes::Attribute) ? node.left : node.right) diff --git a/activerecord/lib/active_record/relation/where_clause_factory.rb b/activerecord/lib/active_record/relation/where_clause_factory.rb index 1374785354..4ae94f4bfe 100644 --- a/activerecord/lib/active_record/relation/where_clause_factory.rb +++ b/activerecord/lib/active_record/relation/where_clause_factory.rb @@ -11,7 +11,7 @@ module ActiveRecord def build(opts, other) case opts when String, Array - parts = [klass.send(:sanitize_sql, other.empty? ? opts : ([opts] + other))] + parts = [klass.sanitize_sql(other.empty? ? opts : ([opts] + other))] when Hash attributes = predicate_builder.resolve_column_aliases(opts) attributes = klass.send(:expand_hash_conditions_for_aggregates, attributes) |