SQLServer: added tests to ensure all database statements are closed, refactored identity_insert management code to use blocks, removed update/delete rowcount code out of execute and into update/delete, changed insert to go through execute method, removed unused quoting methods, disabled pessimistic locking tests as feature is currently unsupported, fixed RakeFile to load sqlserver specific tests whether running in ado or odbc mode, fixed support for recently added decimal types, added support for limits on integer types. Closes #5670.

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

6 files changed, 289 insertions, 177 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 16391e81c4..e19b8d2f90 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,9 @@
 *SVN*
 
+* SQLServer: added tests to ensure all database statements are closed, refactored identity_insert management code to use blocks, removed update/delete rowcount code out of execute and into update/delete, changed insert to go through execute method, removed unused quoting methods, disabled pessimistic locking tests as feature is currently unsupported, fixed RakeFile to load sqlserver specific tests whether running in ado or odbc mode, fixed support for recently added decimal types, added support for limits on integer types. #5670 [Tom Ward]
+
+* SQLServer: fix db:schema:dump case-sensitivity. #4684 [Will Rogers]
+
 * Oracle: BigDecimal support. #5667 [schoenm@earthlink.net]
 
 * Numeric and decimal columns map to BigDecimal instead of Float. Those with scale 0 map to Integer. #5454 [robbat2@gentoo.org, work@ashleymoran.me.uk]
diff --git a/activerecord/Rakefile b/activerecord/Rakefile
index 97f9cc353a..c54731e121 100755
--- a/activerecord/Rakefile
+++ b/activerecord/Rakefile
@@ -30,7 +30,11 @@ task :default => [ :test_mysql, :test_sqlite, :test_postgresql ]
 for adapter in %w( mysql postgresql sqlite sqlite3 firebird sqlserver sqlserver_odbc db2 oracle sybase openbase frontbase )
   Rake::TestTask.new("test_#{adapter}") { |t|
     t.libs << "test" << "test/connections/native_#{adapter}"
-    t.pattern = "test/*_test{,_#{adapter}}.rb"
+    if adapter =~ /^sqlserver/
+      t.pattern = "test/*_test{,_sqlserver}.rb"
+    else
+      t.pattern = "test/*_test{,_#{adapter}}.rb"
+    end
     t.verbose = true
   }
 end
diff --git a/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb
index 04a6443159..341a388121 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb
@@ -13,12 +13,14 @@ require 'bigdecimal/util'
 #
 # Modifications (ODBC): Mark Imbriaco <mark.imbriaco@pobox.com>
 # Date: 6/26/2005
-#
-# Current maintainer: Ryan Tomayko <rtomayko@gmail.com>
-#
+
 # Modifications (Migrations): Tom Ward <tom@popdog.net>
 # Date: 27/10/2005
 #
+# Modifications (Numerous fixes as maintainer): Ryan Tomayko <rtomayko@gmail.com>
+# Date: Up to July 2006
+
+# Current maintainer: Tom Ward <tom@popdog.net>
 
 module ActiveRecord
   class Base
@@ -81,7 +83,7 @@ module ActiveRecord
         else super
         end
       end
-
+      
       def cast_to_time(value)
         return value if value.is_a?(Time)
         time_array = ParseDate.parsedate(value)
@@ -89,6 +91,8 @@ module ActiveRecord
       end
 
       def cast_to_datetime(value)
+        return value.to_time if value.is_a?(DBI::Timestamp)
+        
         if value.is_a?(Time)
           if value.year != 0 and value.month != 0 and value.day != 0
             return value
@@ -96,9 +100,24 @@ module ActiveRecord
             return Time.mktime(2000, 1, 1, value.hour, value.min, value.sec) rescue nil
           end
         end
+   
+        if value.is_a?(DateTime)
+          return Time.mktime(value.year, value.mon, value.day, value.hour, value.min, value.sec)
+        end
+        
         return cast_to_time(value) if value.is_a?(Date) or value.is_a?(String) rescue nil
         value
       end
