Added adapter independent limit clause as a two-element array with the first being the limit, the second being the offset #795 [Sam Stephenson]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@944 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

8 files changed, 69 insertions, 24 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 594d1dc1e2..5cd2126568 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,12 @@
 *SVN*
 
+* Added adapter independent limit clause as a two-element array with the first being the limit, the second being the offset #795 [Sam Stephenson]. Example:
+
+    Developer.find_all nil, 'id ASC', 5      # return the first five developers 
+    Developer.find_all nil, 'id ASC', [3, 8] # return three developers, starting from #8 and forward
+    
+  This doesn't yet work with the DB2 or MS SQL adapters. Patches to make that happen are encouraged. 
+
 * Added alias_method :to_param, :id to Base, such that Active Record objects to be used as URL parameters in Action Pack automatically #812 [Nicholas Seckar/Sam Stephenson]
 
 * Improved the performance of the OCI8 adapter for Oracle #723 [pilx/gjenkins]
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index b945d8d0bc..400f0ceb81 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -346,16 +346,18 @@ module ActiveRecord #:nodoc:
       # table in the database. The +conditions+ can be used to narrow the selection of objects (WHERE-part),
       # such as by "color = 'red'", and arrangement of the selection can be done through +orderings+ (ORDER BY-part),
       # such as by "last_name, first_name DESC". A maximum of returned objects and their offset can be specified in 
-      # +limit+ (LIMIT...OFFSET-part). Examples:
+      # +limit+ with either just a single integer as the limit or as an array with the first element as the limit, 
+      # the second as the offset. Examples:
       #   Project.find_all "category = 'accounts'", "last_accessed DESC", 15
-      #   Project.find_all ["category = ?", category_name], "created ASC", ["? OFFSET ?", 15, 20]
+      #   Project.find_all ["category = ?", category_name], "created ASC", [15, 20]
       def find_all(conditions = nil, orderings = nil, limit = nil, joins = nil)
         sql  = "SELECT * FROM #{table_name} " 
         sql << "#{joins} " if joins
         add_conditions!(sql, conditions)
         sql << "ORDER BY #{orderings} " unless orderings.nil?
 
-        connection.add_limit!(sql, sanitize_sql(limit)) unless limit.nil?
+        limit = sanitize_sql(limit) if limit.is_a? Array and limit.first.is_a? String
+        connection.add_limit!(sql, limit) if limit
 
         find_by_sql(sql)
       end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 0ed496ea43..bc30a94c97 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -353,10 +353,22 @@ module ActiveRecord
       def structure_dump() end
 
       def add_limit!(sql, limit)
+        if limit.is_a? Array
+          limit, offset = *limit
+          add_limit_with_offset!(sql, limit.to_i, offset.to_i)
+        else
+          add_limit_without_offset!(sql, limit)
+        end
+      end
+        
+      def add_limit_with_offset!(sql, limit, offset)
+        sql << " LIMIT #{limit} OFFSET #{offset}"
+      end
+      
+      def add_limit_without_offset!(sql, limit)
         sql << " LIMIT #{limit}"
       end
 
-
       def initialize_schema_information
         begin
           execute "CREATE TABLE schema_info (version #{native_database_types[:integer][:name]}#{native_database_types[:integer][:limit]})"
