Merge [5937] from trunk. References #6956.

git-svn-id: http://svn-commit.rubyonrails.org/rails/branches/1-2-pre-release@5938 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
rails · Jan 15, 2007 · aa30fc7 · aa30fc7
1 parent 5f11417
commit aa30fc7
Show file tree

Hide file tree

Showing 11 changed files with 51 additions and 31 deletions.
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* change_column accepts :default => nil.  #6956 [dcmanges, Jeremy Kemper]
+
 * MySQL, PostgreSQL: change_column_default quotes the default value and doesn't lose column type information.  #3987, #6664 [Jonathan Viney, manfred, altano@bigfoot.com]
 
 * Oracle: create_table takes a :sequence_name option to override the 'tablename_seq' default.  #7000 [Michael Schoen]

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -253,9 +253,7 @@ def [](name)
       #   Requests a maximum column length (<tt>:string</tt>, <tt>:text</tt>,
       #   <tt>:binary</tt> or <tt>:integer</tt> columns only)
       # * <tt>:default</tt>:
-      #   The column's default value.  You cannot explicitely set the default
-      #   value to +NULL+.  Simply leave off this option if you want a +NULL+
-      #   default value.
+      #   The column's default value. Use nil for NULL.
       # * <tt>:null</tt>:
       #   Allows or disallows +NULL+ values in the column.  This option could
       #   have been named <tt>:null_allowed</tt>.
@@ -271,11 +269,11 @@ def [](name)
       #   <tt>:precision</tt>.
       # * MySQL: <tt>:precision</tt> [1..63], <tt>:scale</tt> [0..30]. 
       #   Default is (10,0).
-      # * PostGres?: <tt>:precision</tt> [1..infinity], 
+      # * PostgreSQL: <tt>:precision</tt> [1..infinity], 
       #   <tt>:scale</tt> [0..infinity]. No default.
-      # * Sqlite2: Any <tt>:precision</tt> and <tt>:scale</tt> may be used. 
+      # * SQLite2: Any <tt>:precision</tt> and <tt>:scale</tt> may be used. 
       #   Internal storage as strings. No default.
-      # * Sqlite3: No restrictions on <tt>:precision</tt> and <tt>:scale</tt>,
+      # * SQLite3: No restrictions on <tt>:precision</tt> and <tt>:scale</tt>,
       #   but the maximum supported <tt>:precision</tt> is 16. No default.
       # * Oracle: <tt>:precision</tt> [1..38], <tt>:scale</tt> [-84..127]. 
       #   Default is (38,0).

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -273,10 +273,10 @@ def type_to_sql(type, limit = nil, precision = nil, scale = nil) #:nodoc:
           column_type_sql << "(#{limit})" if limit
           column_type_sql
         end
-      end            
-    
+      end
+
       def add_column_options!(sql, options) #:nodoc:
-        sql << " DEFAULT #{quote(options[:default], options[:column])}" unless options[:default].nil?
+        sql << " DEFAULT #{quote(options[:default], options[:column])}" if options_include_default?(options)
         sql << " NOT NULL" if options[:null] == false
       end
 
@@ -293,6 +293,11 @@ def distinct(columns, order_by)
       def add_order_by_for_association_limiting!(sql, options)
         sql << "ORDER BY #{options[:order]}"
       end
+
+      protected
+        def options_include_default?(options)
+          options.include?(:default) && !(options[:null] == false && options[:default].nil?)
+        end
     end
   end
 end
diff --git a/activerecord/lib/active_record/connection_adapters/firebird_adapter.rb b/activerecord/lib/active_record/connection_adapters/firebird_adapter.rb
@@ -505,8 +505,8 @@ def add_column(table_name, column_name, type, options = {}) # :nodoc:
 
       def change_column(table_name, column_name, type, options = {}) # :nodoc:
         change_column_type(table_name, column_name, type, options)
-        change_column_position(table_name, column_name, options[:position]) if options[:position]
-        change_column_default(table_name, column_name, options[:default]) if options.has_key?(:default)
+        change_column_position(table_name, column_name, options[:position]) if options.include?(:position)
+        change_column_default(table_name, column_name, options[:default]) if options_include_default?(options)
       end
 
       def change_column_default(table_name, column_name, default) # :nodoc:

diff --git a/activerecord/lib/active_record/connection_adapters/frontbase_adapter.rb b/activerecord/lib/active_record/connection_adapters/frontbase_adapter.rb
@@ -795,12 +795,12 @@ def add_column(table_name, column_name, type, options = {})
 
       def add_column_options!(sql, options) #:nodoc:
         default_value = quote(options[:default], options[:column])
-        if options[:default]
+        if options_include_default?(options)
           if options[:type] == :boolean
             default_value = options[:default] == 0 ? quoted_false : quoted_true
           end
+          sql << " DEFAULT #{default_value}"
         end
-        sql << " DEFAULT #{default_value}" unless options[:default].nil?
         sql << " NOT NULL" if options[:null] == false
       end
 
@@ -828,17 +828,15 @@ def change_column(table_name, column_name, type, options = {}) #:nodoc:
         execute(change_column_sql)
         change_column_sql = %( ALTER TABLE "#{table_name}" ALTER COLUMN "#{column_name}" )
 
