Merge pull request #34970 from kamipo/timestamps_with_precision_by_default

Make `t.timestamps` with precision by default.

14 files changed, 206 insertions, 75 deletions

diff --git a/actionmailbox/test/dummy/db/migrate/20180208205311_create_action_mailbox_tables.rb b/actionmailbox/test/dummy/db/migrate/20180208205311_create_action_mailbox_tables.rb
index 89ab66c1a9..2bf4335808 100644
--- a/actionmailbox/test/dummy/db/migrate/20180208205311_create_action_mailbox_tables.rb
+++ b/actionmailbox/test/dummy/db/migrate/20180208205311_create_action_mailbox_tables.rb
@@ -5,11 +5,7 @@ class CreateActionMailboxTables < ActiveRecord::Migration[6.0]
       t.string  :message_id, null: false
       t.string  :message_checksum, null: false
 
-      if supports_datetime_with_precision?
-        t.timestamps precision: 6
-      else
-        t.timestamps
-      end
+      t.timestamps
 
       t.index [ :message_id, :message_checksum ], name: "index_action_mailbox_inbound_emails_uniqueness", unique: true
     end
diff --git a/actiontext/test/dummy/db/migrate/2018052816_create_action_text_tables.rb b/actiontext/test/dummy/db/migrate/2018052816_create_action_text_tables.rb
index 74c7a0ecb9..6e7177620f 100644
--- a/actiontext/test/dummy/db/migrate/2018052816_create_action_text_tables.rb
+++ b/actiontext/test/dummy/db/migrate/2018052816_create_action_text_tables.rb
@@ -5,8 +5,7 @@ class CreateActionTextTables < ActiveRecord::Migration[6.0]
       t.text       :body, limit: 16777215
       t.references :record, null: false, polymorphic: true, index: false
 
-      t.datetime :created_at, null: false
-      t.datetime :updated_at, null: false
+      t.timestamps
 
       t.index [ :record_type, :record_id, :name ], name: "index_action_text_rich_texts_uniqueness", unique: true
     end
diff --git a/actiontext/test/dummy/db/schema.rb b/actiontext/test/dummy/db/schema.rb
index 7f8f4dff4e..71080a1c69 100644
--- a/actiontext/test/dummy/db/schema.rb
+++ b/actiontext/test/dummy/db/schema.rb
@@ -17,8 +17,8 @@ ActiveRecord::Schema.define(version: 2018_10_03_185713) do
     t.text "body", limit: 16777215
     t.string "record_type", null: false
     t.integer "record_id", null: false
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at", precision: 6, null: false
+    t.datetime "updated_at", precision: 6, null: false
     t.index ["record_type", "record_id", "name"], name: "index_action_text_rich_texts_uniqueness", unique: true
   end
 
@@ -45,14 +45,14 @@ ActiveRecord::Schema.define(version: 2018_10_03_185713) do
 
   create_table "messages", force: :cascade do |t|
     t.string "subject"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at", precision: 6, null: false
+    t.datetime "updated_at", precision: 6, null: false
   end
 
   create_table "people", force: :cascade do |t|
     t.string "name"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at", precision: 6, null: false
+    t.datetime "updated_at", precision: 6, null: false
   end
 
 end
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 2a0cd81be5..167ade30e3 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Make `t.timestamps` with precision by default.
+
+    *Ryuta Kamizono*
+
+
 ## Rails 6.0.0.beta1 (January 18, 2019) ##
 
 *   Remove deprecated `#set_state` from the transaction object.
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
index db489143af..28d37881b6 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -263,6 +263,7 @@ module ActiveRecord
       deprecate :indexes=
 
       def initialize(
+        conn,
         name,
         temporary: false,
         if_not_exists: false,
@@ -271,6 +272,7 @@ module ActiveRecord
         comment: nil,
         **
       )
+        @conn = conn
         @columns_hash = {}
         @indexes = []
         @foreign_keys = []
@@ -410,6 +412,10 @@ module ActiveRecord
       def timestamps(**options)
         options[:null] = false if options[:null].nil?
 
