Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Change behavior of count(:limit => x, :offset => y) to limit/offset b…

…efore counting.
  • Loading branch information...
commit d5994ee48af14d67f0eec7d23863d4b19211b078 1 parent 1db4969
@jmileham jmileham authored
View
40 activerecord/lib/active_record/relation/calculations.rb
@@ -183,10 +183,13 @@ def perform_calculation(operation, column_name, options = {})
end
end
- def aggregate_column(column_name)
+ def aggregate_column(column_name, subquery_alias = nil)
if @klass.column_names.include?(column_name.to_s)
- Arel::Attribute.new(@klass.unscoped.table, column_name)
+ Arel::Attribute.new(subquery_alias || @klass.unscoped.table, column_name)
else
+ if subquery_alias && (split_name = column_name.to_s.split(".")).length > 1
+ column_name = split_name.last
+ end
Arel.sql(column_name == :all ? "*" : column_name.to_s)
end
end
@@ -196,24 +199,22 @@ def operation_over_aggregate_column(column, operation, distinct)
end
def execute_simple_calculation(operation, column_name, distinct) #:nodoc:
- column = aggregate_column(column_name)
-
# Postgresql doesn't like ORDER BY when there are no GROUP BY
relation = except(:order)
- select_value = operation_over_aggregate_column(column, operation, distinct)
- relation.select_values = [select_value]
+ if operation == "count" && (relation.limit_value || relation.offset_value)
+ # Shortcut when limit is zero.
+ return 0 if relation.limit_value == 0
- query_builder = relation.arel
+ query_builder = build_count_subquery(relation, column_name, distinct)
+ else
+ column = aggregate_column(column_name)
- if operation == "count"
- limit = relation.limit_value
- offset = relation.offset_value
+ select_value = operation_over_aggregate_column(column, operation, distinct)
- unless limit && offset
- query_builder.limit = nil
- query_builder.offset = nil
- end
+ relation.select_values = [select_value]
+
+ query_builder = relation.arel
end
type_cast_calculated_value(@klass.connection.select_value(query_builder.to_sql), column_for(column_name), operation)
@@ -312,5 +313,16 @@ def select_for_count
select if select !~ /(,|\*)/
end
end
+
+ def build_count_subquery(relation, column_name, distinct)
+ # Arel doesn't do subqueries
+ subquery_alias = arel_table.alias("subquery_for_count")
+ aliased_column = aggregate_column(column_name, subquery_alias)
+ select_value = operation_over_aggregate_column(aliased_column, 'count', distinct)
+
+ relation.select_values = [(column_name == :all ? 1 : aggregate_column(column_name))]
+ subquery_sql = "(#{relation.arel.to_sql}) #{subquery_alias.name}"
+ subquery_alias.relation.select_manager.project(select_value).from(subquery_sql)
+ end
end
end
View
37 activerecord/test/cases/calculations_test.rb
@@ -65,7 +65,7 @@ def test_should_group_by_field
c = Account.sum(:credit_limit, :group => :firm_id)
[1,6,2].each { |firm_id| assert c.keys.include?(firm_id) }
end
-
+
def test_should_group_by_multiple_fields
c = Account.count(:all, :group => ['firm_id', :credit_limit])
[ [nil, 50], [1, 50], [6, 50], [6, 55], [9, 53], [2, 60] ].each { |firm_and_limit| assert c.keys.include?(firm_and_limit) }
@@ -109,27 +109,42 @@ def test_should_limit_calculation_with_offset
assert_equal [2, 6], c.keys.compact
end
- def test_limit_with_offset_is_kept
+ def test_limit_should_apply_before_count
+ accounts = Account.limit(3).where('firm_id IS NOT NULL')
+
+ assert_equal 3, accounts.count(:firm_id)
+ assert_equal 3, accounts.select(:firm_id).count
+ end
+
+ def test_count_should_shortcut_with_limit_zero
+ accounts = Account.limit(0)
+
+ assert_no_queries { assert_equal 0, accounts.count }
+ end
+
+ def test_limit_is_kept
return if current_adapter?(:OracleAdapter)
- queries = assert_sql { Account.limit(1).offset(1).count }
+ queries = assert_sql { Account.limit(1).count }
assert_equal 1, queries.length
assert_match(/LIMIT/, queries.first)
- assert_match(/OFFSET/, queries.first)
end
- def test_offset_without_limit_removes_offset
+ def test_offset_is_kept
+ return if current_adapter?(:OracleAdapter)
+
queries = assert_sql { Account.offset(1).count }
assert_equal 1, queries.length
- assert_no_match(/LIMIT/, queries.first)
- assert_no_match(/OFFSET/, queries.first)
+ assert_match(/OFFSET/, queries.first)
end
- def test_limit_without_offset_removes_limit
- queries = assert_sql { Account.limit(1).count }
+ def test_limit_with_offset_is_kept
+ return if current_adapter?(:OracleAdapter)
+
+ queries = assert_sql { Account.limit(1).offset(1).count }
assert_equal 1, queries.length
- assert_no_match(/LIMIT/, queries.first)
- assert_no_match(/OFFSET/, queries.first)
+ assert_match(/LIMIT/, queries.first)
+ assert_match(/OFFSET/, queries.first)
end
def test_no_limit_no_offset
View
28 activerecord/test/cases/relations_test.rb
@@ -666,6 +666,34 @@ def test_size
assert_no_queries { assert_equal 5, best_posts.size }
end
+ def test_size_with_limit
+ posts = Post.limit(6)
+
+ assert_queries(1) { assert_equal 6, posts.size }
+ assert ! posts.loaded?
+
+ best_posts = posts.where(:comments_count => 0)
+ best_posts.to_a # force load
+ assert_no_queries { assert_equal 5, best_posts.size }
+ end
+
+ def test_size_with_zero_limit
+ posts = Post.limit(0)
+
+ assert_no_queries { assert_equal 0, posts.size }
+ assert ! posts.loaded?
+
+ posts.to_a # force load
+ assert_no_queries { assert_equal 0, posts.size }
+ end
+
+ def test_empty_with_zero_limit
+ posts = Post.limit(0)
+
+ assert_no_queries { assert_equal true, posts.empty? }
+ assert ! posts.loaded?
+ end
+
def test_count_complex_chained_relations
posts = Post.select('comments_count').where('id is not null').group("author_id").where("comments_count > 0")
Please sign in to comment.
Something went wrong with that request. Please try again.