-        default_value = quote(options[:default], options[:column])
-        if options[:default]
+        if options_include_default?(options)
+          default_value = quote(options[:default], options[:column])
           if type == :boolean
             default_value = options[:default] == 0 ? quoted_false : quoted_true
           end
-        end
-
-        if default_value != "NULL"
           change_column_sql << " SET DEFAULT #{default_value}"
-          execute(change_column_sql)
         end
+
+        execute(change_column_sql)
 
 #         change_column_sql = "ALTER TABLE #{table_name} CHANGE #{column_name} #{column_name} #{type_to_sql(type, options[:limit])}"
 #         add_column_options!(change_column_sql, options)

diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -364,10 +364,10 @@ def change_column_default(table_name, column_name, default) #:nodoc:
       end
 
       def change_column(table_name, column_name, type, options = {}) #:nodoc:
-        if options[:default].nil?
+        unless options_include_default?(options)
           options[:default] = select_one("SHOW COLUMNS FROM #{table_name} LIKE '#{column_name}'")["Default"]
         end
-        
+
         change_column_sql = "ALTER TABLE #{table_name} CHANGE #{column_name} #{column_name} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
         add_column_options!(change_column_sql, options)
         execute(change_column_sql)

diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -321,10 +321,10 @@ def add_column(table_name, column_name, type, options = {})
         execute("ALTER TABLE #{table_name} ADD COLUMN #{column_name} #{type_to_sql(type, options[:limit])}")
 
         # Set optional default. If not null, update nulls to the new default.
-        unless default.nil?
+        if options_include_default?(options)
           change_column_default(table_name, column_name, default)
           if notnull
-            execute("UPDATE #{table_name} SET #{column_name}='#{default}' WHERE #{column_name} IS NULL")
+            execute("UPDATE #{table_name} SET #{column_name}=#{quote(default, options[:column])} WHERE #{column_name} IS NULL")
           end
         end
 
@@ -345,7 +345,10 @@ def change_column(table_name, column_name, type, options = {}) #:nodoc:
           rename_column(table_name, "#{column_name}_ar_tmp", column_name)
           commit_db_transaction
         end
-        change_column_default(table_name, column_name, options[:default]) unless options[:default].nil?
+
+        if options_include_default?(options)
+          change_column_default(table_name, column_name, options[:default])
+        end
       end
 
       def change_column_default(table_name, column_name, default) #:nodoc:

diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -254,10 +254,11 @@ def change_column_default(table_name, column_name, default) #:nodoc:
 
       def change_column(table_name, column_name, type, options = {}) #:nodoc:
         alter_table(table_name) do |definition|
+          include_default = options_include_default?(options)
           definition[column_name].instance_eval do
             self.type    = type
-            self.limit   = options[:limit] if options[:limit]
-            self.default = options[:default] unless options[:default].nil?
+            self.limit   = options[:limit] if options.include?(:limit)
+            self.default = options[:default] if include_default
           end
         end
       end

diff --git a/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb
@@ -459,9 +459,9 @@ def rename_column(table, column, new_column_name)
 
       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])}"]
-        unless options[:default].nil?
+        if options_include_default?(options)
           remove_default_constraint(table_name, column_name)
-          sql_commands << "ALTER TABLE #{table_name} ADD CONSTRAINT DF_#{table_name}_#{column_name} DEFAULT #{quote(options[:default])} FOR #{column_name}"
+          sql_commands << "ALTER TABLE #{table_name} ADD CONSTRAINT DF_#{table_name}_#{column_name} DEFAULT #{quote(options[:default], options[:column])} FOR #{column_name}"
         end
         sql_commands.each {|c|
           execute(c)

diff --git a/activerecord/lib/active_record/connection_adapters/sybase_adapter.rb b/activerecord/lib/active_record/connection_adapters/sybase_adapter.rb
@@ -381,7 +381,7 @@ def remove_index(table_name, options = {})
       end
 
       def add_column_options!(sql, options) #:nodoc:
-        sql << " DEFAULT #{quote(options[:default], options[:column])}" unless options[:default].nil?
+        sql << " DEFAULT #{quote(options[:default], options[:column])}" if options_include_default?(options)
 
         if check_null_for_column?(options[:column], sql)
           sql << (options[:null] == false ? " NOT NULL" : " NULL")

diff --git a/activerecord/test/migration_test.rb b/activerecord/test/migration_test.rb
@@ -94,8 +94,10 @@ def test_create_table_adds_id
     end
 
     def test_create_table_with_not_null_column
-      Person.connection.create_table :testings do |t|
-        t.column :foo, :string, :null => false
+      assert_nothing_raised do
+        Person.connection.create_table :testings do |t|
+          t.column :foo, :string, :null => false
+        end
       end
 
       assert_raises(ActiveRecord::StatementInvalid) do
@@ -424,6 +426,17 @@ def test_change_column
       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 }
     end
+
+    def test_change_column_with_nil_default
+      Person.connection.add_column "people", "contributor", :boolean, :default => true
+      Person.reset_column_information
+      assert Person.new.contributor?
+
+      assert_nothing_raised { Person.connection.change_column "people", "contributor", :boolean, :default => nil }
+      Person.reset_column_information
+      assert !Person.new.contributor?
+      assert_nil Person.new.contributor
+    end
 
     def test_change_column_with_new_default
       Person.connection.add_column "people", "administrator", :boolean, :default => true