Fixed SQL Server adapter so it honors options[:conditions] when applying :limits (closes #1978) [Tom Ward] Fixed SQL Server adapter to pass even more tests and do even better (closes #2634) [rtomayko@gmail.com]

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

5 files changed, 39 insertions, 15 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index e228cbaeb1..f34800a3e4 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,9 @@
 *SVN*
 
+* Fixed SQL Server adapter to pass even more tests and do even better #2634 [rtomayko@gmail.com]
+
+* Fixed SQL Server adapter so it honors options[:conditions] when applying :limits #1978 [Tom Ward]
+
 * Added migration support to SQL Server adapter (please someone do the same for Oracle and DB2) #2625 [Tom Ward]
 
 * Use AR::Base.silence rather than AR::Base.logger.silence in fixtures to preserve Log4r compatibility.  #2618 [dansketcher@gmail.com]
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index 6a6f368210..847cd4eeee 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -222,7 +222,7 @@ module ActiveRecord
       def dump_schema_information #:nodoc:
         begin
           if (current_schema = ActiveRecord::Migrator.current_version) > 0
-            return "INSERT INTO #{ActiveRecord::Migrator.schema_info_table_name} (version) VALUES (#{current_schema});" 
+            return "INSERT INTO #{ActiveRecord::Migrator.schema_info_table_name} (version) VALUES (#{current_schema})" 
           end
         rescue ActiveRecord::StatementInvalid 
           # No Schema Info
diff --git a/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb
index 7f8933fbb4..7b0a994b46 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb
@@ -77,6 +77,7 @@ module ActiveRecord
         when :datetime  then cast_to_datetime(value)
         when :timestamp then cast_to_time(value)
         when :time      then cast_to_time(value)
+        when :date      then cast_to_datetime(value)
         when :boolean   then value == true or (value =~ /^t(rue)?$/i) == 0 or value.to_s == '1'
         else value
         end
@@ -198,7 +199,7 @@ module ActiveRecord
       end
 
       def select_one(sql, name = nil)
-        add_limit!(sql, nil)
+        add_limit!(sql, :limit => 1)
         result = select(sql, name)
         result.nil? ? nil : result.first
       end
@@ -317,15 +318,27 @@ module ActiveRecord
       end
 
       def add_limit_offset!(sql, options)
-        if options.has_key?(:limit) and options.has_key?(:offset) and !options[:limit].nil? and !options[:offset].nil?
-          options[:order] ||= "id ASC"
-          total_rows = @connection.select_all("SELECT count(*) as TotalRows from #{get_table_name(sql)}")[0][:TotalRows].to_i
-          if (options[:limit] + options[:offset]) > total_rows
-            options[:limit] = (total_rows - options[:offset] > 0) ? (total_rows - options[:offset]) : 1
+        if options[:limit] and options[:offset]
+          total_rows = @connection.select_all("SELECT count(*) as TotalRows from (#{sql.gsub(/SELECT/i, "SELECT TOP 1000000000")}) tally")[0][:TotalRows].to_i
+          if (options[:limit] + options[:offset]) >= total_rows
+            options[:limit] = (total_rows - options[:offset] >= 0) ? (total_rows - options[:offset]) : 0
           end
-          sql.gsub!(/SELECT/i, "SELECT * FROM ( SELECT TOP #{options[:limit]} * FROM ( SELECT TOP #{options[:limit] + options[:offset]}")<<" ) AS tmp1 ORDER BY #{change_order_direction(options[:order])} ) AS tmp2 ORDER BY #{options[:order]}"
-        else
-          sql.gsub!(/SELECT/i, "SELECT TOP #{options[:limit]}") unless options[:limit].nil?
+          sql.sub!(/^\s*SELECT/i, "SELECT * FROM (SELECT TOP #{options[:limit]} * FROM (SELECT TOP #{options[:limit] + options[:offset]} ")
+          sql << ") AS tmp1"
+          if options[:order]
+            options[:order] = options[:order].split(',').map do |field|
+              parts = field.split(" ")
+              if sql =~ /#{parts[0]} AS (t\d_r\d\d?)/
+                  parts[0] = $1
+              end
+              parts.join(' ')
+            end.join(', ')
+            sql << " ORDER BY #{change_order_direction(options[:order])}) AS tmp2 ORDER BY #{options[:order]}"
+          else
+            sql << " ) AS tmp2"
+          end
+        elsif sql !~ /^\s*SELECT (@@|COUNT\()/i
+          sql.sub!(/^\s*SELECT/i, "SELECT TOP #{options[:limit]}") unless options[:limit].nil?
         end
       end
 
@@ -423,9 +436,9 @@ module ActiveRecord
         end
 
         def get_table_name(sql)
-          if sql =~ /into\s*([^\s]+)\s*|update\s*([^\s]+)\s*/i
+          if sql =~ /into\s*([^\(\s]+)\s*|update\s*([^\(\s]+)\s*/i
             $1
-          elsif sql =~ /from\s*([^\s]+)\s*/i
+          elsif sql =~ /from\s*([^\(\s]+)\s*/i
             $1
           else
             nil
@@ -460,8 +473,8 @@ module ActiveRecord
 
         def get_special_columns(table_name)
           special = []
-          @table_columns = {} unless @table_columns
-          @table_columns[table_name] = columns(table_name) if @table_columns[table_name] == nil
+          @table_columns ||= {}
+          @table_columns[table_name] ||= columns(table_name)
           @table_columns[table_name].each do |col|
             special << col.name if col.is_special
           end
@@ -472,6 +485,7 @@ module ActiveRecord
           special_cols = get_special_columns(get_table_name(sql))
           for col in special_cols.to_a
             sql.gsub!(Regexp.new(" #{col.to_s} = "), " #{col.to_s} LIKE ")
+            sql.gsub!(/ORDER BY #{col.to_s}/i, '')
           end
           sql
         end
diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb
index bfaed76a56..7c30308a48 100644
--- a/activerecord/test/finder_test.rb
+++ b/activerecord/test/finder_test.rb
@@ -56,6 +56,12 @@ class FinderTest < Test::Unit::TestCase
     assert_equal(1, entrants.size)
     assert_equal(entrants(:third).name, entrants.first.name)
   end
+  
+  def test_find_with_limit_and_condition
+    developers = Developer.find(:all, :order => "id DESC", :conditions => "salary = 100000", :limit => 3, :offset =>7)
+    assert_equal(1, developers.size)
+    assert_equal("fixture_3", developers.first.name)
+  end
 
   def test_find_with_entire_select_statement
     topics = Topic.find_by_sql "SELECT * FROM topics WHERE author_name = 'Mary'"
diff --git a/activerecord/test/fixtures/db_definitions/sqlserver.drop.sql b/activerecord/test/fixtures/db_definitions/sqlserver.drop.sql
index 4d78d6fda1..edb9aa6e8d 100644
--- a/activerecord/test/fixtures/db_definitions/sqlserver.drop.sql
+++ b/activerecord/test/fixtures/db_definitions/sqlserver.drop.sql
@@ -22,6 +22,6 @@ DROP TABLE authors;
 DROP TABLE tasks;
 DROP TABLE categories;
 DROP TABLE categories_posts;
-DROP TABLE fk_test_has_pd;
 DROP TABLE fk_test_has_fk;
+DROP TABLE fk_test_has_pk;
 DROP TABLE keyboards;