Sybase adapter fixes. Closes #6926 [jsheets]

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

activerecord/CHANGELOG

 *SVN*
 
+* Bring the sybase adapter up to scratch for 1.2 release. [jsheets]
+
 * Rollback #new_record? and #id values for created records that rollback in an after_save callback. Closes #6910 [Ben Curren]
 
 * Pushing a record on an association collection doesn't unnecessarily load all the associated records.  [Obie Fernandez, Jeremy Kemper]

activerecord/lib/active_record/connection_adapters/sybase_adapter.rb

 # sybase_adaptor.rb
-# Author: John Sheets <dev@metacasa.net>
+# Author: John R. Sheets
+# 
+# 01 Mar 2006: Initial version.  Based on code from Will Sobel
+#              (http://dev.rubyonrails.org/ticket/2030)
 # 
-# 01 Mar 2006: Initial version.
 # 17 Mar 2006: Added support for migrations; fixed issues with :boolean columns.
+# 
 # 13 Apr 2006: Improved column type support to properly handle dates and user-defined
 #              types; fixed quoting of integer columns.
-#
-# Based on code from Will Sobel (http://dev.rubyonrails.org/ticket/2030)
-#
+# 
+# 05 Jan 2007: Updated for Rails 1.2 release:
+#              restricted Fixtures#insert_fixtures monkeypatch to Sybase adapter;
+#              removed SQL type precision from TEXT type to fix broken
+#              ActiveRecordStore (jburks, #6878); refactored select() to use execute();
+#              fixed leaked exception for no-op change_column(); removed verbose SQL dump
+#              from columns(); added missing scale parameter in normalize_type().
 
 require 'active_record/connection_adapters/abstract_adapter'
 
@@ -77,7 +84,7 @@ module ConnectionAdapters
     #   2> go
     class SybaseAdapter < AbstractAdapter # :nodoc:
       class ColumnWithIdentity < Column
-        attr_reader :identity, :primary
+        attr_reader :identity
 
         def initialize(name, default, sql_type = nil, nullable = nil, identity = nil, primary = nil)
           super(name, default, sql_type, nullable)
@@ -98,16 +105,6 @@ def simplified_type(field_type)
           end
         end
 
-        def self.string_to_time(string)
-          return string unless string.is_a?(String)
-
-          # Since Sybase doesn't handle DATE or TIME, handle it here.
-          # Populate nil year/month/day with string_to_dummy_time() values.
-          time_array = ParseDate.parsedate(string)[0..5]
-          time_array[0] ||= 2000; time_array[1] ||= 1; time_array[2] ||= 1;
-          Time.send(Base.default_timezone, *time_array) rescue nil
-        end
-
         def self.string_to_binary(value)
           "0x#{value.unpack("H*")[0]}"
         end
@@ -148,6 +145,15 @@ def native_database_types
         }
       end
 
+      def type_to_sql(type, limit = nil, precision = nil, scale = nil) #:nodoc:
+        return super unless type.to_s == 'integer'
+        if !limit.nil? && limit < 4
+          'smallint'
+        else
+          'integer'
+        end
+      end
+
       def adapter_name
         'Sybase'
       end
@@ -170,14 +176,6 @@ def table_alias_length
         30
       end
 
-      def columns(table_name, name = nil)
-        table_structure(table_name).inject([]) do |columns, column|
-          name, default, type, nullable, identity, primary = column
-          columns << ColumnWithIdentity.new(name, default, type, nullable, identity, primary)
-          columns
-        end
-      end
-
       def insert(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil)
         begin
           table_name = get_table_name(sql)
@@ -212,46 +210,62 @@ def insert(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil)
       end
 
       def execute(sql, name = nil)
-        log(sql, name) do
-          @connection.context.reset
-          @connection.set_rowcount(@limit || 0)
-          @limit = @offset = nil
-          @connection.sql_norow(sql)
-          if @connection.cmd_fail? or @connection.context.failed?
-            raise "SQL Command Failed for #{name}: #{sql}\nMessage: #{@connection.context.message}"
-          end
-        end
-        # Return rows affected
+        raw_execute(sql, name)
         @connection.results[0].row_count
       end
 
