Permalink
Browse files

Deprecate the `:distinct` option for `Relation#count`.

We moved more and more away from passing options to finder / calculation
methods. The `:distinct` option in `#count` was one of the remaining places.
Since we can now combine `Relation#distinct` with `Relation#count` the option
is no longer necessary and can be deprecated.
  • Loading branch information...
senny committed Mar 12, 2013
1 parent a1bb6c8 commit cd87c85ef05e47f6ea1ce7422ae033fe6d82b030
View
@@ -1,5 +1,18 @@
## Rails 4.0.0 (unreleased) ##
+* The `:distinct` option for `Relation#count` is deprecated. You
+ should use `Relation#distinct` instead.
+
+ Example:
+
+ # Before
+ Post.select(:author_name).count(distinct: true)
+
+ # After
+ Post.select(:author_name).distinct.count
+
+ *Yves Senn*
+
* Rename `Relation#uniq` to `Relation#distinct`. `#uniq` is still
available as an alias but we encourage to use `#distinct` instead.
Also `Relation#uniq_value` is aliased to `Relation#distinct_value`,
@@ -174,13 +174,14 @@ def count(column_name = nil, count_options = {})
reflection.klass.count_by_sql(custom_counter_sql)
else
+ relation = scope
if association_scope.distinct_value
# This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL.
column_name ||= reflection.klass.primary_key
- count_options[:distinct] = true
+ relation = relation.distinct
end
- value = scope.count(column_name, count_options)
+ value = relation.count(column_name)
limit = options[:limit]
offset = options[:offset]
@@ -11,7 +11,7 @@ module Calculations
# Person.count(:all)
# # => performs a COUNT(*) (:all is an alias for '*')
#
- # Person.count(:age, distinct: true)
+ # Person.distinct.count(:age)
# # => counts the number of different age values
#
# If +count+ is used with +group+, it returns a Hash whose keys represent the aggregated column,
@@ -199,7 +199,12 @@ def perform_calculation(operation, column_name, options = {})
operation = operation.to_s.downcase
# If #count is used with #distinct / #uniq it is considered distinct. (eg. relation.distinct.count)
- distinct = options[:distinct] || self.distinct_value
+ distinct = self.distinct_value
+ if options.has_key?(:distinct)
+ ActiveSupport::Deprecation.warn "The :distinct option for `Relation#count` is deprecated. " \
+ "Please use `Relation#distinct` instead. (eg. `relation.distinct.count`)"
+ distinct = options[:distinct]
+ end
if operation == "count"
column_name ||= (select_for_count || :all)
@@ -55,7 +55,7 @@ def test_cascaded_eager_association_loading_with_join_for_count
assert_nothing_raised do
assert_equal 4, categories.count
assert_equal 4, categories.to_a.count
- assert_equal 3, categories.count(:distinct => true)
+ assert_equal 3, categories.distinct.count
assert_equal 3, categories.to_a.uniq.size # Must uniq since instantiating with inner joins will get dupes
end
end
@@ -82,7 +82,7 @@ def test_calculate_honors_implicit_inner_joins
def test_calculate_honors_implicit_inner_joins_and_distinct_and_conditions
real_count = Author.all.to_a.select {|a| a.posts.any? {|p| p.title =~ /^Welcome/} }.length
- authors_with_welcoming_post_titles = Author.all.merge!(:joins => :posts, :where => "posts.title like 'Welcome%'").calculate(:count, 'authors.id', :distinct => true)
+ authors_with_welcoming_post_titles = Author.all.merge!(joins: :posts, where: "posts.title like 'Welcome%'").distinct.calculate(:count, 'authors.id')
assert_equal real_count, authors_with_welcoming_post_titles, "inner join and conditions should have only returned authors posting titles starting with 'Welcome'"
end
@@ -1105,7 +1105,7 @@ def test_count_with_join
res6 = Post.count_by_sql "SELECT COUNT(DISTINCT p.id) FROM posts p, comments co WHERE p.#{QUOTED_TYPE} = 'Post' AND p.id=co.post_id"
res7 = nil
assert_nothing_raised do
- res7 = Post.where("p.#{QUOTED_TYPE} = 'Post' AND p.id=co.post_id").joins("p, comments co").select("p.id").count(distinct: true)
+ res7 = Post.where("p.#{QUOTED_TYPE} = 'Post' AND p.id=co.post_id").joins("p, comments co").select("p.id").distinct.count
end
assert_equal res6, res7
end
@@ -305,8 +305,8 @@ def test_should_group_by_summed_field_through_association_and_having
end
def test_should_count_selected_field_with_include
- assert_equal 6, Account.includes(:firm).count(:distinct => true)
- assert_equal 4, Account.includes(:firm).select(:credit_limit).count(:distinct => true)
+ assert_equal 6, Account.includes(:firm).distinct.count
+ assert_equal 4, Account.includes(:firm).distinct.select(:credit_limit).count
end
def test_should_not_perform_joined_include_by_default
@@ -341,6 +341,16 @@ def test_count_with_column_parameter
assert_equal 5, Account.count(:firm_id)
end
+ def test_count_distinct_option_is_deprecated
+ assert_deprecated do
+ assert_equal 4, Account.select(:credit_limit).count(distinct: true)
+ end
+
+ assert_deprecated do
+ assert_equal 6, Account.select(:credit_limit).count(distinct: false)
+ end
+ end
+
def test_count_with_distinct
assert_equal 4, Account.select(:credit_limit).distinct.count
assert_equal 4, Account.select(:credit_limit).uniq.count
@@ -352,7 +362,7 @@ def test_count_with_column_and_options_parameter
def test_should_count_field_in_joined_table
assert_equal 5, Account.joins(:firm).count('companies.id')
- assert_equal 4, Account.joins(:firm).count('companies.id', :distinct => true)
+ assert_equal 4, Account.joins(:firm).distinct.count('companies.id')
end
def test_should_count_field_in_joined_table_with_group_by
@@ -456,7 +466,7 @@ def test_distinct_is_honored_when_used_with_count_operation_after_group
approved_topics_count = Topic.group(:approved).count(:author_name)[true]
assert_equal approved_topics_count, 3
# Count the number of distinct authors for approved Topics
- distinct_authors_for_approved_count = Topic.group(:approved).count(:author_name, :distinct => true)[true]
+ distinct_authors_for_approved_count = Topic.group(:approved).distinct.count(:author_name)[true]
assert_equal distinct_authors_for_approved_count, 2
end
@@ -790,11 +790,11 @@ def test_count
def test_count_with_distinct
posts = Post.all
- assert_equal 3, posts.count(:comments_count, :distinct => true)
- assert_equal 11, posts.count(:comments_count, :distinct => false)
+ assert_equal 3, posts.distinct(true).count(:comments_count)
+ assert_equal 11, posts.distinct(false).count(:comments_count)
- assert_equal 3, posts.select(:comments_count).count(:distinct => true)
- assert_equal 11, posts.select(:comments_count).count(:distinct => false)
+ assert_equal 3, posts.distinct(true).select(:comments_count).count
+ assert_equal 11, posts.distinct(false).select(:comments_count).count
end
def test_count_explicit_columns

0 comments on commit cd87c85

Please sign in to comment.