diff options
5 files changed, 31 insertions, 10 deletions
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 5aee74ae61..8789bdc9ca 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -5,12 +5,12 @@ module ActiveRecord # = Active Record Relation class Relation MULTI_VALUE_METHODS = [:includes, :eager_load, :preload, :select, :group, - :order, :joins, :having, :references, + :order, :joins, :references, :extending, :unscope] SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :from, :reordering, :reverse_order, :distinct, :create_with, :uniq] - CLAUSE_METHODS = [:where] + CLAUSE_METHODS = [:where, :having] INVALID_METHODS_FOR_DELETE_ALL = [:limit, :distinct, :offset, :group, :having] VALUE_METHODS = MULTI_VALUE_METHODS + SINGLE_VALUE_METHODS + CLAUSE_METHODS @@ -467,8 +467,10 @@ module ActiveRecord invalid_methods = INVALID_METHODS_FOR_DELETE_ALL.select { |method| if MULTI_VALUE_METHODS.include?(method) send("#{method}_values").any? - else + elsif SINGLE_VALUE_METHODS.include?(method) send("#{method}_value") + elsif CLAUSE_METHODS.include?(method) + send("#{method}_clause").any? end } if invalid_methods.any? diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 1d4cb1a83b..67ac350b76 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -290,7 +290,7 @@ module ActiveRecord operation, distinct).as(aggregate_alias) ] - select_values += select_values unless having_values.empty? + select_values += select_values unless having_clause.empty? select_values.concat group_fields.zip(group_aliases).map { |field,aliaz| if field.respond_to?(:as) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 31533b0939..9e72dd319e 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -93,10 +93,11 @@ module ActiveRecord end def bind_values - where_clause.binds + where_clause.binds + having_clause.binds end def bind_values=(values) + self.having_clause = Relation::WhereClause.new(having_clause.predicates, []) self.where_clause = Relation::WhereClause.new(where_clause.predicates, values || []) end @@ -605,7 +606,7 @@ module ActiveRecord def having!(opts, *rest) # :nodoc: references!(PredicateBuilder.references(opts)) if Hash === opts - self.having_values += build_where(opts, rest) + self.having_clause += having_clause_factory.build(opts, rest) self end @@ -869,7 +870,7 @@ module ActiveRecord collapse_wheres(arel, (where_clause.predicates - [''])) #TODO: Add uniq with real value comparison / ignore uniqs that have binds - arel.having(*having_values.uniq.reject(&:blank?)) unless having_values.empty? + arel.having(*having_clause.predicates.uniq.reject(&:blank?)) if having_clause.any? arel.take(connection.sanitize_limit(limit_value)) if limit_value arel.skip(offset_value.to_i) if offset_value @@ -1108,9 +1109,11 @@ module ActiveRecord def new_where_clause Relation::WhereClause.empty end + alias new_having_clause new_where_clause def where_clause_factory @where_clause_factory ||= Relation::WhereClauseFactory.new(klass, predicate_builder) end + alias having_clause_factory where_clause_factory end end diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index 90fb85cbf1..8b9ba3e633 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -3,7 +3,7 @@ module ActiveRecord class WhereClause # :nodoc: attr_reader :predicates, :binds - delegate :empty?, to: :predicates + delegate :any?, :empty?, to: :predicates def initialize(predicates, binds) @predicates = predicates diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index d272b9b929..e0e04a0891 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1468,10 +1468,26 @@ class RelationTest < ActiveRecord::TestCase def test_doesnt_add_having_values_if_options_are_blank scope = Post.having('') - assert_equal [], scope.having_values + assert scope.having_clause.empty? scope = Post.having([]) - assert_equal [], scope.having_values + assert scope.having_clause.empty? + end + + def test_having_with_binds_for_both_where_and_having + post = Post.first + having_then_where = Post.having(id: post.id).where(title: post.title).group(:title) + where_then_having = Post.where(title: post.title).having(id: post.id).group(:title) + + assert_equal [post], having_then_where + assert_equal [post], where_then_having + end + + def test_multiple_where_and_having_clauses + post = Post.first + having_then_where = Post.having(id: post.id).where(title: post.title).having(id: post.id).where(title: post.title).group(:title) + + assert_equal [post], having_then_where end def test_references_triggers_eager_loading |