+        if !options.key?(:precision) && @conn.supports_datetime_with_precision?
+          options[:precision] = 6
+        end
+
         column(:created_at, :datetime, options)
         column(:updated_at, :datetime, options)
       end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index 1afbf79836..d88e75d692 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -1129,6 +1129,10 @@ module ActiveRecord
       def add_timestamps(table_name, options = {})
         options[:null] = false if options[:null].nil?
 
+        if !options.key?(:precision) && supports_datetime_with_precision?
+          options[:precision] = 6
+        end
+
         add_column table_name, :created_at, :datetime, options
         add_column table_name, :updated_at, :datetime, options
       end
@@ -1290,7 +1294,7 @@ module ActiveRecord
         end
 
         def create_table_definition(*args)
-          TableDefinition.new(*args)
+          TableDefinition.new(self, *args)
         end
 
         def create_alter_table(name)
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index ce2a1737f9..415a0576c4 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -711,6 +711,10 @@ module ActiveRecord
         def add_timestamps_for_alter(table_name, options = {})
           options[:null] = false if options[:null].nil?
 
+          if !options.key?(:precision) && supports_datetime_with_precision?
+            options[:precision] = 6
+          end
+
           [add_column_for_alter(table_name, :created_at, :datetime, options), add_column_for_alter(table_name, :updated_at, :datetime, options)]
         end
 
diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
index 47b5c4b9ec..e9484a08de 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
@@ -127,7 +127,7 @@ module ActiveRecord
           end
 
           def create_table_definition(*args)
-            MySQL::TableDefinition.new(*args)
+            MySQL::TableDefinition.new(self, *args)
           end
 
           def new_column_from_field(table_name, field)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
index 9c44cf46aa..946436f7f9 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -637,7 +637,7 @@ module ActiveRecord
           end
 
           def create_table_definition(*args)
-            PostgreSQL::TableDefinition.new(*args)
+            PostgreSQL::TableDefinition.new(self, *args)
           end
 
           def create_alter_table(name)
@@ -718,6 +718,10 @@ module ActiveRecord
           def add_timestamps_for_alter(table_name, options = {})
             options[:null] = false if options[:null].nil?
 
+            if !options.key?(:precision) && supports_datetime_with_precision?
+              options[:precision] = 6
+            end
+
             [add_column_for_alter(table_name, :created_at, :datetime, options), add_column_for_alter(table_name, :updated_at, :datetime, options)]
           end
 
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb
index 8650c07bab..2394982a7d 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb
@@ -62,7 +62,7 @@ module ActiveRecord
           end
 
           def create_table_definition(*args)
-            SQLite3::TableDefinition.new(*args)
+            SQLite3::TableDefinition.new(self, *args)
           end
 
           def new_column_from_field(table_name, field)
diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb
index 608182e363..94906a2943 100644
--- a/activerecord/lib/active_record/migration/compatibility.rb
+++ b/activerecord/lib/active_record/migration/compatibility.rb
@@ -16,13 +16,55 @@ module ActiveRecord
       V6_0 = Current
 
       class V5_2 < V6_0
+        module TableDefinition
+          def timestamps(**options)
+            options[:precision] ||= nil
+            super
+          end
+        end
+
         module CommandRecorder
           def invert_transaction(args, &block)
             [:transaction, args, block]
           end
         end
 
+        def create_table(table_name, **options)
+          if block_given?
+            super { |t| yield compatible_table_definition(t) }
+          else
+            super
+          end
+        end
+
+        def change_table(table_name, **options)
+          if block_given?
+            super { |t| yield compatible_table_definition(t) }
+          else
+            super
+          end
+        end
+
+        def create_join_table(table_1, table_2, **options)
+          if block_given?
+            super { |t| yield compatible_table_definition(t) }
+          else
+            super
+          end
+        end
+
+        def add_timestamps(table_name, **options)
+          options[:precision] ||= nil
+          super
+        end
+
         private
+          def compatible_table_definition(t)
+            class << t
+              prepend TableDefinition
+            end
+            t
+          end
 
           def command_recorder
             recorder = super
