Fixed handling of nil number columns on Oracle and cleaned up tests for Oracle in general #2555 [schoenm@earthlink.net]

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

9 files changed, 79 insertions, 41 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 2b0cca15f2..a19e005f36 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Fixed handling of nil number columns on Oracle and cleaned up tests for Oracle in general #2555 [schoenm@earthlink.net]
+
 * Added quoted_true and quoted_false methods to db2_adapter and cleaned up tests for DB2 #2493 [maik schmidt]
 
 
diff --git a/activerecord/lib/active_record/connection_adapters/oci_adapter.rb b/activerecord/lib/active_record/connection_adapters/oci_adapter.rb
index 9897403797..395fbb44f0 100644
--- a/activerecord/lib/active_record/connection_adapters/oci_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/oci_adapter.rb
@@ -26,10 +26,15 @@ begin
       class OCIColumn < Column #:nodoc:
         attr_reader :sql_type
 
-        def initialize(name, default, limit, sql_type, scale)
-          @name, @limit, @sql_type, @scale, @sequence = name, limit, sql_type, scale
-          @type = simplified_type sql_type
-          @default = type_cast default
+        def initialize(name, default, sql_type, limit, scale, null)
+          @name, @limit, @sql_type, @scale, @null = name, limit, sql_type, scale, null
+
+          @type = simplified_type(sql_type)
+          @default = type_cast(default)
+
+          @primary = nil
+          @text    = [:string, :text].include? @type
+          @number  = [:float, :integer].include? @type
         end
 
         def simplified_type(field_type)
@@ -127,6 +132,36 @@ begin
           name =~ /[A-Z]/ ? "\"#{name}\"" : name
         end
 
+        def tables(name = nil)
+          select_all("select lower(table_name) from user_tables").inject([]) do | tabs, t |
+            tabs << t.to_a.first.last
+          end
+        end
+
+        def indexes(table_name, name = nil) #:nodoc:
+          result = select_all(<<-SQL, name)
+            SELECT lower(i.index_name) as index_name, i.uniqueness, lower(c.column_name) as column_name
+              FROM user_indexes i, user_ind_columns c
+             WHERE c.index_name = i.index_name
+               AND i.index_name NOT IN (SELECT index_name FROM user_constraints WHERE constraint_type = 'P')
+              ORDER BY i.index_name, c.column_position
+          SQL
+
+          current_index = nil
+          indexes = []
+
+          result.each do |row|
+            if current_index != row['index_name']
+              indexes << IndexDefinition.new(table_name, row['index_name'], row['uniqueness'] == "UNIQUE", [])
+              current_index = row['index_name']
+            end
+
+            indexes.last.columns << row['column_name']
+          end
+
+          indexes
+        end
+
         def structure_dump
           s = select_all("select sequence_name from user_sequences").inject("") do |structure, seq|
             structure << "create sequence #{seq.to_a.first.last};\n\n"
