Allow change_column to set NOT NULL in the PostgreSQL adaptor. Closes #3904 [tarmo]

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

3 files changed, 29 insertions, 18 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 5e3280dda7..c5b908b0e4 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Allow change_column to set NOT NULL in the PostgreSQL adapter [tarmo]
+
 * Fix that ActiveRecord would create attribute methods and override custom attribute getters if the method is also defined in Kernel.methods. [Rick]
 
 * Don't call attr_readonly on polymorphic belongs_to associations, in case it matches the name of some other non-ActiveRecord class/module.  [Rick]
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 786ad1c864..331f331de7 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -579,22 +579,11 @@ module ActiveRecord
         default = options[:default]
         notnull = options[:null] == false
 
-        quoted_column_name = quote_column_name(column_name)
-
         # Add the column.
-        execute("ALTER TABLE #{table_name} ADD COLUMN #{quoted_column_name} #{type_to_sql(type, options[:limit])}")
+        execute("ALTER TABLE #{table_name} ADD COLUMN #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit])}")
 
-        # Set optional default. If not null, update nulls to the new default.
-        if options_include_default?(options)
-          change_column_default(table_name, column_name, default)
-          if notnull
-            execute("UPDATE #{table_name} SET #{quoted_column_name}=#{quote(default, options[:column])} WHERE #{quoted_column_name} IS NULL")
-          end
-        end
-
-        if notnull
-          execute("ALTER TABLE #{table_name} ALTER #{quoted_column_name} SET NOT NULL")
-        end
+        change_column_default(table_name, column_name, default) if options_include_default?(options)
+        change_column_null(table_name, column_name, false, default) if notnull
       end
 
       # Changes the column of a table.
@@ -612,9 +601,8 @@ module ActiveRecord
           commit_db_transaction
         end
 
-        if options_include_default?(options)
-          change_column_default(table_name, column_name, options[:default])
-        end
+        change_column_default(table_name, column_name, options[:default]) if options_include_default?(options)
+        change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null)
       end
 
       # Changes the default value of a table column.
@@ -622,6 +610,13 @@ module ActiveRecord
         execute "ALTER TABLE #{table_name} ALTER COLUMN #{quote_column_name(column_name)} SET DEFAULT #{quote(default)}"
       end
 
+      def change_column_null(table_name, column_name, null, default = nil)
+        unless null || default.nil?
+          execute("UPDATE #{table_name} SET #{quote_column_name(column_name)}=#{quote(default)} WHERE #{quote_column_name(column_name)} IS NULL")
+        end
+        execute("ALTER TABLE #{table_name} ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL")
+      end
+
       # Renames a column in a table.
       def rename_column(table_name, column_name, new_column_name)
         execute "ALTER TABLE #{table_name} RENAME COLUMN #{quote_column_name(column_name)} TO #{quote_column_name(new_column_name)}"
diff --git a/activerecord/test/migration_test.rb b/activerecord/test/migration_test.rb
index 9ac9bb10fd..a1eda28aa4 100644
--- a/activerecord/test/migration_test.rb
+++ b/activerecord/test/migration_test.rb
@@ -42,7 +42,7 @@ if ActiveRecord::Base.connection.supports_migrations?
       Reminder.reset_column_information
 
       %w(last_name key bio age height wealth birthday favorite_day
-         moment_of_truth male administrator).each do |column|
+         moment_of_truth male administrator funny).each do |column|
         Person.connection.remove_column('people', column) rescue nil
       end
       Person.connection.remove_column("people", "first_name") rescue nil
@@ -450,6 +450,20 @@ if ActiveRecord::Base.connection.supports_migrations?
         ActiveRecord::Base.connection.drop_table :octopi rescue nil
       end
     end
+    
+    unless current_adapter?(:SQLiteAdapter)
+    def test_change_column_nullability
+      Person.connection.add_column "people", "funny", :boolean
+      Person.reset_column_information
+      assert Person.columns_hash["funny"].null, "Column 'funny' must initially allow nulls"
+      Person.connection.change_column "people", "funny", :boolean, :null => false, :default => true
+      Person.reset_column_information
+      assert !Person.columns_hash["funny"].null, "Column 'funny' must *not* allow nulls at this point"
+      Person.connection.change_column "people", "funny", :boolean, :null => true
+      Person.reset_column_information
+      assert Person.columns_hash["funny"].null, "Column 'funny' must allow nulls again at this point"
+    end
+    end
 
     def test_rename_table_with_an_index
       begin