Merge pull request rails#49090 from skipkayhil/hm-change-column-precision-6

Fix change_column not setting precision for sqlite

Author: rafaelfranca

activerecord/CHANGELOG.md

@@ -1,3 +1,8 @@
+*   Fix `change_column` not setting `precision: 6` on `datetime` columns when
+    using 7.0+ Migrations and SQLite.
+
+    *Hartley McGuire*
+
 *   Support composite identifiers in `to_key`
 
     `to_key` avoids wrapping `#id` value into an `Array` if `#id` already an array

activerecord/lib/active_record/connection_adapters/sqlite3/schema_definitions.rb

@@ -5,6 +5,12 @@ module ConnectionAdapters
     module SQLite3
       # = Active Record SQLite3 Adapter \Table Definition
       class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition
+        def change_column(column_name, type, **options)
+          name = column_name.to_s
+          @columns_hash[name] = nil
+          column(name, type, **options)
+        end
+
         def references(*args, **options)
           super(*args, type: :integer, **options)
         end

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -328,10 +328,7 @@ def change_column_null(table_name, column_name, null, default = nil) # :nodoc:
 
       def change_column(table_name, column_name, type, **options) # :nodoc:
         alter_table(table_name) do |definition|
-          definition[column_name].instance_eval do
-            self.type = aliased_types(type.to_s, type)
-            self.options.merge!(options)
-          end
+          definition.change_column(column_name, type, **options)
         end
       end
 

activerecord/test/cases/migration/compatibility_test.rb

@@ -336,22 +336,6 @@ def migrate(x)
         connection.drop_table :more_testings rescue nil
       end
 
-      def test_datetime_doesnt_set_precision_on_create_table
-        migration = Class.new(ActiveRecord::Migration[6.1]) {
-          def migrate(x)
-            create_table :more_testings do |t|
-              t.datetime :published_at
-            end
-          end
-        }.new
-
-        ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate
-
-        assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
-      ensure
-        connection.drop_table :more_testings rescue nil
-      end
-
       def test_datetime_doesnt_set_precision_on_add_column_5_0
         migration = Class.new(ActiveRecord::Migration[5.0]) {
           def migrate(x)
@@ -376,28 +360,6 @@ def migrate(x)
         assert connection.column_exists?(:testings, :published_at, **precision_implicit_default)
       end
 
-      def test_datetime_doesnt_set_precision_on_change_column_6_1
-        create_migration = Class.new(ActiveRecord::Migration[6.1]) {
-          def migrate(x)
-            create_table :more_testings do |t|
-              t.date :published_at
-            end
-          end
-        }.new(nil, 0)
-
-        change_migration = Class.new(ActiveRecord::Migration[6.1]) {
-          def migrate(x)
-            change_column :more_testings, :published_at, :datetime
-          end
-        }.new(nil, 1)
-
-        ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
-
-        assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
-      ensure
-        connection.drop_table :more_testings rescue nil
-      end
-
       def test_change_table_allows_if_exists_option_on_7_0
         migration = Class.new(ActiveRecord::Migration[7.0]) {
           def migrate(x)
@@ -723,6 +685,44 @@ def migrate(x)
     connection.drop_table :more_testings rescue nil
   end
 
+  def test_datetime_doesnt_set_precision_on_create_table
+    migration = Class.new(migration_class) {
+      def migrate(x)
+        create_table :more_testings do |t|
+          t.datetime :published_at
+        end
+      end
+    }.new
+
+    ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate
+
+    assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
+  ensure
+    connection.drop_table :more_testings rescue nil
+  end
+
+  def test_datetime_doesnt_set_precision_on_change_column
+    create_migration = Class.new(migration_class) {
+      def migrate(x)
+        create_table :more_testings do |t|
+          t.date :published_at
+        end
+      end
+    }.new(nil, 0)
+
+    change_migration = Class.new(migration_class) {
+      def migrate(x)
+        change_column :more_testings, :published_at, :datetime
+      end
+    }.new(nil, 1)
+
+    ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
+
+    assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
+  ensure
+    connection.drop_table :more_testings rescue nil
+  end
+
   private
     def precision_implicit_default
       if current_adapter?(:Mysql2Adapter, :TrilogyAdapter)
@@ -757,6 +757,46 @@ def migrate(x)
   ensure
     connection.drop_table :more_testings rescue nil
   end
+
+  def test_datetime_sets_precision_6_on_create_table
+    migration = Class.new(migration_class) {
+      def migrate(x)
+        create_table :more_testings do |t|
+          t.datetime :published_at
+        end
+      end
+    }.new
+
+    ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate
+
+    assert connection.column_exists?(:more_testings, :published_at, precision: 6)
+  ensure
+    connection.drop_table :more_testings rescue nil
+  end
+
+  def test_datetime_sets_precision_6_on_change_column
+    create_migration = Class.new(migration_class) {
+      def migrate(x)
+        create_table :more_testings do |t|
+          t.date :published_at
+        end
+      end
+    }.new(nil, 0)
+
+    $global = true
+
+    change_migration = Class.new(migration_class) {
+      def migrate(x)
+        change_column :more_testings, :published_at, :datetime
+      end
+    }.new(nil, 1)
+
+    ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
+
+    assert connection.column_exists?(:more_testings, :published_at, precision: 6)
+  ensure
+    connection.drop_table :more_testings rescue nil
+  end
 end
 
 class BaseCompatibilityTest < ActiveRecord::TestCase