@@ -87,35 +129,12 @@ module ActiveRecord
             options[:id] = :integer
           end
 
-          if block_given?
-            super do |t|
-              yield compatible_table_definition(t)
-            end
-          else
-            super
-          end
-        end
-
-        def change_table(table_name, options = {})
-          if block_given?
-            super do |t|
-              yield compatible_table_definition(t)
-            end
-          else
-            super
-          end
+          super
         end
 
         def create_join_table(table_1, table_2, column_options: {}, **options)
           column_options.reverse_merge!(type: :integer)
-
-          if block_given?
-            super do |t|
-              yield compatible_table_definition(t)
-            end
-          else
-            super
-          end
+          super
         end
 
         def add_column(table_name, column_name, type, options = {})
@@ -136,7 +155,7 @@ module ActiveRecord
             class << t
               prepend TableDefinition
             end
-            t
+            super
           end
       end
 
@@ -154,33 +173,13 @@ module ActiveRecord
           end
         end
 
-        def create_table(table_name, options = {})
-          if block_given?
-            super do |t|
-              yield compatible_table_definition(t)
-            end
-          else
-            super
-          end
-        end
-
-        def change_table(table_name, options = {})
-          if block_given?
-            super do |t|
-              yield compatible_table_definition(t)
-            end
-          else
-            super
-          end
-        end
-
-        def add_reference(*, **options)
+        def add_reference(table_name, ref_name, **options)
           options[:index] ||= false
           super
         end
         alias :add_belongs_to :add_reference
 
-        def add_timestamps(_, **options)
+        def add_timestamps(table_name, **options)
           options[:null] = true if options[:null].nil?
           super
         end
diff --git a/activerecord/test/cases/ar_schema_test.rb b/activerecord/test/cases/ar_schema_test.rb
index 98af75e8ca..9d88b14dab 100644
--- a/activerecord/test/cases/ar_schema_test.rb
+++ b/activerecord/test/cases/ar_schema_test.rb
@@ -157,4 +157,55 @@ class ActiveRecordSchemaTest < ActiveRecord::TestCase
     assert @connection.column_exists?(:has_timestamps, :created_at, null: false)
     assert @connection.column_exists?(:has_timestamps, :updated_at, null: false)
   end
+
+  if subsecond_precision_supported?
+    def test_timestamps_sets_presicion_on_create_table
+      ActiveRecord::Schema.define do
+        create_table :has_timestamps do |t|
+          t.timestamps
+        end
+      end
+
+      assert @connection.column_exists?(:has_timestamps, :created_at, precision: 6, null: false)
+      assert @connection.column_exists?(:has_timestamps, :updated_at, precision: 6, null: false)
+    end
+
+    def test_timestamps_sets_presicion_on_change_table
+      ActiveRecord::Schema.define do
+        create_table :has_timestamps
+
+        change_table :has_timestamps do |t|
+          t.timestamps default: Time.now
+        end
+      end
+
+      assert @connection.column_exists?(:has_timestamps, :created_at, precision: 6, null: false)
+      assert @connection.column_exists?(:has_timestamps, :updated_at, precision: 6, null: false)
+    end
+
+    if ActiveRecord::Base.connection.supports_bulk_alter?
+      def test_timestamps_sets_presicion_on_change_table_with_bulk
+        ActiveRecord::Schema.define do
+          create_table :has_timestamps
+
+          change_table :has_timestamps, bulk: true do |t|
+            t.timestamps default: Time.now
+          end
+        end
+
+        assert @connection.column_exists?(:has_timestamps, :created_at, precision: 6, null: false)
+        assert @connection.column_exists?(:has_timestamps, :updated_at, precision: 6, null: false)
+      end
+    end
+
+    def test_timestamps_sets_presicion_on_add_timestamps
+      ActiveRecord::Schema.define do
+        create_table :has_timestamps
+        add_timestamps :has_timestamps, default: Time.now
+      end
+
+      assert @connection.column_exists?(:has_timestamps, :created_at, precision: 6, null: false)
+      assert @connection.column_exists?(:has_timestamps, :updated_at, precision: 6, null: false)
+    end
+  end
 end
diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb
index 418793d1af..5753bd7117 100644
--- a/activerecord/test/cases/migration/compatibility_test.rb
+++ b/activerecord/test/cases/migration/compatibility_test.rb
@@ -137,6 +137,68 @@ module ActiveRecord
         assert connection.column_exists?(:testings, :updated_at, null: true)
       end
 
+      def test_timestamps_doesnt_set_precision_on_create_table
+        migration = Class.new(ActiveRecord::Migration[5.2]) {
+          def migrate(x)
+            create_table :more_testings do |t|
+              t.timestamps
+            end
+          end
+        }.new
+
+        ActiveRecord::Migrator.new(:up, [migration]).migrate
+
+        assert connection.column_exists?(:more_testings, :created_at, null: false, **precision_implicit_default)
+        assert connection.column_exists?(:more_testings, :updated_at, null: false, **precision_implicit_default)
+      ensure
+        connection.drop_table :more_testings rescue nil
+      end
+
+      def test_timestamps_doesnt_set_precision_on_change_table
+        migration = Class.new(ActiveRecord::Migration[5.2]) {
+          def migrate(x)
+            change_table :testings do |t|
+              t.timestamps default: Time.now
+            end
+          end
+        }.new
+
+        ActiveRecord::Migrator.new(:up, [migration]).migrate
+
+        assert connection.column_exists?(:testings, :created_at, null: false, **precision_implicit_default)
+        assert connection.column_exists?(:testings, :updated_at, null: false, **precision_implicit_default)
+      end
+
+      if ActiveRecord::Base.connection.supports_bulk_alter?
+        def test_timestamps_doesnt_set_precision_on_change_table_with_bulk
+          migration = Class.new(ActiveRecord::Migration[5.2]) {
+            def migrate(x)
+              change_table :testings, bulk: true do |t|
+                t.timestamps
+              end
+            end
+          }.new
+
+          ActiveRecord::Migrator.new(:up, [migration]).migrate
+
+          assert connection.column_exists?(:testings, :created_at, null: false, **precision_implicit_default)
+          assert connection.column_exists?(:testings, :updated_at, null: false, **precision_implicit_default)
+        end
+      end
+
+      def test_timestamps_doesnt_set_precision_on_add_timestamps
+        migration = Class.new(ActiveRecord::Migration[5.2]) {
+          def migrate(x)
+            add_timestamps :testings, default: Time.now
+          end
+        }.new
+
+        ActiveRecord::Migrator.new(:up, [migration]).migrate
+
+        assert connection.column_exists?(:testings, :created_at, null: false, **precision_implicit_default)
+        assert connection.column_exists?(:testings, :updated_at, null: false, **precision_implicit_default)
+      end
+
       def test_legacy_migrations_raises_exception_when_inherited
         e = assert_raises(StandardError) do
           class_eval("class LegacyMigration < ActiveRecord::Migration; end")
@@ -176,6 +238,15 @@ module ActiveRecord
           ActiveRecord::Base.clear_cache!
         end
       end
+
+      private
+        def precision_implicit_default
+          if current_adapter?(:Mysql2Adapter)
+            { presicion: 0 }
+          else
+            { presicion: nil }
+          end
+        end
     end
   end
 end
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 7034c773d2..86d5a67a13 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -682,11 +682,7 @@ ActiveRecord::Schema.define do
   create_table :pets, primary_key: :pet_id, force: true do |t|
     t.string :name
     t.integer :owner_id, :integer
-    if subsecond_precision_supported?
-      t.timestamps null: false, precision: 6
-    else
-      t.timestamps null: false
-    end
+    t.timestamps
   end
 
   create_table :pets_treasures, force: true do |t|
@@ -904,11 +900,7 @@ ActiveRecord::Schema.define do
     t.string   :parent_title
     t.string   :type
     t.string   :group
-    if subsecond_precision_supported?
-      t.timestamps null: true, precision: 6
-    else
-      t.timestamps null: true
-    end
+    t.timestamps null: true
   end
 
   create_table :toys, primary_key: :toy_id, force: true do |t|