+      
+      # TODO: Find less hack way to convert DateTime objects into Times
+      
+      def self.string_to_time(value)
+        if value.is_a?(DateTime)
+          return Time.mktime(value.year, value.mon, value.day, value.hour, value.min, value.sec)
+        else
+          super
+        end
+      end
 
       # These methods will only allow the adapter to insert binary data with a length of 7K or less
       # because of a SQL Server statement length policy.
@@ -184,8 +203,8 @@ module ActiveRecord
           :timestamp   => { :name => "datetime" },
           :time        => { :name => "datetime" },
           :date        => { :name => "datetime" },
-          :binary      => { :name => "image" },
-          :boolean     => { :name => "bit" }
+          :binary      => { :name => "image"},
+          :boolean     => { :name => "bit"}
         }
       end
 
@@ -197,6 +216,18 @@ module ActiveRecord
         true
       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
+          'integer'
+        elsif limit < 4
+          'smallint'
+        else
+          'bigint'
+        end
+      end
+
       # CONNECTION MANAGEMENT ====================================#
 
       # Returns true if the connection is active.
@@ -236,16 +267,20 @@ module ActiveRecord
         return [] if table_name.blank?
         table_name = table_name.to_s if table_name.is_a?(Symbol)
         table_name = table_name.split('.')[-1] unless table_name.nil?
-        sql = "SELECT COLUMN_NAME as ColName,
-                      COLUMN_DEFAULT as DefaultValue,
-                       DATA_TYPE as ColType,
-                       IS_NULLABLE As IsNullable,
-                       COL_LENGTH('#{table_name}', COLUMN_NAME) as Length,
-                       COLUMNPROPERTY(OBJECT_ID('#{table_name}'), COLUMN_NAME, 'IsIdentity') as IsIdentity,
-                       NUMERIC_PRECISION as [Precision],
-                       NUMERIC_SCALE as Scale
-               FROM INFORMATION_SCHEMA.COLUMNS
-               WHERE TABLE_NAME = '#{table_name}'"
+        sql = %Q{
+          SELECT 
+            cols.COLUMN_NAME as ColName,  
+            cols.COLUMN_DEFAULT as DefaultValue,
+            cols.NUMERIC_SCALE as numeric_scale,
+            cols.NUMERIC_PRECISION as numeric_precision, 
+            cols.DATA_TYPE as ColType, 
+            cols.IS_NULLABLE As IsNullable,  
+            COL_LENGTH(cols.TABLE_NAME, cols.COLUMN_NAME) as Length,  
+            COLUMNPROPERTY(OBJECT_ID(cols.TABLE_NAME), cols.COLUMN_NAME, 'IsIdentity') as IsIdentity,  
+            cols.NUMERIC_SCALE as Scale 
+          FROM INFORMATION_SCHEMA.COLUMNS cols 
+          WHERE cols.TABLE_NAME = '#{table_name}'   
+        }
         # Comment out if you want to have the Columns select statment logged.
         # Personally, I think it adds unnecessary bloat to the log. 
         # If you do comment it out, make sure to un-comment the "result" line that follows
