Merge pull request #34970 from kamipo/timestamps_with_precision_by_default

Make `t.timestamps` with precision by default.

actionmailbox/test/dummy/db/migrate/20180208205311_create_action_mailbox_tables.rb

@@ -5,11 +5,7 @@ def change
       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

actiontext/test/dummy/db/migrate/2018052816_create_action_text_tables.rb

@@ -5,8 +5,7 @@ def change
       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

actiontext/test/dummy/db/schema.rb

@@ -17,8 +17,8 @@
     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 @@
 
   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

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.

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

@@ -263,6 +263,7 @@ class TableDefinition
       deprecate :indexes=
 
       def initialize(
+        conn,
         name,
         temporary: false,
         if_not_exists: false,
@@ -271,6 +272,7 @@ def initialize(
         comment: nil,
         **
       )
+        @conn = conn
         @columns_hash = {}
         @indexes = []
         @foreign_keys = []
@@ -410,6 +412,10 @@ def foreign_key(table_name, options = {}) # :nodoc:
       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

activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb

@@ -1129,6 +1129,10 @@ def columns_for_distinct(columns, orders) # :nodoc:
       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 @@ def schema_creation
         end
 
         def create_table_definition(*args)
-          TableDefinition.new(*args)
+          TableDefinition.new(self, *args)
         end
 
         def create_alter_table(name)

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -711,6 +711,10 @@ def remove_index_for_alter(table_name, options = {})
         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
 

activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb

@@ -127,7 +127,7 @@ def schema_creation
           end
 
           def create_table_definition(*args)
-            MySQL::TableDefinition.new(*args)
+            MySQL::TableDefinition.new(self, *args)
           end
 
           def new_column_from_field(table_name, field)

activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb

@@ -637,7 +637,7 @@ def schema_creation
           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 @@ def change_column_null_for_alter(table_name, column_name, null, default = nil)
           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
 

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

@@ -62,7 +62,7 @@ def schema_creation
           end
 
           def create_table_definition(*args)
-            SQLite3::TableDefinition.new(*args)
+            SQLite3::TableDefinition.new(self, *args)
           end
 
           def new_column_from_field(table_name, field)

activerecord/lib/active_record/migration/compatibility.rb

@@ -16,13 +16,55 @@ def self.find(version)
       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 @@ def create_table(table_name, options = {})
             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 @@ def compatible_table_definition(t)
             class << t
               prepend TableDefinition
             end
-            t
+            super
           end
       end
 
@@ -154,33 +173,13 @@ def timestamps(**options)
           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

activerecord/test/cases/ar_schema_test.rb

@@ -157,4 +157,55 @@ def test_timestamps_without_null_set_null_to_false_on_add_timestamps
     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

activerecord/test/cases/migration/compatibility_test.rb

@@ -137,6 +137,68 @@ def migrate(x)
         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 @@ def migrate(x)
           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

activerecord/test/schema/schema.rb

@@ -682,11 +682,7 @@
   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 @@
     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|