diff --git a/activerecord/lib/active_record/connection_adapters/db2_adapter.rb b/activerecord/lib/active_record/connection_adapters/db2_adapter.rb
index d5bb1147f9..b712971609 100644
--- a/activerecord/lib/active_record/connection_adapters/db2_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/db2_adapter.rb
@@ -89,7 +89,11 @@ begin
           s.gsub(/'/, "''") # ' (for ruby-mode)
         end
 
-        def add_limit!(sql, limit)
+        def add_limit_with_offset!(sql, limit, offset)
+          raise ArgumentError, 'add_limit_with_offset! not implemented'
+        end
+
+        def add_limit_without_offset!(sql, limit)
           sql << " FETCH FIRST #{limit} ROWS ONLY"
         end
 
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index fd730a12dc..57f3b6f5e9 100755
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -172,7 +172,11 @@ module ActiveRecord
           structure += select_one("SHOW CREATE TABLE #{table.to_a.first.last}")["Create Table"] + ";\n\n"
         end
       end
- 
+      
+      def add_limit_with_offset!(sql, limit, offset)
+        sql << " LIMIT #{offset}, #{limit}"
+      end
+       
       def recreate_database(name)
         drop_database(name)
         create_database(name)
diff --git a/activerecord/lib/active_record/connection_adapters/oci_adapter.rb b/activerecord/lib/active_record/connection_adapters/oci_adapter.rb
index 54feebcb38..06e2273b28 100644
--- a/activerecord/lib/active_record/connection_adapters/oci_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/oci_adapter.rb
@@ -182,10 +182,6 @@ begin
         alias :update :execute
         alias :delete :execute
 
-        def add_limit!(sql, limit)
-          sql << "LIMIT=" << limit.to_s
-        end
-
         def begin_db_transaction()
           @connection.autocommit = false
         end
diff --git a/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb
index f6325d845c..adc7e9d98f 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb
@@ -253,19 +253,12 @@ EOL
         "[#{name}]"
       end
 
-      def add_limit!(sql, limit)
-        if sql =~ /LIMIT/i
-          limit = sql.slice!(/LIMIT.*/).gsub(/LIMIT.(.*)$/, '\1')
-        end
-        if !limit.nil?
-          limit_amount = limit.to_s.include?("OFFSET") ? get_offset_amount(limit) : Array.new([limit])
-          order_by = sql.include?("ORDER BY") ? get_order_by(sql.sub(/.*ORDER\sBY./, "")) : nil
-          if limit_amount.size == 2
-            sql.gsub!(/SELECT/i, "SELECT * FROM ( SELECT TOP #{limit_amount[0]} * FROM ( SELECT TOP #{limit_amount[1]}")<<" ) AS tmp1 ORDER BY #{order_by[1]} ) AS tmp2 ORDER BY #{order_by[0]}"
-          else
-            sql.gsub!(/SELECT/i, "SELECT TOP #{limit_amount[0]}")
-          end
-        end
+      def add_limit_with_offset!(sql, limit, offset)
+        raise ArgumentError, 'add_limit_with_offset! not implemented'
+      end
+
+      def add_limit_without_offset!(sql, limit)
+        raise ArgumentError, 'add_limit_without_offset! not implemented'
       end
 
       def recreate_database(name)
diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb
index 3b239b581c..de6d240faa 100755
--- a/activerecord/test/finder_test.rb
+++ b/activerecord/test/finder_test.rb
@@ -4,7 +4,7 @@ require 'fixtures/topic'
 require 'fixtures/entrant'
 
 class FinderTest < Test::Unit::TestCase
-  fixtures :companies, :topics, :entrants
+  fixtures :companies, :topics, :entrants, :developers
   
   def test_find
     assert_equal(@topics["first"]["title"], Topic.find(1).title)
@@ -244,6 +244,33 @@ class FinderTest < Test::Unit::TestCase
     assert_raises(ActiveRecord::StatementInvalid) { Topic.find_by_sql "select 1 from badtable" }
   end
 
+  def test_find_all_with_limit
+    first_five_developers = Developer.find_all nil, 'id ASC', 5
+    assert_equal 5, first_five_developers.length
+    assert_equal 'David', first_five_developers.first.name
+    assert_equal 'fixture_5', first_five_developers.last.name
+    
+    no_developers = Developer.find_all nil, 'id ASC', 0
+    assert_equal 0, no_developers.length
+    
+    assert_equal first_five_developers, Developer.find_all(nil, 'id ASC', [5])
+    assert_equal no_developers, Developer.find_all(nil, 'id ASC', [0])
+  end
+  
+  def test_find_all_with_limit_and_offset
+    first_three_developers = Developer.find_all nil, 'id ASC', [3, 0]
+    second_three_developers = Developer.find_all nil, 'id ASC', [3, 3]
+    last_two_developers = Developer.find_all nil, 'id ASC', [3, 8]
+    
+    assert_equal 3, first_three_developers.length
+    assert_equal 3, second_three_developers.length
+    assert_equal 2, last_two_developers.length
+    
+    assert_equal 'David', first_three_developers.first.name
+    assert_equal 'fixture_4', second_three_developers.first.name
+    assert_equal 'fixture_9', last_two_developers.first.name
+  end
+
   protected
     def bind(statement, *vars)
       if vars.first.is_a?(Hash)