From e508595cf70d6ebd057d8bfa6f2cd25a119cda3f Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 28 Oct 2005 09:20:05 +0000 Subject: [PATCH] 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 --- activerecord/CHANGELOG | 4 ++ .../abstract/schema_statements.rb | 2 +- .../connection_adapters/sqlserver_adapter.rb | 40 +++++++++++++------ activerecord/test/finder_test.rb | 6 +++ .../db_definitions/sqlserver.drop.sql | 2 +- 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 @@ def initialize_schema_information 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 @@ def type_cast(value) 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 @@ def select_all(sql, name = nil) 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 @@ def quote_column_name(name) 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 @@ def enable_identity_insert(table_name, enable = true) 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 @@ def change_order_direction(order) 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 @@ def repair_special_columns(sql) 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 @@ def test_find_all_with_prepared_limit_and_offset 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;