From 79e01be44666a1292ab3df0d59df98f69bf9b39b Mon Sep 17 00:00:00 2001 From: miloops Date: Fri, 21 Nov 2008 19:20:33 -0300 Subject: [PATCH] Add :having option to find, to use in combination with grouped finds. Also added to has_many and has_and_belongs_to_many associations. --- activerecord/lib/active_record/associations.rb | 12 ++++++++---- .../associations/association_proxy.rb | 1 + activerecord/lib/active_record/base.rb | 9 ++++++--- .../has_and_belongs_to_many_associations_test.rb | 5 +++++ .../associations/has_many_associations_test.rb | 5 +++++ activerecord/test/cases/finder_test.rb | 7 +++++++ activerecord/test/models/author.rb | 1 + activerecord/test/models/category.rb | 1 + activerecord/test/models/project.rb | 1 + 9 files changed, 35 insertions(+), 7 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 7f78191..ca24842 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -722,6 +722,8 @@ module ActiveRecord # Specify second-order associations that should be eager loaded when the collection is loaded. # [:group] # An attribute name by which the result should be grouped. Uses the GROUP BY SQL-clause. + # [:having] + # Combined with +:group+ this can be used to filter the records that a GROUP BY returns. Uses the HAVING SQL-clause. # [:limit] # An integer determining the limit on the number of rows that should be returned. # [:offset] @@ -1179,6 +1181,8 @@ module ActiveRecord # Specify second-order associations that should be eager loaded when the collection is loaded. # [:group] # An attribute name by which the result should be grouped. Uses the GROUP BY SQL-clause. + # [:having] + # Combined with +:group+ this can be used to filter the records that a GROUP BY returns. Uses the HAVING SQL-clause. # [:limit] # An integer determining the limit on the number of rows that should be returned. # [:offset] @@ -1551,7 +1555,7 @@ module ActiveRecord @@valid_keys_for_has_many_association = [ :class_name, :table_name, :foreign_key, :primary_key, :dependent, - :select, :conditions, :include, :order, :group, :limit, :offset, + :select, :conditions, :include, :order, :group, :having, :limit, :offset, :as, :through, :source, :source_type, :uniq, :finder_sql, :counter_sql, @@ -1607,7 +1611,7 @@ module ActiveRecord mattr_accessor :valid_keys_for_has_and_belongs_to_many_association @@valid_keys_for_has_and_belongs_to_many_association = [ :class_name, :table_name, :join_table, :foreign_key, :association_foreign_key, - :select, :conditions, :include, :order, :group, :limit, :offset, + :select, :conditions, :include, :order, :group, :having, :limit, :offset, :uniq, :finder_sql, :counter_sql, :delete_sql, :insert_sql, :before_add, :after_add, :before_remove, :after_remove, @@ -1656,7 +1660,7 @@ module ActiveRecord add_conditions!(sql, options[:conditions], scope) add_limited_ids_condition!(sql, options, join_dependency) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) - add_group!(sql, options[:group], scope) + add_group!(sql, options[:group], options[:having], scope) add_order!(sql, options[:order], scope) add_limit!(sql, options, scope) if using_limitable_reflections?(join_dependency.reflections) add_lock!(sql, options, scope) @@ -1712,7 +1716,7 @@ module ActiveRecord end add_conditions!(sql, options[:conditions], scope) - add_group!(sql, options[:group], scope) + add_group!(sql, options[:group], options[:having], scope) if order && is_distinct connection.add_order_by_for_association_limiting!(sql, :order => order) diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index d1a79df..75ec4fb 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -188,6 +188,7 @@ module ActiveRecord def merge_options_from_reflection!(options) options.reverse_merge!( :group => @reflection.options[:group], + :having => @reflection.options[:having], :limit => @reflection.options[:limit], :offset => @reflection.options[:offset], :joins => @reflection.options[:joins], diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index cff5fd7..3360851 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -521,6 +521,7 @@ module ActiveRecord #:nodoc: # * :conditions - An SQL fragment like "administrator = 1", [ "user_name = ?", username ], or ["user_name = :user_name", { :user_name => user_name }]. See conditions in the intro. # * :order - An SQL fragment like "created_at DESC, name". # * :group - An attribute name by which the result should be grouped. Uses the GROUP BY SQL-clause. + # * :having - Combined with +:group+ this can be used to filter the records that a GROUP BY returns. Uses the HAVING SQL-clause. # * :limit - An integer determining the limit on the number of rows that should be returned. # * :offset - An integer determining the offset from where the rows should be fetched. So at 5, it would skip rows 0 through 4. # * :joins - Either an SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id" (rarely needed) @@ -1632,7 +1633,7 @@ module ActiveRecord #:nodoc: add_joins!(sql, options[:joins], scope) add_conditions!(sql, options[:conditions], scope) - add_group!(sql, options[:group], scope) + add_group!(sql, options[:group], options[:having], scope) add_order!(sql, options[:order], scope) add_limit!(sql, options, scope) add_lock!(sql, options, scope) @@ -1688,13 +1689,15 @@ module ActiveRecord #:nodoc: end end - def add_group!(sql, group, scope = :auto) + def add_group!(sql, group, having, scope = :auto) if group sql << " GROUP BY #{group}" + sql << " HAVING #{having}" if having else scope = scope(:find) if :auto == scope if scope && (scoped_group = scope[:group]) sql << " GROUP BY #{scoped_group}" + sql << " HAVING #{scoped_having}" if (scoped_having = scope[:having]) end end end @@ -2259,7 +2262,7 @@ module ActiveRecord #:nodoc: end VALID_FIND_OPTIONS = [ :conditions, :include, :joins, :limit, :offset, - :order, :select, :readonly, :group, :from, :lock ] + :order, :select, :readonly, :group, :having, :from, :lock ] def validate_find_options(options) #:nodoc: options.assert_valid_keys(VALID_FIND_OPTIONS) diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index b5bedf3..2f08e09 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -658,6 +658,11 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal 1, categories(:technology).posts_gruoped_by_title.size end + def test_find_scoped_grouped_having + assert_equal 2, projects(:active_record).well_payed_salary_groups.size + assert projects(:active_record).well_payed_salary_groups.all? { |g| g.salary > 10000 } + end + def test_get_ids assert_equal projects(:active_record, :action_controller).map(&:id).sort, developers(:david).project_ids.sort assert_equal [projects(:active_record).id], developers(:jamis).project_ids diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 59784e1..816ceb6 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -255,6 +255,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 2, companies(:first_firm).clients_grouped_by_name.length end + def test_find_scoped_grouped_having + assert_equal 1, authors(:david).popular_grouped_posts.length + assert_equal 0, authors(:mary).popular_grouped_posts.length + end + def test_adding force_signal37_to_load_all_clients_of_firm natural = Client.new("name" => "Natural Company") diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 153880a..d4d770b 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -175,6 +175,13 @@ class FinderTest < ActiveRecord::TestCase assert_equal 4, developers.map(&:salary).uniq.size end + def test_find_with_group_and_having + developers = Developer.find(:all, :group => "salary", :having => "sum(salary) > 10000", :select => "salary") + assert_equal 3, developers.size + assert_equal 3, developers.map(&:salary).uniq.size + assert developers.all? { |developer| developer.salary > 10000 } + end + def test_find_with_entire_select_statement topics = Topic.find_by_sql "SELECT * FROM topics WHERE author_name = 'Mary'" diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index e5b19ff..4ffac4f 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -1,6 +1,7 @@ class Author < ActiveRecord::Base has_many :posts has_many :posts_with_comments, :include => :comments, :class_name => "Post" + has_many :popular_grouped_posts, :include => :comments, :class_name => "Post", :group => "type", :having => "SUM(comments_count) > 1", :select => "type" has_many :posts_with_comments_sorted_by_comment_id, :include => :comments, :class_name => "Post", :order => 'comments.id' has_many :posts_sorted_by_id_limited, :class_name => "Post", :order => 'posts.id', :limit => 1 has_many :posts_with_categories, :include => :categories, :class_name => "Post" diff --git a/activerecord/test/models/category.rb b/activerecord/test/models/category.rb index 4e9d247..5efce6a 100644 --- a/activerecord/test/models/category.rb +++ b/activerecord/test/models/category.rb @@ -14,6 +14,7 @@ class Category < ActiveRecord::Base :class_name => 'Post', :conditions => { :title => 'Yet Another Testing Title' } + has_and_belongs_to_many :popular_grouped_posts, :class_name => "Post", :group => "posts.type", :having => "sum(comments.post_id) > 2", :include => :comments has_and_belongs_to_many :posts_gruoped_by_title, :class_name => "Post", :group => "title", :select => "title" def self.what_are_you diff --git a/activerecord/test/models/project.rb b/activerecord/test/models/project.rb index 44c692b..550d4ae 100644 --- a/activerecord/test/models/project.rb +++ b/activerecord/test/models/project.rb @@ -13,6 +13,7 @@ class Project < ActiveRecord::Base :after_add => Proc.new {|o, r| o.developers_log << "after_adding#{r.id || ''}"}, :before_remove => Proc.new {|o, r| o.developers_log << "before_removing#{r.id}"}, :after_remove => Proc.new {|o, r| o.developers_log << "after_removing#{r.id}"} + has_and_belongs_to_many :well_payed_salary_groups, :class_name => "Developer", :group => "salary", :having => "SUM(salary) > 10000", :select => "SUM(salary) as salary" attr_accessor :developers_log -- 1.5.5.1