Add change_column, change_column_null case

README.md

@@ -27,6 +27,7 @@ It checks for common things like:
 * Mixing data changes with index or schema migrations
 * Performing data or schema migrations with the DDL transaction disabled
 * Using `each` instead of `find_each` to loop thru `ActiveRecord` objects
+* Check slow database, when change column options null is false with default value 
 
 These exceptions display clear instructions of how to perform the same operation the "zero downtime way".
 

lib/zero_downtime_migrations.rb

@@ -8,6 +8,8 @@
 require_relative "zero_downtime_migrations/validation"
 require_relative "zero_downtime_migrations/validation/add_column"
 require_relative "zero_downtime_migrations/validation/add_index"
+require_relative "zero_downtime_migrations/validation/change_column"
+require_relative "zero_downtime_migrations/validation/change_column_null"
 require_relative "zero_downtime_migrations/validation/ddl_migration"
 require_relative "zero_downtime_migrations/validation/find_each"
 require_relative "zero_downtime_migrations/validation/mixed_migration"

lib/zero_downtime_migrations/validation/change_column.rb

@@ -0,0 +1,55 @@
+module ZeroDowntimeMigrations
+  class Validation
+    class ChangeColumn < Validation
+      def validate!
+        return unless options.key?(:null)
+        return if options[:null] || options[:default].nil?
+        error!(message)
+      end
+
+      private
+
+      def message
+        <<-MESSAGE.strip_heredoc
+          Changing a column with a false null and default value is unsafe!
+
+          This action lead to slow database, If table size is big.
+
+          If you're 100% positive that this migration is safe, then wrap the
+          call to `change_column` in a `safety_assured` block.
+
+            class Change#{column_title}To#{table_title} < ActiveRecord::Migration
+              def change
+                safety_assured { change_column :#{table}, :#{column}, :#{column_type}, #{column_options} }
+              end
+            end
+
+        MESSAGE
+      end
+
+      def column
+        args[1]
+      end
+
+      def column_options
+        options.map { |k, v| "#{k}: #{v.inspect}" }.join ', '
+      end
+
+      def column_title
+        column.to_s.camelize
+      end
+
+      def column_type
+        args[2]
+      end
+
+      def table
+        args[0]
+      end
+
+      def table_title
+        table.to_s.camelize
+      end
+    end
+  end
+end

lib/zero_downtime_migrations/validation/change_column_null.rb

@@ -0,0 +1,64 @@
+module ZeroDowntimeMigrations
+  class Validation
+    class ChangeColumnNull < Validation
+      def validate!
+        null = args[2]
+        default = args[3]
+        return if null || default.nil?
+        error!(message)
+      end
+
+      private
+
+      def message
+        <<-MESSAGE.strip_heredoc
+          Changing a column with a false null and default value is unsafe!
+
+          This action lead to slow database, If table size is big. 
+
+          If you're 100% positive that this migration is safe, then wrap the
+          call to `change_column_null` in a `safety_assured` block.
+
+            class Change#{column_title}To#{table_title} < ActiveRecord::Migration
+              def change
+                safety_assured { change_column_null :#{table}, :#{column}, #{column_null}, default: #{column_default} }
+              end
+            end
+
+          or `change_column`, if you use `change_column`
+
+            class Change#{column_title}To#{table_title} < ActiveRecord::Migration
+              def change
+                safety_assured { change_column :#{table}, :#{column}, :column_type, #{column_null}, default: #{column_default} }
+              end
+            end
+
+        MESSAGE
+      end
+
+      def column
+        args[1]
+      end
+
+      def column_default
+        args[3].inspect
+      end
+
+      def column_null
+        args[2].inspect
+      end
+
+      def column_title
+        column.to_s.camelize
+      end
+
+      def table
+        args[0]
+      end
+
+      def table_title
+        table.to_s.camelize
+      end
+    end
+  end
+end

spec/zero_downtime_migrations/validation/change_column_null_spec.rb

@@ -0,0 +1,143 @@
+RSpec.describe ZeroDowntimeMigrations::Validation::ChangeColumnNull do
+  let(:error) { ZeroDowntimeMigrations::UnsafeMigrationError }
+
+  context "with a true null" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.0]) do
+        def change
+          change_column_null :users, :email, true
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.not_to raise_error(error)
+    end
+  end
+
+  context "with a false null" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.0]) do
+        def change
+          change_column_null :users, :email, false
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.not_to raise_error(error)
+    end
+  end
+
+  context "with a true null has default" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.0]) do
+        def change
+          change_column_null :users, :email, true, ''
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.not_to raise_error(error)
+    end
+  end
+
+  context "with a false null has default" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.0]) do
+        def change
+          change_column_null :users, :email, false, ''
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.to raise_error(error)
+    end
+  end
+
+  context "without a null" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.0]) do
+        def change
+          change_column :users, :email, :text
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.not_to raise_error(error)
+    end
+  end
+
+  context "with a default" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.0]) do
+        def change
+          change_column :users, :email, :text, default: ''
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.not_to raise_error(error)
+    end
+  end
+
+  context "with a true null" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.0]) do
+        def change
+          change_column :users, :email, :text, null: true
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.not_to raise_error(error)
+    end
+  end
+
+  context "with a true null and has default" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.0]) do
+        def change
+          change_column :users, :email, :text, null: true, default: ''
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.not_to raise_error(error)
+    end
+  end
+
+  context "with a false null" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.0]) do
+        def change
+          change_column :users, :email, :text, null: false
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.not_to raise_error(error)
+    end
+  end
+
+  context "with a false null and has default" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.0]) do
+        def change
+          change_column :users, :email, :text, null: false, default: ''
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.to raise_error(error)
+    end
+  end
+end

spec/zero_downtime_migrations/validation/change_column_spec.rb

@@ -0,0 +1,87 @@
+RSpec.describe ZeroDowntimeMigrations::Validation::ChangeColumnNull do
+  let(:error) { ZeroDowntimeMigrations::UnsafeMigrationError }
+
+  context "without a null" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.2]) do
+        def change
+          change_column :users, :email, :text
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.not_to raise_error(error)
+    end
+  end
+
+  context "with a default" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.2]) do
+        def change
+          change_column :users, :email, :text, default: ''
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.not_to raise_error(error)
+    end
+  end
+
+  context "with a true null" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.2]) do
+        def change
+          change_column :users, :email, :text, null: true
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.not_to raise_error(error)
+    end
+  end
+
+  context "with a true null and has default" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.2]) do
+        def change
+          change_column :users, :email, :text, null: true, default: ''
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.not_to raise_error(error)
+    end
+  end
+
+  context "with a false null" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.2]) do
+        def change
+          change_column :users, :email, :text, null: false
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.not_to raise_error(error)
+    end
+  end
+
+  context "with a false null and has default" do
+    let(:migration) do
+      Class.new(ActiveRecord::Migration[5.2]) do
+        def change
+          change_column :users, :email, :text, null: false, default: ''
+        end
+      end
+    end
+
+    it "raises an unsafe migration error" do
+      expect { migration.migrate(:up) }.to raise_error(error)
+    end
+  end
+end