From 84481dd3b42159428cd48ba67f811ed24deb6ed0 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Fri, 5 Jan 2007 00:12:06 +0000 Subject: Sybase adapter fixes. Closes #6926 [jsheets] git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5839 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/CHANGELOG | 2 + .../connection_adapters/sybase_adapter.rb | 281 ++++++++++----------- activerecord/test/associations/eager_test.rb | 2 +- activerecord/test/base_test.rb | 10 +- .../test/fixtures/db_definitions/sybase.drop.sql | 1 + .../test/fixtures/db_definitions/sybase.sql | 4 +- activerecord/test/locking_test.rb | 4 +- activerecord/test/migration_test.rb | 10 +- 8 files changed, 159 insertions(+), 155 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index f6d3bc2ae9..0404f2e315 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *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] diff --git a/activerecord/lib/active_record/connection_adapters/sybase_adapter.rb b/activerecord/lib/active_record/connection_adapters/sybase_adapter.rb index b8a32de2f8..4287dbe144 100644 --- a/activerecord/lib/active_record/connection_adapters/sybase_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sybase_adapter.rb @@ -1,13 +1,20 @@ # sybase_adaptor.rb -# Author: John Sheets +# 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 ActiveRecord # 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 @@ module ActiveRecord 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 @@ module ActiveRecord } 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 = <= 128 + primary = (sysstat2 & 8) == 8 + ColumnWithIdentity.new(name, default_value, type, nullable, identity, primary) end - indexes end def quoted_true @@ -302,7 +316,7 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 = <= 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 diff --git a/activerecord/test/associations/eager_test.rb b/activerecord/test/associations/eager_test.rb index e8e14c2c50..b303363f00 100644 --- a/activerecord/test/associations/eager_test.rb +++ b/activerecord/test/associations/eager_test.rb @@ -374,7 +374,7 @@ class EagerAssociationTest < Test::Unit::TestCase 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") diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb index ab73ea77a7..672ad6f0bc 100755 --- a/activerecord/test/base_test.rb +++ b/activerecord/test/base_test.rb @@ -622,8 +622,8 @@ class BasicsTest < Test::Unit::TestCase 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 @@ class BasicsTest < Test::Unit::TestCase 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 @@ class BasicsTest < Test::Unit::TestCase 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 diff --git a/activerecord/test/fixtures/db_definitions/sybase.drop.sql b/activerecord/test/fixtures/db_definitions/sybase.drop.sql index fa51eefeeb..ebf2fc9736 100644 --- a/activerecord/test/fixtures/db_definitions/sybase.drop.sql +++ b/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 diff --git a/activerecord/test/fixtures/db_definitions/sybase.sql b/activerecord/test/fixtures/db_definitions/sybase.sql index 79c7b940b5..8544a4150e 100644 --- a/activerecord/test/fixtures/db_definitions/sybase.sql +++ b/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), diff --git a/activerecord/test/locking_test.rb b/activerecord/test/locking_test.rb index f786d77027..471771bfbb 100644 --- a/activerecord/test/locking_test.rb +++ b/activerecord/test/locking_test.rb @@ -81,9 +81,9 @@ end # 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 diff --git a/activerecord/test/migration_test.rb b/activerecord/test/migration_test.rb index 3cfbcac82e..ce79ffc80e 100644 --- a/activerecord/test/migration_test.rb +++ b/activerecord/test/migration_test.rb @@ -58,8 +58,8 @@ if ActiveRecord::Base.connection.supports_migrations? 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 @@ if ActiveRecord::Base.connection.supports_migrations? 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 @@ if ActiveRecord::Base.connection.supports_migrations? # 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 @@ if ActiveRecord::Base.connection.supports_migrations? # 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") -- cgit v1.2.3