[Rails3] Fix scoped counts.

RAILS3_NOTES

@@ -65,12 +65,6 @@
 
 = Arel Todo
 
-  338 examples, 0 failures, 3 pending
-
-  * unit/relations
-    • skip_spec.rb  (try adding more, maybe with order)
-    • take_spec.rb  (try adding more, maybe with order)
-
 
   * Review/remove our long winded add_limit_offset!
 

lib/arel/engines/sql/compilers/sqlserver_compiler.rb

@@ -12,7 +12,13 @@ module SqlCompiler
     class SQLServerCompiler < GenericCompiler
 
       def select_sql
-        relation.skipped ? select_sql_with_skipped : select_sql_without_skipped
+        if complex_count_sql?
+          select_sql_with_complex_count
+        elsif relation.skipped
+          select_sql_with_skipped
+        else
+          select_sql_without_skipped
+        end
       end
 
       def delete_sql
@@ -25,6 +31,12 @@ def delete_sql
 
       protected
 
+      def complex_count_sql?
+        projections = relation.projections
+        Count === projections.first && projections.size == 1 &&
+          (relation.taken.present? || relation.wheres.present?) && relation.joins(self).blank?
+      end
+
       def taken_only?
         relation.taken.present? && relation.skipped.blank?
       end
@@ -43,6 +55,31 @@ def all_select_clauses_aliased?
         end
       end
 
+      def select_sql_with_complex_count
+        joins   = relation.joins(self)
+        wheres  = relation.where_clauses
+        groups  = relation.group_clauses
+        havings = relation.having_clauses
+        orders  = relation.order_clauses
+        taken   = relation.taken.to_i
+        skipped = relation.skipped.to_i
+        top_clause = "TOP (#{taken+skipped}) " if relation.taken.present?
+        build_query \
+          "SELECT COUNT([count]) AS [count_id]",
+          "FROM (",
+            "SELECT #{top_clause}ROW_NUMBER() OVER (ORDER BY #{rowtable_order_clauses.join(', ')}) AS [rn],",
+            "1 AS [count]",
+            "FROM #{relation.from_clauses}",
+            (locked unless locked.blank?),
+            (joins unless joins.blank?),
+            ("WHERE #{wheres.join(' AND ')}" unless wheres.blank?),
+            ("GROUP BY #{groups.join(', ')}" unless groups.blank?),
+            ("HAVING #{havings.join(' AND ')}" unless havings.blank?),
+            ("ORDER BY #{orders.join(', ')}" unless orders.blank?),
+          ") AS [_rnt]",
+          "WHERE [_rnt].[rn] > #{relation.skipped.to_i}"
+      end
+
       def select_sql_without_skipped(windowed=false)
         selects = relation.select_clauses
         joins   = relation.joins(self)

test/cases/offset_and_limit_test_sqlserver.rb

@@ -1,9 +1,10 @@
 require 'cases/sqlserver_helper'
 require 'models/book'
+require 'models/post'
 
 class OffsetAndLimitTestSqlserver < ActiveRecord::TestCase
 
-  class Account < ActiveRecord::Base; end
+  fixtures :posts
 
   setup     :create_10_books
   teardown  :destroy_all_books
@@ -49,19 +50,27 @@ class Account < ActiveRecord::Base; end
       assert_sql(sql) { Book.limit(3).offset(5).all }
     end
 
-    should_eventually 'add locks to deepest sub select in limit offset sql that has a limited tally' do
-      options = { :limit => 3, :offset => 5, :lock => 'WITH (NOLOCK)' }
-      select_sql = 'SELECT * FROM books'
-      expected_sql = "SELECT * FROM (SELECT TOP 3 * FROM (SELECT TOP 8 * FROM books WITH (NOLOCK)) AS tmp1) AS tmp2"
-      @connection.add_limit_offset! select_sql, options
-      assert_equal expected_sql, @connection.add_lock!(select_sql,options)
+    should 'add locks to deepest sub select' do
+      pattern = /FROM \[books\] WITH \(NOLOCK\)/
+      assert_sql(pattern) { Book.all :limit => 3, :offset => 5, :lock => 'WITH (NOLOCK)' }
+      assert_sql(pattern) { Book.count :limit => 3, :offset => 5, :lock => 'WITH (NOLOCK)' }
     end
 
-    should_eventually 'not create invalid SQL with subquery SELECTs with TOP' do
-      options = { :limit => 5, :offset => 1 }
-      subquery_select_sql = 'SELECT *, (SELECT TOP (1) [id] FROM [books]) AS [book_id] FROM [books]'
-      expected_sql = "SELECT * FROM (SELECT TOP 5 * FROM (SELECT TOP 6 *, (SELECT TOP 1 id FROM books) AS book_id FROM books) AS tmp1) AS tmp2"
-      assert_equal expected_sql, @connection.add_limit_offset!(subquery_select_sql,options)
+    context 'with count' do
+
+      should 'pass a gauntlet of window tests' do
+        assert_equal 7, Post.count
+        assert_equal 1, Post.limit(1).offset(1).size
+        assert_equal 1, Post.limit(1).offset(5).size
+        assert_equal 1, Post.limit(1).offset(6).size
+        assert_equal 0, Post.limit(1).offset(7).size
+        assert_equal 3, Post.limit(3).offset(4).size
+        assert_equal 2, Post.limit(3).offset(5).size
+        assert_equal 1, Post.limit(3).offset(6).size
+        assert_equal 0, Post.limit(3).offset(7).size
+        assert_equal 0, Post.limit(3).offset(8).size
+      end
+
     end
 
   end
@@ -70,6 +79,7 @@ class Account < ActiveRecord::Base; end
   protected
 
   def create_10_books
+    Book.delete_all
     @books = (1..10).map {|i| Book.create!}
   end
 

test/cases/pessimistic_locking_test_sqlserver.rb

@@ -64,12 +64,10 @@ def setup
       20.times { |n| Person.create!(:first_name => "Thing_#{n}") }
     end
 
-    should_eventually 'cope with un-locked paginated results' do
-      tally_not_locked = %r|SELECT count\(\*\) as TotalRows from \(SELECT TOP 1000000000 \* FROM \[people\]\s+WITH \(NOLOCK\) \) tally|
-      inner_tmp_not_locked = %r|SELECT TOP 15 \* FROM \[people\] WITH \(NOLOCK\)|
-      # Currently association limiting is not locked like the parent.
-      association_limiting_not_locked = %r|SELECT \[readers\]\.\* FROM \[readers\] WITH \(NOLOCK\) WHERE \(\[readers\]\.person_id IN \(1,2,3,4,5\)\)|
-      assert_sql(tally_not_locked,inner_tmp_not_locked) do
+    should 'cope with eager loading un-locked paginated' do
+      eager_ids_sql = /SELECT DISTINCT TOP \(5\).*FROM \[people\] WITH \(NOLOCK\)/
+      loader_sql = /FROM \[people\] WITH \(NOLOCK\).*WHERE \(\[people\].\[id\] IN/
+      assert_sql(eager_ids_sql,loader_sql) do
         Person.all(:include => :readers, :lock => 'WITH (NOLOCK)', :limit => 5, :offset => 10)
       end
     end