@@ -211,7 +246,7 @@ begin
 
         def columns(table_name, name = nil)
           select_all(%Q{
-              select column_name, data_type, data_default, data_length, data_scale
+              select column_name, data_type, data_default, data_length, data_scale, nullable
               from user_catalog cat, user_synonyms syn, all_tab_columns col
               where cat.table_name = '#{table_name.upcase}'
               and syn.synonym_name (+)= cat.table_name
@@ -221,9 +256,10 @@ begin
             OCIColumn.new(
               oci_downcase(row['column_name']), 
               row['data_default'],
-              row['data_length'], 
               row['data_type'], 
-              row['data_scale']
+              row['data_length'], 
+              row['data_scale'],
+              row['nullable'] == 'Y'
             )
           end
         end
diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb
index 2a7c2ed670..ed81645c5e 100755
--- a/activerecord/test/base_test.rb
+++ b/activerecord/test/base_test.rb
@@ -594,7 +594,9 @@ class BasicsTest < Test::Unit::TestCase
     attributes = { "last_read(1i)" => "2004", "last_read(2i)" => "6", "last_read(3i)" => "24" }
     topic = Topic.find(1)
     topic.attributes = attributes
-    assert_equal Date.new(2004, 6, 24).to_s, topic.last_read.to_s
+    # note that extra #to_date call allows test to pass for Oracle, which 
+    # treats dates/times the same
+    assert_equal Date.new(2004, 6, 24).to_s, topic.last_read.to_date.to_s
   end
 
   def test_multiparameter_attributes_on_date_with_empty_date
@@ -606,7 +608,9 @@ class BasicsTest < Test::Unit::TestCase
     attributes = { "last_read(1i)" => "2004", "last_read(2i)" => "6", "last_read(3i)" => "" }
     topic = Topic.find(1)
     topic.attributes = attributes
-    assert_equal Date.new(2004, 6, 1).to_s, topic.last_read.to_s
+    # note that extra #to_date call allows test to pass for Oracle, which 
+    # treats dates/times the same
+    assert_equal Date.new(2004, 6, 1).to_s, topic.last_read.to_date.to_s
   end
 
   def test_multiparameter_attributes_on_date_with_all_empty
@@ -647,8 +651,8 @@ class BasicsTest < Test::Unit::TestCase
 
   def test_attributes_on_dummy_time
     # Oracle does not have a TIME datatype.
-    if ActiveRecord::ConnectionAdapters.const_defined? :OracleAdapter
-      return true if ActiveRecord::Base.connection.instance_of?(ActiveRecord::ConnectionAdapters::OracleAdapter)
+    if ActiveRecord::ConnectionAdapters.const_defined? :OCIAdapter
+      return true if ActiveRecord::Base.connection.instance_of?(ActiveRecord::ConnectionAdapters::OCIAdapter)
     end
     # Sqlserver doesn't either .
     if ActiveRecord::ConnectionAdapters.const_defined? :SQLServerAdapter
diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb
index e1ec629fd5..bfaed76a56 100644
--- a/activerecord/test/finder_test.rb
+++ b/activerecord/test/finder_test.rb
@@ -47,20 +47,14 @@ class FinderTest < Test::Unit::TestCase
   end
 
   def test_find_all_with_prepared_limit_and_offset
-    if ActiveRecord::ConnectionAdapters.const_defined? :OracleAdapter
-      if ActiveRecord::Base.connection.instance_of?(ActiveRecord::ConnectionAdapters::OracleAdapter)
-        assert_raises(ArgumentError) { Entrant.find(:all, :order => 'id ASC', :limit => 2, :offset => 1) }
-      end
-    else
-      entrants = Entrant.find(:all, :order => "id ASC", :limit => 2, :offset => 1)
+    entrants = Entrant.find(:all, :order => "id ASC", :limit => 2, :offset => 1)
 
-      assert_equal(2, entrants.size)
-      assert_equal(entrants(:second).name, entrants.first.name)
+    assert_equal(2, entrants.size)
+    assert_equal(entrants(:second).name, entrants.first.name)
 
-      entrants = Entrant.find(:all, :order => "id ASC", :limit => 2, :offset => 2)
-      assert_equal(1, entrants.size)
-      assert_equal(entrants(:third).name, entrants.first.name)
-    end
+    entrants = Entrant.find(:all, :order => "id ASC", :limit => 2, :offset => 2)
+    assert_equal(1, entrants.size)
+    assert_equal(entrants(:third).name, entrants.first.name)
   end
 
   def test_find_with_entire_select_statement
diff --git a/activerecord/test/fixtures/db_definitions/create_oracle_db.bat b/activerecord/test/fixtures/db_definitions/create_oracle_db.bat
index 25c7e18856..e69de29bb2 100644
--- a/activerecord/test/fixtures/db_definitions/create_oracle_db.bat
+++ b/activerecord/test/fixtures/db_definitions/create_oracle_db.bat
@@ -1,5 +0,0 @@
-sqlplus arunit/arunit @ drop_oracle_tables
-sqlplus arunit/arunit @ oracle
-sqlplus arunit2/arunit2 @ drop_oracle_tables2
-sqlplus arunit2/arunit2 @ oracle2
-
diff --git a/activerecord/test/fixtures/db_definitions/create_oracle_db.sh b/activerecord/test/fixtures/db_definitions/create_oracle_db.sh
index 25c7e18856..e69de29bb2 100644
--- a/activerecord/test/fixtures/db_definitions/create_oracle_db.sh
+++ b/activerecord/test/fixtures/db_definitions/create_oracle_db.sh
@@ -1,5 +0,0 @@
-sqlplus arunit/arunit @ drop_oracle_tables
-sqlplus arunit/arunit @ oracle
-sqlplus arunit2/arunit2 @ drop_oracle_tables2
-sqlplus arunit2/arunit2 @ oracle2
-
diff --git a/activerecord/test/fixtures/db_definitions/oci.sql b/activerecord/test/fixtures/db_definitions/oci.sql
index 8a8dcd0681..cedd92ad53 100644
--- a/activerecord/test/fixtures/db_definitions/oci.sql
+++ b/activerecord/test/fixtures/db_definitions/oci.sql
@@ -259,3 +259,5 @@ create table keyboards (
   key_number integer not null,
   name varchar(50) default null
 );
+create sequence keyboards_seq minvalue 10000;
+
diff --git a/activerecord/test/fixtures_test.rb b/activerecord/test/fixtures_test.rb
index 888d743078..69ffcc6019 100755
--- a/activerecord/test/fixtures_test.rb
+++ b/activerecord/test/fixtures_test.rb
@@ -52,6 +52,11 @@ class FixturesTest < Test::Unit::TestCase
   end
 
   def test_inserts_with_pre_and_suffix
+    # not supported yet in OCI adapter
+    if ActiveRecord::ConnectionAdapters.const_defined? :OCIAdapter
+      return true if ActiveRecord::Base.connection.instance_of?(ActiveRecord::ConnectionAdapters::OCIAdapter)
+    end
+
     ActiveRecord::Base.connection.create_table :prefix_topics_suffix do |t|
       t.column :title, :string
       t.column :author_name, :string
diff --git a/activerecord/test/schema_dumper_test.rb b/activerecord/test/schema_dumper_test.rb
index e24724c9f8..e57429a4c8 100644
--- a/activerecord/test/schema_dumper_test.rb
+++ b/activerecord/test/schema_dumper_test.rb
@@ -4,16 +4,21 @@ require 'stringio'
 
 if ActiveRecord::Base.connection.respond_to?(:tables)
 
-  class SchemaDumperTest < Test::Unit::TestCase
-    def test_schema_dump
-      stream = StringIO.new
-      ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection, stream)
-      output = stream.string
+  unless ActiveRecord::ConnectionAdapters.const_defined?(:OCIAdapter) && \
+    ActiveRecord::Base.connection.instance_of?(ActiveRecord::ConnectionAdapters::OCIAdapter)
 
-      assert_match %r{create_table "accounts"}, output
-      assert_match %r{create_table "authors"}, output
-      assert_no_match %r{create_table "schema_info"}, output
+    class SchemaDumperTest < Test::Unit::TestCase
+      def test_schema_dump
+        stream = StringIO.new
+        ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection, stream)
+        output = stream.string
+
+        assert_match %r{create_table "accounts"}, output
+        assert_match %r{create_table "authors"}, output
+        assert_no_match %r{create_table "schema_info"}, output
+      end
     end
+
   end
 
 end