-      def begin_db_transaction()    execute "BEGIN TRAN" end
-      def commit_db_transaction()   execute "COMMIT TRAN" end
-      def rollback_db_transaction() execute "ROLLBACK TRAN" end
+      def begin_db_transaction()    raw_execute "BEGIN TRAN" end
+      def commit_db_transaction()   raw_execute "COMMIT TRAN" end
+      def rollback_db_transaction() raw_execute "ROLLBACK TRAN" end
 
       def current_database
         select_one("select DB_NAME() as name")["name"]
       end
 
       def tables(name = nil)
-        tables = []
-        select("select name from sysobjects where type='U'", name).each do |row|
-          tables << row['name']
-        end
-        tables
+        select("select name from sysobjects where type='U'", name).map { |row| row['name'] }
       end
 
       def indexes(table_name, name = nil)
-        indexes = []
-        select("exec sp_helpindex #{table_name}", name).each do |index|
+        select("exec sp_helpindex #{table_name}", name).map do |index|
           unique = index["index_description"] =~ /unique/
           primary = index["index_description"] =~ /^clustered/
           if !primary
             cols = index["index_keys"].split(", ").each { |col| col.strip! }
-            indexes << IndexDefinition.new(table_name, index["index_name"], unique, cols)
+            IndexDefinition.new(table_name, index["index_name"], unique, cols)
           end