@@ -255,7 +290,7 @@ module ActiveRecord
         result.each do |field|
           default = field[:DefaultValue].to_s.gsub!(/[()\']/,"") =~ /null/ ? nil : field[:DefaultValue]
           if field[:ColType] =~ /numeric|decimal/i
-            type = "#{field[:ColType]}(#{field[:Precision]},#{field[:Scale]})"
+            type = "#{field[:ColType]}(#{field[:numeric_precision]},#{field[:numeric_scale]})"
           else
             type = "#{field[:ColType]}(#{field[:Length]})"
           end
@@ -267,62 +302,36 @@ module ActiveRecord
       end
 
       def insert(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil)
-        begin
-          table_name = get_table_name(sql)
-          col = get_identity_column(table_name)
-          ii_enabled = false
-
-          if col != nil
-            if query_contains_identity_column(sql, col)
-              begin
-                execute enable_identity_insert(table_name, true)
-                ii_enabled = true
-              rescue Exception => e
-                raise ActiveRecordError, "IDENTITY_INSERT could not be turned ON"
-              end
-            end
-          end
-          log(sql, name) do
-            @connection.execute(sql).finish
-            id_value || select_one("SELECT @@IDENTITY AS Ident")["Ident"]
-          end
-        ensure
-          if ii_enabled
-            begin
-              execute enable_identity_insert(table_name, false)
-            rescue Exception => e
-              raise ActiveRecordError, "IDENTITY_INSERT could not be turned OFF"
-            end
-          end
-        end
+        execute(sql, name)
+        id_value || select_one("SELECT @@IDENTITY AS Ident")["Ident"]
       end
 
+      def update(sql, name = nil)
+        execute(sql, name) do |handle|
+          handle.rows
+        end || select_one("SELECT @@ROWCOUNT AS AffectedRows")["AffectedRows"]        
+      end
+      
+      alias_method :delete, :update
+
       def execute(sql, name = nil)
-        if sql =~ /^\s*INSERT/i
-          insert(sql, name)
-        elsif sql =~ /^\s*UPDATE|^\s*DELETE/i
+        if sql =~ /^\s*INSERT/i && (table_name = query_requires_identity_insert?(sql))
           log(sql, name) do
-            ret = @connection.execute(sql).finish
-            retVal = select_one("SELECT @@ROWCOUNT AS AffectedRows")["AffectedRows"]
+            with_identity_insert_enabled(table_name) do 
+              @connection.execute(sql) do |handle|
+                yield(handle) if block_given?
+              end
+            end
           end
         else
           log(sql, name) do
-            if block_given?
-              @connection.execute(sql) do |sth|
-                yield(sth)
-              end
-            else
-              @connection.execute(sql).finish
+            @connection.execute(sql) do |handle|
+              yield(handle) if block_given?
             end
           end
         end
       end
-
-      def update(sql, name = nil)
-        execute(sql, name)
-      end
-      alias_method :delete, :update
-
+      
       def begin_db_transaction
         @connection["AutoCommit"] = false
       rescue Exception => e
@@ -356,14 +365,6 @@ module ActiveRecord
         string.gsub(/\'/, "''")
       end
 
-      def quoted_true
-        "1"
-      end
-      
-      def quoted_false
-        "0"
-      end
-
       def quote_column_name(name)
         "[#{name}]"
       end
@@ -420,7 +421,7 @@ module ActiveRecord
       end
 
       def tables(name = nil)
-        execute("SELECT table_name from information_schema.tables WHERE table_type = 'BASE TABLE'", name) do |sth|
+        execute("SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_TYPE = 'BASE TABLE'", name) do |sth|
           sth.inject([]) do |tables, field|
             table_name = field[0]
             tables << table_name unless table_name == 'dtproperties'
@@ -450,19 +451,25 @@ module ActiveRecord
         execute "EXEC sp_rename '#{name}', '#{new_name}'"
       end
       
-      def remove_column(table_name, column_name)
-        execute "ALTER TABLE #{table_name} DROP COLUMN #{column_name}"
-      end 
-      
+      # Adds a new column to the named table.
+      # See TableDefinition#column for details of the options you can use.
+      def add_column(table_name, column_name, type, options = {})
+        add_column_sql = "ALTER TABLE #{table_name} ADD #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
+        add_column_options!(add_column_sql, options)
+        # TODO: Add support to mimic date columns, using constraints to mark them as such in the database
+        # add_column_sql << " CONSTRAINT ck__#{table_name}__#{column_name}__date_only CHECK ( CONVERT(CHAR(12), #{quote_column_name(column_name)}, 14)='00:00:00:000' )" if type == :date       
+        execute(add_column_sql)
+      end
+       
       def rename_column(table, column, new_column_name)
         execute "EXEC sp_rename '#{table}.#{column}', '#{new_column_name}'"
       end
       
       def change_column(table_name, column_name, type, options = {}) #:nodoc:
         sql_commands = ["ALTER TABLE #{table_name} ALTER COLUMN #{column_name} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"]
-        if options[:default]
+        unless options[:default].nil?
           remove_default_constraint(table_name, column_name)
-          sql_commands << "ALTER TABLE #{table_name} ADD CONSTRAINT DF_#{table_name}_#{column_name} DEFAULT #{options[:default]} FOR #{column_name}"
+          sql_commands << "ALTER TABLE #{table_name} ADD CONSTRAINT DF_#{table_name}_#{column_name} DEFAULT #{quote(options[:default])} FOR #{column_name}"
         end
         sql_commands.each {|c|
           execute(c)
@@ -470,22 +477,32 @@ module ActiveRecord
       end
       
       def remove_column(table_name, column_name)
+        remove_check_constraints(table_name, column_name)
         remove_default_constraint(table_name, column_name)
-        execute "ALTER TABLE #{table_name} DROP COLUMN #{column_name}"
+        execute "ALTER TABLE [#{table_name}] DROP COLUMN [#{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|
+        constraints = 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"
+        
+        constraints.each do |constraint|
           execute "ALTER TABLE #{table_name} DROP CONSTRAINT #{constraint["name"]}"
-        }
+        end
+      end
+      
+      def remove_check_constraints(table_name, column_name)
+        # TODO remove all constraints in single method
+        constraints = select "SELECT CONSTRAINT_NAME FROM INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE where TABLE_NAME = '#{table_name}' and COLUMN_NAME = '#{column_name}'"
+        constraints.each do |constraint|
+          execute "ALTER TABLE #{table_name} DROP CONSTRAINT #{constraint["CONSTRAINT_NAME"]}"
+        end
       end
       
       def remove_index(table_name, options = {})
         execute "DROP INDEX #{table_name}.#{quote_column_name(index_name(table_name, options))}"
       end
 
-      private
+      private 
         def select(sql, name = nil)
           rows = []
           repair_special_columns(sql)
@@ -494,7 +511,10 @@ module ActiveRecord
               record = {}
               row.column_names.each do |col|
                 record[col] = row[col]
-                record[col] = record[col].to_time if record[col].is_a? DBI::Timestamp
+                if record[col].is_a? DBI::Timestamp
+                  ts = record[col]
+                  record[col] = DateTime.new(ts.year, ts.month, ts.day, ts.hour, ts.minute, ts.sec)
+                end
               end
               rows << record
             end
@@ -502,10 +522,21 @@ module ActiveRecord
           rows
         end
 
-        def enable_identity_insert(table_name, enable = true)
-          if has_identity_column(table_name)
-            "SET IDENTITY_INSERT #{table_name} #{enable ? 'ON' : 'OFF'}"
-          end
+        # Turns IDENTITY_INSERT ON for table during execution of the block
+        # N.B. This sets the state of IDENTITY_INSERT to OFF after the
+        # block has been executed without regard to its previous state
+
+        def with_identity_insert_enabled(table_name, &block)
+          set_identity_insert(table_name, true)
+          yield
+        ensure
+          set_identity_insert(table_name, false)  
+        end
+        
+        def set_identity_insert(table_name, enable = true)
+          execute "SET IDENTITY_INSERT #{table_name} #{enable ? 'ON' : 'OFF'}"
+        rescue Exception => e
+          raise ActiveRecordError, "IDENTITY_INSERT could not be turned #{enable ? 'ON' : 'OFF'} for table #{table_name}"  
         end
 
         def get_table_name(sql)
@@ -518,11 +549,7 @@ module ActiveRecord
           end
         end
 
-        def has_identity_column(table_name)
-          !get_identity_column(table_name).nil?
-        end
-
-        def get_identity_column(table_name)
+        def identity_column(table_name)
           @table_columns = {} unless @table_columns
           @table_columns[table_name] = columns(table_name) if @table_columns[table_name] == nil
           @table_columns[table_name].each do |col|
@@ -532,8 +559,10 @@ module ActiveRecord
           return nil
         end
 
-        def query_contains_identity_column(sql, col)
-          sql =~ /\[#{col}\]/
+        def query_requires_identity_insert?(sql)
+          table_name = get_table_name(sql)
+          id_column = identity_column(table_name)
+          sql =~ /\[#{id_column}\]/ ? table_name : nil
         end
 
         def change_order_direction(order)
diff --git a/activerecord/test/adapter_test_sqlserver.rb b/activerecord/test/adapter_test_sqlserver.rb
new file mode 100644
index 0000000000..11f244e9f7
--- /dev/null
+++ b/activerecord/test/adapter_test_sqlserver.rb
@@ -0,0 +1,67 @@
+require 'abstract_unit'

+require 'fixtures/default'

+require 'fixtures/post'

+require 'fixtures/task'

+

+class SqlServerAdapterTest < Test::Unit::TestCase

+  fixtures :posts, :tasks

+  

+  def setup

+    @connection = ActiveRecord::Base.connection

+  end

+  

+  def test_execute_without_block_closes_statement

+    assert_all_statements_used_are_closed do

+      @connection.execute("SELECT 1")

+    end

+  end

+  

+  def test_execute_with_block_closes_statement

+    assert_all_statements_used_are_closed do

+      @connection.execute("SELECT 1") do |sth|

+        assert !sth.finished?, "Statement should still be alive within block"

+      end

+    end

+  end

+  

+  def test_insert_with_identity_closes_statement

+    assert_all_statements_used_are_closed do

+      @connection.insert("INSERT INTO accounts ([id], [firm_id],[credit_limit]) values (999, 1, 50)")

+    end

+  end

+  

+  def test_insert_without_identity_closes_statement

+    assert_all_statements_used_are_closed do

+      @connection.insert("INSERT INTO accounts ([firm_id],[credit_limit]) values (1, 50)")

+    end

+  end

+  

+  def test_active_closes_statement

+    assert_all_statements_used_are_closed do

+      @connection.active?

+    end

+  end 

+

+  def assert_all_statements_used_are_closed(&block)

+    existing_handles = []

+    ObjectSpace.each_object(DBI::StatementHandle) {|handle| existing_handles << handle}

+    GC.disable

+    

+    yield

+    

+    used_handles = []

+    ObjectSpace.each_object(DBI::StatementHandle) {|handle| used_handles << handle unless existing_handles.include? handle}

+         

+    assert_block "No statements were used within given block" do

+      used_handles.size > 0

+    end

+    

+    ObjectSpace.each_object(DBI::StatementHandle) do |handle|

+      assert_block "Statement should have been closed within given block" do 

+        handle.finished?

+      end      

+    end

+  ensure

+    GC.enable

+  end

+end

diff --git a/activerecord/test/locking_test.rb b/activerecord/test/locking_test.rb
index 58c34a69e3..00df4bb8e4 100644
--- a/activerecord/test/locking_test.rb
+++ b/activerecord/test/locking_test.rb
@@ -63,91 +63,96 @@ end
 # is so cumbersome. Will deadlock Ruby threads if the underlying db.execute
 # blocks, so separate script called by Kernel#system is needed.
 # (See exec vs. async_exec in the PostgreSQL adapter.)
-class PessimisticLockingTest < Test::Unit::TestCase
-  self.use_transactional_fixtures = false
-  fixtures :people
 
-  def setup
-    @allow_concurrency = ActiveRecord::Base.allow_concurrency
-    ActiveRecord::Base.allow_concurrency = true
-  end
-
-  def teardown
-    ActiveRecord::Base.allow_concurrency = @allow_concurrency
-  end
+# TODO: The SQL Server adapter currently has no support for pessimistic locking
 
-  # Test that the adapter doesn't blow up on add_lock!
-  def test_sane_find_with_lock
-    assert_nothing_raised do
-      Person.transaction do
-        Person.find 1, :lock => true
-      end
+unless current_adapter?(:SQLServerAdapter)
+  class PessimisticLockingTest < Test::Unit::TestCase
+    self.use_transactional_fixtures = false
+    fixtures :people
+  
+    def setup
+      @allow_concurrency = ActiveRecord::Base.allow_concurrency
+      ActiveRecord::Base.allow_concurrency = true
     end
-  end
-
-  # Test no-blowup for scoped lock.
-  def test_sane_find_with_lock
-    assert_nothing_raised do
-      Person.transaction do
-        Person.with_scope(:find => { :lock => true }) do
-          Person.find 1
+  
+    def teardown
+      ActiveRecord::Base.allow_concurrency = @allow_concurrency
+    end
+  
+    # Test that the adapter doesn't blow up on add_lock!
+    def test_sane_find_with_lock
+      assert_nothing_raised do
+        Person.transaction do
+          Person.find 1, :lock => true
         end
       end
     end
-  end
-
-  # Locking a record reloads it.
-  def test_sane_lock_method
-    assert_nothing_raised do
-      Person.transaction do
-        person = Person.find 1
-        old, person.first_name = person.first_name, 'fooman'
-        person.lock!
-        assert_equal old, person.first_name
+  
+    # Test no-blowup for scoped lock.
+    def test_sane_find_with_lock
+      assert_nothing_raised do
+        Person.transaction do
+          Person.with_scope(:find => { :lock => true }) do
+            Person.find 1
+          end
+        end
       end
     end
-  end
-
-  if current_adapter?(:PostgreSQLAdapter, :OracleAdapter)
-    def test_no_locks_no_wait
-      first, second = duel { Person.find 1 }
-      assert first.end > second.end
-    end
-
-    def test_second_lock_waits
-      assert [0.2, 1, 5].any? { |zzz|
-        first, second = duel(zzz) { Person.find 1, :lock => true }
-        second.end > first.end
-      }
+  
+    # Locking a record reloads it.
+    def test_sane_lock_method
+      assert_nothing_raised do
+        Person.transaction do
+          person = Person.find 1
+          old, person.first_name = person.first_name, 'fooman'
+          person.lock!
+          assert_equal old, person.first_name
+        end
+      end
     end
-
-    protected
-      def duel(zzz = 5)
-        t0, t1, t2, t3 = nil, nil, nil, nil
-
-        a = Thread.new do
-          t0 = Time.now
-          Person.transaction do
-            yield
-            sleep zzz       # block thread 2 for zzz seconds
+  
+    if current_adapter?(:PostgreSQLAdapter, :OracleAdapter)
+      def test_no_locks_no_wait
+        first, second = duel { Person.find 1 }
+        assert first.end > second.end
+      end
+  
+      def test_second_lock_waits
+        assert [0.2, 1, 5].any? { |zzz|
+          first, second = duel(zzz) { Person.find 1, :lock => true }
+          second.end > first.end
+        }
+      end
+  
+      protected
+        def duel(zzz = 5)
+          t0, t1, t2, t3 = nil, nil, nil, nil
+  
+          a = Thread.new do
+            t0 = Time.now
+            Person.transaction do
+              yield
+              sleep zzz       # block thread 2 for zzz seconds
+            end
+            t1 = Time.now
           end
-          t1 = Time.now
-        end
-
-        b = Thread.new do
-          sleep zzz / 2.0   # ensure thread 1 tx starts first
-          t2 = Time.now
-          Person.transaction { yield }
-          t3 = Time.now
+  
+          b = Thread.new do
+            sleep zzz / 2.0   # ensure thread 1 tx starts first
+            t2 = Time.now
+            Person.transaction { yield }
+            t3 = Time.now
+          end
+  
+          a.join
+          b.join
+  
+          assert t1 > t0 + zzz
+          assert t2 > t0
+          assert t3 > t2
+          [t0.to_f..t1.to_f, t2.to_f..t3.to_f]
         end
-
-        a.join
-        b.join
-
-        assert t1 > t0 + zzz
-        assert t2 > t0
-        assert t3 > t2
-        [t0.to_f..t1.to_f, t2.to_f..t3.to_f]
-      end
+    end
   end
 end
diff --git a/activerecord/test/migration_test.rb b/activerecord/test/migration_test.rb
index 3d4dfedf3e..649ee54439 100644
--- a/activerecord/test/migration_test.rb
+++ b/activerecord/test/migration_test.rb
@@ -39,8 +39,8 @@ if ActiveRecord::Base.connection.supports_migrations?
       end
       Reminder.reset_column_information
 
-      %w(last_name key bio age height wealth birthday favorite_day male
-         mail administrator).each do |column|
+      %w(last_name key bio age height wealth birthday favorite_day
+         male administrator).each do |column|
         Person.connection.remove_column('people', column) rescue nil
       end
       Person.connection.remove_column("people", "first_name") rescue nil
@@ -177,11 +177,13 @@ if ActiveRecord::Base.connection.supports_migrations?
       Person.connection.create_table :testings do |t|
         t.column :foo, :string
       end
-      Person.connection.execute "insert into testings values (1, 'hello')"
+      
+      con = Person.connection     
+      Person.connection.execute "insert into testings (#{con.quote_column_name('id')}, #{con.quote_column_name('foo')}) values (1, 'hello')"
       assert_nothing_raised {Person.connection.add_column :testings, :bar, :string, :null => false, :default => "default" }
 
       assert_raises(ActiveRecord::StatementInvalid) do
-        Person.connection.execute "insert into testings values (2, 'hello', NULL)"
+        Person.connection.execute "insert into testings (#{con.quote_column_name('id')}, #{con.quote_column_name('foo')}, #{con.quote_column_name('bar')}) values (2, 'hello', NULL)"
       end
     ensure
       Person.connection.drop_table :testings rescue nil
@@ -267,7 +269,7 @@ if ActiveRecord::Base.connection.supports_migrations?
       assert_equal Time, bob.birthday.class
 
       if current_adapter?(:SQLServerAdapter, :OracleAdapter, :SybaseAdapter)
-        # SQL Server, Sybase, and Oracle don't differentiate between date/time
+        # Sybase, and Oracle don't differentiate between date/time
         assert_equal Time, bob.favorite_day.class
       else
         assert_equal Date, bob.favorite_day.class
@@ -355,7 +357,8 @@ if ActiveRecord::Base.connection.supports_migrations?
         ActiveRecord::Base.connection.rename_table :octopuses, :octopi
 
         # Using explicit id in insert for compatibility across all databases
-        assert_nothing_raised { ActiveRecord::Base.connection.execute "INSERT INTO octopi VALUES (1, 'http://www.foreverflying.com/octopus-black7.jpg')" }
+        con = ActiveRecord::Base.connection     
+        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')" }
 
         assert_equal 'http://www.foreverflying.com/octopus-black7.jpg', ActiveRecord::Base.connection.select_value("SELECT url FROM octopi WHERE id=1")
 
@@ -379,7 +382,7 @@ if ActiveRecord::Base.connection.supports_migrations?
       old_columns = Topic.connection.columns(Topic.table_name, "#{name} Columns")
       assert old_columns.find { |c| c.name == 'approved' and c.type == :boolean and c.default == true }
       assert_nothing_raised { Topic.connection.change_column :topics, :approved, :boolean, :default => false }
-      new_columns = Topic.connection.columns(Topic.table_name, "#{name} Columns")
+      new_columns = Topic.connection.columns(Topic.table_name, "#{name} Columns")     
       assert_nil new_columns.find { |c| c.name == 'approved' and c.type == :boolean and c.default == true }
       assert new_columns.find { |c| c.name == 'approved' and c.type == :boolean and c.default == false }
       assert_nothing_raised { Topic.connection.change_column :topics, :approved, :boolean, :default => true }