From 54a2bf66019d2694ff53f666765faf5bca927c09 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Fri, 25 Feb 2011 15:38:59 -0800
Subject: removing limits and offsets from COUNT queries unless both are
 specified. [#6268 state:resolved]

---
 activerecord/CHANGELOG                             |  9 +++++++
 .../lib/active_record/relation/calculations.rb     | 14 ++++++++++-
 activerecord/lib/active_record/test_case.rb        |  1 +
 activerecord/test/cases/calculations_test.rb       | 28 ++++++++++++++++++++++
 4 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 1f343f690c..92848c8a7d 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,14 @@
 *Rails 3.1.0 (unreleased)*
 
+* limits and offsets are removed from COUNT queries unless both are supplied.
+  For example:
+
+    People.limit(1).count           # => 'SELECT COUNT(*) FROM people'
+    People.offset(1).count          # => 'SELECT COUNT(*) FROM people'
+    People.limit(1).offset(1).count # => 'SELECT COUNT(*) FROM people LIMIT 1 OFFSET 1'
+
+  [lighthouse #6262]
+
 * ActiveRecord::Associations::AssociationProxy has been split. There is now an Association class
   (and subclasses) which are responsible for operating on associations, and then a separate,
   thin wrapper called CollectionProxy, which proxies collection associations.
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index abc4c54109..c1842b1a96 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -204,7 +204,19 @@ module ActiveRecord
 
       relation.select_values = [select_value]
 
-      type_cast_calculated_value(@klass.connection.select_value(relation.to_sql), column_for(column_name), operation)
+      query_builder = relation.arel
+
+      if operation == "count"
+        limit  = relation.limit_value
+        offset = relation.offset_value
+
+        unless limit && offset
+          query_builder.limit  = nil
+          query_builder.offset = nil
+        end
+      end
+
+      type_cast_calculated_value(@klass.connection.select_value(query_builder.to_sql), column_for(column_name), operation)
     end
 
     def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
diff --git a/activerecord/lib/active_record/test_case.rb b/activerecord/lib/active_record/test_case.rb
index 4e711c4884..29efbbcb8c 100644
--- a/activerecord/lib/active_record/test_case.rb
+++ b/activerecord/lib/active_record/test_case.rb
@@ -26,6 +26,7 @@ module ActiveRecord
     def assert_sql(*patterns_to_match)
       $queries_executed = []
       yield
+      $queries_executed
     ensure
       failed_patterns = []
       patterns_to_match.each do |pattern|
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index 3121f1615d..991b1ab181 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -109,6 +109,34 @@ class CalculationsTest < ActiveRecord::TestCase
     assert_equal [2, 6], c.keys.compact
   end
 
+  def test_limit_with_offset_is_kept
+    queries = assert_sql { Account.limit(1).offset(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
+    queries = assert_sql { Account.offset(1).count }
+    assert_equal 1, queries.length
+    assert_no_match(/LIMIT/, queries.first)
+    assert_no_match(/OFFSET/, queries.first)
+  end
+
+  def test_limit_without_offset_removes_limit
+    queries = assert_sql { Account.limit(1).count }
+    assert_equal 1, queries.length
+    assert_no_match(/LIMIT/, queries.first)
+    assert_no_match(/OFFSET/, queries.first)
+  end
+
+  def test_no_limit_no_offset
+    queries = assert_sql { Account.count }
+    assert_equal 1, queries.length
+    assert_no_match(/LIMIT/, queries.first)
+    assert_no_match(/OFFSET/, queries.first)
+  end
+
   def test_should_group_by_summed_field_having_condition
     c = Account.sum(:credit_limit, :group => :firm_id,
                                    :having => 'sum(credit_limit) > 50')
-- 
cgit v1.2.3