+        end.compact
+      end
+
+      def columns(table_name, name = nil)
+        sql = <<SQLTEXT
+SELECT col.name AS name, type.name AS type, col.prec, col.scale,
+  col.length, col.status, obj.sysstat2, def.text
+ FROM sysobjects obj, syscolumns col, systypes type, syscomments def
+ WHERE obj.id = col.id AND col.usertype = type.usertype AND col.cdefault *= def.id
+  AND obj.type = 'U' AND obj.name = '#{table_name}' ORDER BY col.colid
+SQLTEXT
+        @logger.debug "Get Column Info for table '#{table_name}'" if @logger
+        @connection.set_rowcount(0)
+        @connection.sql(sql)
+
+        raise "SQL Command for table_structure for #{table_name} failed\nMessage: #{@connection.context.message}" if @connection.context.failed?
+        return nil if @connection.cmd_fail?
+
+        @connection.top_row_result.rows.map do |row|
+          name, type, prec, scale, length, status, sysstat2, default = row
+          name.sub!(/_$/o, '')
+          type = normalize_type(type, prec, scale, length)
+          default_value = nil
+          if default =~ /DEFAULT\s+(.+)/o
+            default_value = $1.strip
+            default_value = default_value[1...-1] if default_value =~ /^['"]/o
+          end
+          nullable = (status & 8) == 8
+          identity = status >= 128
+          primary = (sysstat2 & 8) == 8
+          ColumnWithIdentity.new(name, default_value, type, nullable, identity, primary)
         end
-        indexes
       end
 
       def quoted_true
@@ -302,7 +316,7 @@ def quote_column_name(name)
       def add_limit_offset!(sql, options) # :nodoc:
         @limit = options[:limit]
         @offset = options[:offset]
-        if !normal_select?
+        if use_temp_table?
           # Use temp table to hack offset with Sybase
           sql.sub!(/ FROM /i, ' INTO #artemp FROM ')
         elsif zero_limit?
@@ -318,6 +332,11 @@ def add_limit_offset!(sql, options) # :nodoc:
         end
       end
 
+      def add_lock!(sql, options) #:nodoc:
+        @logger.info "Warning: Sybase :lock option '#{options[:lock].inspect}' not supported" if @logger && options.has_key?(:lock)
+        sql
+      end
+
       def supports_migrations? #:nodoc:
         true
       end
@@ -334,13 +353,14 @@ def change_column(table_name, column_name, type, options = {}) #:nodoc:
         begin
           execute "ALTER TABLE #{table_name} MODIFY #{column_name} #{type_to_sql(type, options[:limit])}"
         rescue StatementInvalid => e
-          # Swallow exception if no-op.
+          # Swallow exception and reset context if no-op.
           raise e unless e.message =~ /no columns to drop, add or modify/
+          @connection.context.reset
         end
 
-        if options[:default]
+        if options.has_key?(:default)
           remove_default_constraint(table_name, column_name)
-          execute "ALTER TABLE #{table_name} REPLACE #{column_name} DEFAULT #{options[:default]}"
+          execute "ALTER TABLE #{table_name} REPLACE #{column_name} DEFAULT #{quote options[:default]}"
         end
       end
 
@@ -350,10 +370,10 @@ def remove_column(table_name, column_name)
       end
 
       def remove_default_constraint(table_name, column_name)
-        defaults = select "select def.name from sysobjects def, syscolumns col, sysobjects tab where col.cdefault = def.id and col.name = '#{column_name}' and tab.name = '#{table_name}' and col.id = tab.id"
-        defaults.each {|constraint|
+        sql = "select def.name from sysobjects def, syscolumns col, sysobjects tab where col.cdefault = def.id and col.name = '#{column_name}' and tab.name = '#{table_name}' and col.id = tab.id"
+        select(sql).each do |constraint|
           execute "ALTER TABLE #{table_name} DROP CONSTRAINT #{constraint["name"]}"
-        }
+        end
       end
 
       def remove_index(table_name, options = {})
@@ -418,34 +438,44 @@ def affected_rows(name = nil)
         end
       end
 
-      def normal_select?
-        # If limit is not set at all, we can ignore offset;
-        # if limit *is* set but offset is zero, use normal select
-        # with simple SET ROWCOUNT.  Thus, only use the temp table
-        # if limit is set and offset > 0.
-        has_limit = !@limit.nil?
-        has_offset = !@offset.nil? && @offset > 0
-        !has_limit || !has_offset
+      # If limit is not set at all, we can ignore offset;
+      # if limit *is* set but offset is zero, use normal select
+      # with simple SET ROWCOUNT.  Thus, only use the temp table
+      # if limit is set and offset > 0.
+      def use_temp_table?
+        !@limit.nil? && !@offset.nil? && @offset > 0
       end
 
       def zero_limit?
         !@limit.nil? && @limit == 0
       end
 
-      # Select limit number of rows starting at optional offset.
-      def select(sql, name = nil)
-        @connection.context.reset
+      def raw_execute(sql, name = nil)
         log(sql, name) do
-          if normal_select?
-            # If limit is not explicitly set, return all results.
-            @logger.debug "Setting row count to (#{@limit || 'off'})" if @logger
-
-            # Run a normal select
-            @connection.set_rowcount(@limit || 0)
+          @connection.context.reset
+          @logger.debug "Setting row count to (#{@limit})" if @logger && @limit
+          @connection.set_rowcount(@limit || 0)
+          if sql =~ /^\s*SELECT/i
             @connection.sql(sql)
           else
+            @connection.sql_norow(sql)
+          end
+          @limit = @offset = nil
+          if @connection.cmd_fail? or @connection.context.failed?
+            raise "SQL Command Failed for #{name}: #{sql}\nMessage: #{@connection.context.message}"
+          end
+        end
+      end
+
+      # Select limit number of rows starting at optional offset.
+      def select(sql, name = nil)
+        if !use_temp_table?
+          execute(sql, name)
+        else
+          log(sql, name) do
             # Select into a temp table and prune results
             @logger.debug "Selecting #{@limit + (@offset || 0)} or fewer rows into #artemp" if @logger
+            @connection.context.reset
             @connection.set_rowcount(@limit + (@offset || 0))
             @connection.sql_norow(sql)  # Select into temp table
             @logger.debug "Deleting #{@offset || 0} or fewer rows from #artemp" if @logger
@@ -456,23 +486,21 @@ def select(sql, name = nil)
           end
         end
 
+        raise StatementInvalid, "SQL Command Failed for #{name}: #{sql}\nMessage: #{@connection.context.message}" if @connection.context.failed? or @connection.cmd_fail?
+      
         rows = []
-        if @connection.context.failed? or @connection.cmd_fail?
-          raise StatementInvalid, "SQL Command Failed for #{name}: #{sql}\nMessage: #{@connection.context.message}"
-        else
-          results = @connection.top_row_result
-          if results && results.rows.length > 0
-            fields = results.columns.map { |column| column.sub(/_$/, '') }
-            results.rows.each do |row|
-              hashed_row = {}
-              row.zip(fields) { |cell, column| hashed_row[column] = cell }
-              rows << hashed_row
-            end
+        results = @connection.top_row_result
+        if results && results.rows.length > 0
+          fields = results.columns.map { |column| column.sub(/_$/, '') }
+          results.rows.each do |row|
+            hashed_row = {}
+            row.zip(fields) { |cell, column| hashed_row[column] = cell }
+            rows << hashed_row
           end
         end
-        @connection.sql_norow("drop table #artemp") if !normal_select?
+        @connection.sql_norow("drop table #artemp") if use_temp_table?
         @limit = @offset = nil
-        return rows
+        rows
       end
 
       def get_table_name(sql)
@@ -490,79 +518,42 @@ def has_identity_column(table_name)
       end
 
       def get_identity_column(table_name)
-        @table_columns ||= {}
-        @table_columns[table_name] ||= columns(table_name)
-        @table_columns[table_name].each do |col|
-          return col.name if col.identity
+        @id_columns ||= {}
+        if !@id_columns.has_key?(table_name)
+          @logger.debug "Looking up identity column for table '#{table_name}'" if @logger
+          col = columns(table_name).detect { |col| col.identity }
+          @id_columns[table_name] = col.nil? ? nil : col.name
         end
-        nil
+        @id_columns[table_name]
       end
 
       def query_contains_identity_column(sql, col)
         sql =~ /\[#{col}\]/
       end
 
-      def table_structure(table_name)
-        sql = <<SQLTEXT
-SELECT col.name AS name, type.name AS type, col.prec, col.scale, col.length,
-  col.status, obj.sysstat2, def.text
- FROM sysobjects obj, syscolumns col, systypes type, syscomments def
- WHERE obj.id = col.id AND col.usertype = type.usertype AND col.cdefault *= def.id
-  AND obj.type = 'U' AND obj.name = '#{table_name}' ORDER BY col.colid
-SQLTEXT
-        log(sql, "Get Column Info ") do
-          @connection.set_rowcount(0)
-          @connection.sql(sql)
-        end
-        if @connection.context.failed?
-          raise "SQL Command for table_structure for #{table_name} failed\nMessage: #{@connection.context.message}"
-        elsif !@connection.cmd_fail?
-          columns = []
-          results = @connection.top_row_result
-          results.rows.each do |row|
-            name, type, prec, scale, length, status, sysstat2, default = row
-            type = normalize_type(type, prec, scale, length)
-            default_value = nil
-            name.sub!(/_$/o, '')
-            if default =~ /DEFAULT\s+(.+)/o
-              default_value = $1.strip
-              default_value = default_value[1...-1] if default_value =~ /^['"]/o
-            end
-            nullable = (status & 8) == 8
-            identity = status >= 128
-            primary = (sysstat2 & 8) == 8
-
-            columns << [name, default_value, type, nullable, identity, primary]
-          end
-          columns
-        else
-          nil
-        end
-      end
-
       # Resolve all user-defined types (udt) to their fundamental types.
       def resolve_type(field_type)
         (@udts ||= {})[field_type] ||= select_one("sp_help #{field_type}")["Storage_type"].strip
       end
 
       def normalize_type(field_type, prec, scale, length)
-        if field_type =~ /numeric/i and (scale.nil? or scale == 0)
-          type = 'int'
+        has_scale = (!scale.nil? && scale > 0)
+        type = if field_type =~ /numeric/i and !has_scale
+          'int'
         elsif field_type =~ /money/i
-          type = 'numeric'
+          'numeric'
         else
-          type = resolve_type(field_type.strip)
+          resolve_type(field_type.strip)
         end
-        size = ''
-        if prec
-          size = "(#{prec})"
-        elsif length && !(type =~ /date|time/)
-          size = "(#{length})"
-        end
-        return type + size
-      end
 
-      def default_value(value)
+        spec = if prec
+          has_scale ? "(#{prec},#{scale})" : "(#{prec})"
+        elsif length && !(type =~ /date|time|text/)
+          "(#{length})"
+        else
+          ''
+        end
+        "#{type}#{spec}"
       end
     end # class SybaseAdapter
 
@@ -654,10 +645,14 @@ class Fixtures
   alias :original_insert_fixtures :insert_fixtures
 
   def insert_fixtures
-    values.each do |fixture|
-      @connection.enable_identity_insert(table_name, true)
-      @connection.execute "INSERT INTO #{@table_name} (#{fixture.key_list}) VALUES (#{fixture.value_list})", 'Fixture Insert'
-      @connection.enable_identity_insert(table_name, false)
+    if @connection.instance_of?(ActiveRecord::ConnectionAdapters::SybaseAdapter)
+      values.each do |fixture|
+        @connection.enable_identity_insert(table_name, true)
+        @connection.execute "INSERT INTO #{@table_name} (#{fixture.key_list}) VALUES (#{fixture.value_list})", 'Fixture Insert'
+        @connection.enable_identity_insert(table_name, false)
+      end
+    else
+      original_insert_fixtures
     end
   end
 end

activerecord/test/associations/eager_test.rb

@@ -374,7 +374,7 @@ def test_preconfigured_includes_with_has_many_and_habtm
   end
   
   def test_count_with_include
-    if current_adapter?(:SQLServerAdapter)
+    if current_adapter?(:SQLServerAdapter, :SybaseAdapter)
       assert_equal 3, authors(:david).posts_with_comments.count(:conditions => "len(comments.body) > 15")
     else
       assert_equal 3, authors(:david).posts_with_comments.count(:conditions => "length(comments.body) > 15")

activerecord/test/base_test.rb

@@ -622,8 +622,8 @@ def test_default_values
     end
   end
 
-  # Oracle and SQLServer do not have a TIME datatype.
-  unless current_adapter?(:SQLServerAdapter, :OracleAdapter)
+  # Oracle, SQLServer, and Sybase do not have a TIME datatype.
+  unless current_adapter?(:SQLServerAdapter, :OracleAdapter, :SybaseAdapter)
     def test_utc_as_time_zone
       Topic.default_timezone = :utc
       attributes = { "bonus_time" => "5:42:00AM" }
@@ -852,8 +852,8 @@ def test_multiparameter_assignment_of_aggregation
   end
 
   def test_attributes_on_dummy_time
-    # Oracle and SQL Server do not have a TIME datatype.
-    return true if current_adapter?(:SQLServerAdapter, :OracleAdapter)
+    # Oracle, SQL Server, and Sybase do not have a TIME datatype.
+    return true if current_adapter?(:SQLServerAdapter, :OracleAdapter, :SybaseAdapter)
 
     attributes = {
       "bonus_time" => "5:42:00AM"
@@ -1075,7 +1075,7 @@ def test_quote_keys
   end
 
   def test_sql_injection_via_find
-    assert_raises(ActiveRecord::RecordNotFound) do
+    assert_raises(ActiveRecord::RecordNotFound, ActiveRecord::StatementInvalid) do
       Topic.find("123456 OR id > 0")
     end
   end

activerecord/test/fixtures/db_definitions/sybase.drop.sql

@@ -29,4 +29,5 @@ DROP TABLE fk_test_has_pk
 DROP TABLE keyboards
 DROP TABLE legacy_things
 DROP TABLE numeric_data
+DROP TABLE schema_info
 go

activerecord/test/fixtures/db_definitions/sybase.sql

@@ -197,12 +197,12 @@ CREATE TABLE keyboards (
 CREATE TABLE legacy_things (
   id numeric(9,0) IDENTITY PRIMARY KEY,
   tps_report_number int default NULL,
-  version int default 0,
+  version int default 0
 )
 
 
 CREATE TABLE numeric_data (
-  id numeric((9,0) IDENTITY PRIMARY KEY,
+  id numeric(9,0) IDENTITY PRIMARY KEY,
   bank_balance numeric(10,2),
   big_bank_balance numeric(15,2),
   world_population numeric(10),

activerecord/test/locking_test.rb

@@ -81,9 +81,9 @@ def test_lock_with_custom_column_without_default_sets_version_to_zero
 # blocks, so separate script called by Kernel#system is needed.
 # (See exec vs. async_exec in the PostgreSQL adapter.)
 
-# TODO: The SQL Server adapter currently has no support for pessimistic locking
+# TODO: The SQL Server and Sybase adapters currently have no support for pessimistic locking
 
-unless current_adapter?(:SQLServerAdapter)
+unless current_adapter?(:SQLServerAdapter, :SybaseAdapter)
   class PessimisticLockingTest < Test::Unit::TestCase
     self.use_transactional_fixtures = false
     fixtures :people, :readers

activerecord/test/migration_test.rb

@@ -58,8 +58,8 @@ def test_add_index
       assert_nothing_raised { Person.connection.add_index("people", "last_name") }
       assert_nothing_raised { Person.connection.remove_index("people", "last_name") }
 
-      # Orcl nds shrt indx nms.
-      unless current_adapter?(:OracleAdapter)
+      # Orcl nds shrt indx nms.  Sybs 2.
+      unless current_adapter?(:OracleAdapter, :SybaseAdapter)
         assert_nothing_raised { Person.connection.add_index("people", ["last_name", "first_name"]) }
         assert_nothing_raised { Person.connection.remove_index("people", :column => ["last_name", "first_name"]) }
         assert_nothing_raised { Person.connection.add_index("people", ["last_name", "first_name"]) }
@@ -188,7 +188,9 @@ def test_add_column_not_null_with_default
       end
       
       con = Person.connection     
+      Person.connection.enable_identity_insert("testings", true) if current_adapter?(:SybaseAdapter)
       Person.connection.execute "insert into testings (#{con.quote_column_name('id')}, #{con.quote_column_name('foo')}) values (1, 'hello')"
+      Person.connection.enable_identity_insert("testings", false) if current_adapter?(:SybaseAdapter)
       assert_nothing_raised {Person.connection.add_column :testings, :bar, :string, :null => false, :default => "default" }
 
       assert_raises(ActiveRecord::StatementInvalid) do
@@ -367,7 +369,9 @@ def test_rename_table
 
         # Using explicit id in insert for compatibility across all databases
         con = ActiveRecord::Base.connection     
+        con.enable_identity_insert("octopi", true) if current_adapter?(:SybaseAdapter)
         assert_nothing_raised { con.execute "INSERT INTO octopi (#{con.quote_column_name('id')}, #{con.quote_column_name('url')}) VALUES (1, 'http://www.foreverflying.com/octopus-black7.jpg')" }
+        con.enable_identity_insert("octopi", false) if current_adapter?(:SybaseAdapter)
 
         assert_equal 'http://www.foreverflying.com/octopus-black7.jpg', ActiveRecord::Base.connection.select_value("SELECT url FROM octopi WHERE id=1")
 
@@ -388,7 +392,9 @@ def test_rename_table_with_an_index
 
         # Using explicit id in insert for compatibility across all databases
         con = ActiveRecord::Base.connection     
+        con.enable_identity_insert("octopi", true) if current_adapter?(:SybaseAdapter)
         assert_nothing_raised { con.execute "INSERT INTO octopi (#{con.quote_column_name('id')}, #{con.quote_column_name('url')}) VALUES (1, 'http://www.foreverflying.com/octopus-black7.jpg')" }
+        con.enable_identity_insert("octopi", false) if current_adapter?(:SybaseAdapter)
 
         assert_equal 'http://www.foreverflying.com/octopus-black7.jpg', ActiveRecord::Base.connection.select_value("SELECT url FROM octopi WHERE id=1")
         assert ActiveRecord::Base.connection.indexes(:octopi).first.columns.include?("url")