[Rails3] Move supporting limit/offset SQL manipulation to notes and defer a few cases.

Author: metaskills

RAILS3_NOTES

@@ -109,11 +109,47 @@ on a local branch of our remote tracking branch.
 
 
   * Review/remove our long winded add_limit_offset!
-    Supporting:
-      - add_lock!
-      - add_limit_offset_for_association_limiting!
-      - sql_for_association_limiting?
-      - change_order_direction
+
+    def add_lock!(sql, options)
+      # http://blog.sqlauthority.com/2007/04/27/sql-server-2005-locking-hints-and-examples/
+      return unless options[:lock]
+      lock_type = options[:lock] == true ? 'WITH(HOLDLOCK, ROWLOCK)' : options[:lock]
+      sql.gsub! %r|LEFT OUTER JOIN\s+(.*?)\s+ON|im, "LEFT OUTER JOIN \\1 #{lock_type} ON"
+      sql.gsub! %r{FROM\s([\w\[\]\.]+)}im, "FROM \\1 #{lock_type}"
+    end
+    
+    def add_limit_offset_for_association_limiting!(sql, options)
+      sql.replace %|
+        SET NOCOUNT ON
+        DECLARE @row_number TABLE (row int identity(1,1), id int)
+        INSERT INTO @row_number (id)
+          #{sql}
+        SET NOCOUNT OFF
+        SELECT id FROM (
+          SELECT TOP #{options[:limit]} * FROM (
+            SELECT TOP #{options[:limit] + options[:offset]} * FROM @row_number ORDER BY row
+          ) AS tmp1 ORDER BY row DESC
+        ) AS tmp2 ORDER BY row
+      |.gsub(/[ \t\r\n]+/,' ')
+    end
+    
+    def sql_for_association_limiting?(sql)
+      if md = sql.match(/^\s*SELECT(.*)FROM.*GROUP BY.*ORDER BY.*/im)
+        select_froms = md[1].split(',')
+        select_froms.size == 1 && !select_froms.first.include?('*')
+      end
+    end
+
+    def change_order_direction(order)
+      order.split(",").collect {|fragment|
+        case fragment
+          when  /\bDESC\b/i     then fragment.gsub(/\bDESC\b/i, "ASC")
+          when  /\bASC\b/i      then fragment.gsub(/\bASC\b/i, "DESC")
+          else                  String.new(fragment).split(',').join(' DESC,') + ' DESC'
+        end
+      }.join(",")
+    end
+  
 
   * Find out if this is needed anywhere.
 

lib/active_record/connection_adapters/sqlserver/database_statements.rb

@@ -375,48 +375,6 @@ def finish_statement_handle(handle)
           handle
         end
 
-        # === SQLServer Specific (Misc, Doomed?) ======================== #
-        
-        def add_lock!(sql, options)
-          # http://blog.sqlauthority.com/2007/04/27/sql-server-2005-locking-hints-and-examples/
-          return unless options[:lock]
-          lock_type = options[:lock] == true ? 'WITH(HOLDLOCK, ROWLOCK)' : options[:lock]
-          sql.gsub! %r|LEFT OUTER JOIN\s+(.*?)\s+ON|im, "LEFT OUTER JOIN \\1 #{lock_type} ON"
-          sql.gsub! %r{FROM\s([\w\[\]\.]+)}im, "FROM \\1 #{lock_type}"
-        end
-        
-        def sql_for_association_limiting?(sql)
-          if md = sql.match(/^\s*SELECT(.*)FROM.*GROUP BY.*ORDER BY.*/im)
-            select_froms = md[1].split(',')
-            select_froms.size == 1 && !select_froms.first.include?('*')
-          end
-        end
-        
-        def add_limit_offset_for_association_limiting!(sql, options)
-          sql.replace %|
-            SET NOCOUNT ON
-            DECLARE @row_number TABLE (row int identity(1,1), id int)
-            INSERT INTO @row_number (id)
-              #{sql}
-            SET NOCOUNT OFF
-            SELECT id FROM (
-              SELECT TOP #{options[:limit]} * FROM (
-                SELECT TOP #{options[:limit] + options[:offset]} * FROM @row_number ORDER BY row
-              ) AS tmp1 ORDER BY row DESC
-            ) AS tmp2 ORDER BY row
-          |.gsub(/[ \t\r\n]+/,' ')
-        end
-        
-        def change_order_direction(order)
-          order.split(",").collect {|fragment|
-            case fragment
-              when  /\bDESC\b/i     then fragment.gsub(/\bDESC\b/i, "ASC")
-              when  /\bASC\b/i      then fragment.gsub(/\bASC\b/i, "DESC")
-              else                  String.new(fragment).split(',').join(' DESC,') + ' DESC'
-            end
-          }.join(",")
-        end
-        
       end
     end
   end

test/cases/adapter_test_sqlserver.rb

@@ -140,19 +140,19 @@ def setup
 
     context 'for #sql_for_association_limiting?' do
 
-      should 'return false for simple selects with no GROUP BY and ORDER BY' do
+      should_eventually 'return false for simple selects with no GROUP BY and ORDER BY' do
         assert !sql_for_association_limiting?("SELECT * FROM [posts]")
       end
 
-      should 'return true to single SELECT, ideally a table/primarykey, that also has a GROUP BY and ORDER BY' do
+      should_eventually 'return true to single SELECT, ideally a table/primarykey, that also has a GROUP BY and ORDER BY' do
         assert sql_for_association_limiting?("SELECT [posts].id FROM...GROUP BY [posts].id ORDER BY MIN(posts.id)")
       end
 
-      should 'return false to single * wildcard SELECT that also has a GROUP BY and ORDER BY' do
+      should_eventually 'return false to single * wildcard SELECT that also has a GROUP BY and ORDER BY' do
         assert !sql_for_association_limiting?("SELECT * FROM...GROUP BY [posts].id ORDER BY MIN(posts.id)")
       end
 
-      should 'return false to multiple columns in the select even when GROUP BY and ORDER BY are present' do
+      should_eventually 'return false to multiple columns in the select even when GROUP BY and ORDER BY are present' do
         sql = "SELECT [accounts].credit_limit, firm_id FROM...GROUP BY firm_id ORDER BY firm_id"
         assert !sql_for_association_limiting?(sql)
       end
@@ -690,10 +690,6 @@ def setup
 
   private
 
-  def sql_for_association_limiting?(sql)
-    @connection.send :sql_for_association_limiting?, sql
-  end
-  
   def orders_and_dirs_set(order)
     @connection.send :orders_and_dirs_set, order
   end

test/cases/offset_and_limit_test_sqlserver.rb

@@ -70,7 +70,7 @@ def setup
       assert_equal(expected_sql, @connection.add_limit_offset!(@select_sql, options))
     end
 
-    should 'add locks to deepest sub select in limit offset sql that has a limited tally' do
+    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)' }
       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