Fix change_column_null to re-declare datetime2 columns correctly

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## Unreleased
 
+#### Fixed
+
+- [#1418](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1418) Fix `change_column_null` to re-declare datetime2 columns correctly.
+
 #### Changed
 
 - [#1405](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1405) Fix `insert_all`/`upsert_all` for single character, temporary, non-ASCII and three part table names, and stop an aliased target being included in the `MERGE` table name.

lib/active_record/connection_adapters/sqlserver/schema_statements.rb

@@ -377,7 +377,12 @@ def change_column_null(table_name, column_name, null, default = nil)
           if !null.nil? && null == false && !default.nil?
             execute("UPDATE #{table_id} SET #{column_id}=#{quote(default)} WHERE #{column_id} IS NULL")
           end
-          sql = "ALTER TABLE #{table_id} ALTER COLUMN #{column_id} #{type_to_sql column.type, limit: column.limit, precision: column.precision, scale: column.scale}"
+
+          # If datetime column has precision then it should be datetime2 column.
+          column_type = column.type
+          column_type = :datetime2 if column.type.to_s == "datetime" && !column.precision.nil?
+
+          sql = "ALTER TABLE #{table_id} ALTER COLUMN #{column_id} #{type_to_sql column_type, limit: column.limit, precision: column.precision, scale: column.scale}"
           sql += " NOT NULL" if !null.nil? && null == false
 
           execute sql

test/cases/change_column_null_test_sqlserver.rb

@@ -2,29 +2,31 @@
 
 require "cases/helper_sqlserver"
 require "migrations/create_clients_and_change_column_null"
+require "cases/migration/helper"
 
 class ChangeColumnNullTestSqlServer < ActiveRecord::TestCase
-  before do
-    @old_verbose = ActiveRecord::Migration.verbose
-    ActiveRecord::Migration.verbose = false
-    CreateClientsAndChangeColumnNull.new.up
-  end
+  include ::ActiveRecord::Migration::TestHelper
 
-  after do
-    CreateClientsAndChangeColumnNull.new.down
-    ActiveRecord::Migration.verbose = @old_verbose
+  def find_column(table, column_name)
+    connection.columns(table).find { |column| column.name == column_name.to_s }
   end
 
-  def find_column(table, name)
-    table.find { |column| column.name == name }
-  end
+  describe "using migration" do
+    before do
+      @old_verbose = ActiveRecord::Migration.verbose
+      ActiveRecord::Migration.verbose = false
+      CreateClientsAndChangeColumnNull.new.up
+    end
+
+    after do
+      CreateClientsAndChangeColumnNull.new.down
+      ActiveRecord::Migration.verbose = @old_verbose
+    end
 
-  let(:clients_table) { connection.columns("clients") }
-  let(:name_column) { find_column(clients_table, "name") }
-  let(:code_column) { find_column(clients_table, "code") }
-  let(:value_column) { find_column(clients_table, "value") }
+    let(:name_column) { find_column("clients", "name") }
+    let(:code_column) { find_column("clients", "code") }
+    let(:value_column) { find_column("clients", "value") }
 
-  describe "#change_column_null" do
     it "does not change the column limit" do
       _(name_column.limit).must_equal 15
     end
@@ -41,4 +43,32 @@ def find_column(table, name)
       _(value_column.scale).must_equal 8
     end
   end
+
+  it "datetime2 column" do
+    add_column :test_models, :expiry_date, :datetime, precision: 6, null: true
+
+    _(find_column(:test_models, :expiry_date).sql_type).must_equal "datetime2(6)"
+    _(find_column(:test_models, :expiry_date).null).must_equal true
+
+    connection.change_column_null(:test_models, :expiry_date, false)
+
+    _(find_column(:test_models, :expiry_date).sql_type).must_equal "datetime2(6)"
+    _(find_column(:test_models, :expiry_date).null).must_equal false
+  ensure
+    remove_column("test_models", "expiry_date")
+  end
+
+  it "datetime column" do
+    add_column :test_models, :expiry_date, :datetime, precision: nil, null: true
+
+    _(find_column(:test_models, :expiry_date).sql_type).must_equal "datetime"
+    _(find_column(:test_models, :expiry_date).null).must_equal true
+
+    connection.change_column_null(:test_models, :expiry_date, false)
+
+    _(find_column(:test_models, :expiry_date).sql_type).must_equal "datetime"
+    _(find_column(:test_models, :expiry_date).null).must_equal false
+  ensure
+    remove_column("test_models", "expiry_date")
+  end
 end