Merge pull request #42297 from robertomiranda/r/default-precision-datime

Set precision 6 by default for datetime columns
rails · Jun 25, 2021 · 5a26648 · 5a26648
2 parents 6ae78e9 + c2a6f61
commit 5a26648
Show file tree

Hide file tree

Showing 11 changed files with 100 additions and 17 deletions.
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Set precision 6 by default for `datetime` columns
+
+    By default, datetime columns will have microseconds precision instead of seconds precision.
+
+    *Roberto Miranda*
+
 *   Allow preloading of associations with instance dependent scopes
 
     *John Hawthorn*, *John Crepezzi*, *Adam Hess*, *Eileen M. Uchitelle*, *Dinah Shi*

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -411,6 +411,12 @@ def column(name, type, index: nil, **options)
           end
         end
 
+        if @conn.supports_datetime_with_precision?
+          if type == :datetime && !options.key?(:precision)
+            options[:precision] = 6
+          end
+        end
+
         @columns_hash[name] = new_column_definition(name, type, **options)
 
         if index

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -613,6 +613,12 @@ def drop_table(table_name, **options)
       def add_column(table_name, column_name, type, **options)
         return if options[:if_not_exists] == true && column_exists?(table_name, column_name)
 
+        if supports_datetime_with_precision?
+          if type == :datetime && !options.key?(:precision)
+            options[:precision] = 6
+          end
+        end
+
         at = create_alter_table table_name
         at.add_column(column_name, type, **options)
         execute schema_creation.accept at

diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb
@@ -295,6 +295,11 @@ def timestamps(**options)
             options[:null] = true if options[:null].nil?
             super
           end
+
+          def column(name, type, index: nil, **options)
+            options[:precision] ||= nil
+            super
+          end
         end
 
         def add_reference(table_name, ref_name, **options)
@@ -324,6 +329,14 @@ def remove_index(table_name, column_name = nil, **options)
           super
         end
 
+        def add_column(table_name, column_name, type, **options)
+          if type == :datetime
+            options[:precision] ||= nil
+          end
+
+          super
+        end
+
         private
           def compatible_table_definition(t)
             class << t

diff --git a/activerecord/test/cases/adapters/postgresql/timestamp_test.rb b/activerecord/test/cases/adapters/postgresql/timestamp_test.rb
@@ -125,7 +125,7 @@ def test_adds_column_as_custom_type
 
     ActiveRecord::ConnectionAdapters::PostgreSQLAdapter::NATIVE_DATABASE_TYPES[:datetimes_as_enum] = { name: "custom_time_format" }
     with_postgresql_datetime_type(:datetimes_as_enum) do
-      ActiveRecord::Migration.new.add_column :postgresql_timestamp_with_zones, :times, :datetime
+      ActiveRecord::Migration.new.add_column :postgresql_timestamp_with_zones, :times, :datetime, precision: nil
 
       assert_equal({ "data_type" => "USER-DEFINED", "udt_name" => "custom_time_format" },
                    PostgresqlTimestampWithZone.connection.execute("select data_type, udt_name from information_schema.columns where column_name = 'times'").to_a.first)

diff --git a/activerecord/test/cases/date_time_precision_test.rb b/activerecord/test/cases/date_time_precision_test.rb
@@ -48,8 +48,8 @@ def test_datetime_precision_is_truncated_on_assignment
     unless current_adapter?(:Mysql2Adapter)
       def test_no_datetime_precision_isnt_truncated_on_assignment
         @connection.create_table(:foos, force: true)
-        @connection.add_column :foos, :created_at, :datetime
-        @connection.add_column :foos, :updated_at, :datetime, precision: 6
+        @connection.add_column :foos, :created_at, :datetime, precision: nil
+        @connection.add_column :foos, :updated_at, :datetime
 
         time = ::Time.now.change(nsec: 123)
         foo = Foo.new(created_at: time, updated_at: time)

diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb
@@ -91,13 +91,13 @@ class PostgresqlDefaultExpressionTest < ActiveRecord::TestCase
       output = dump_table_schema("defaults")
       if ActiveRecord::Base.connection.database_version >= 100000
         assert_match %r/t\.date\s+"modified_date",\s+default: -> { "CURRENT_DATE" }/, output
-        assert_match %r/t\.datetime\s+"modified_time",\s+default: -> { "CURRENT_TIMESTAMP" }/, output
+        assert_match %r/t\.datetime\s+"modified_time",\s+precision: 6,\s+default: -> { "CURRENT_TIMESTAMP" }/, output
       else
         assert_match %r/t\.date\s+"modified_date",\s+default: -> { "\('now'::text\)::date" }/, output
-        assert_match %r/t\.datetime\s+"modified_time",\s+default: -> { "now\(\)" }/, output
+        assert_match %r/t\.datetime\s+"modified_time",\s+precision: 6,\s+default: -> { "now\(\)" }/, output
       end
       assert_match %r/t\.date\s+"modified_date_function",\s+default: -> { "now\(\)" }/, output
-      assert_match %r/t\.datetime\s+"modified_time_function",\s+default: -> { "now\(\)" }/, output
+      assert_match %r/t\.datetime\s+"modified_time_function",\s+precision: 6,\s+default: -> { "now\(\)" }/, output
     end
   end
 end

diff --git a/activerecord/test/cases/migration/change_schema_test.rb b/activerecord/test/cases/migration/change_schema_test.rb
@@ -300,11 +300,11 @@ def test_add_column_with_postgresql_datetime_type
         assert_equal :datetime, column.type
 
         if current_adapter?(:PostgreSQLAdapter)
-          assert_equal "timestamp without time zone", column.sql_type
+          assert_equal "timestamp(6) without time zone", column.sql_type
         elsif current_adapter?(:Mysql2Adapter)
-          assert_equal "datetime", column.sql_type
+          assert_equal "datetime(6)", column.sql_type
         else
-          assert_equal connection.type_to_sql("datetime"), column.sql_type
+          assert_equal connection.type_to_sql("datetime(6)"), column.sql_type
         end
       end
 
@@ -318,7 +318,7 @@ def test_add_column_with_datetime_in_timestamptz_mode
             column = connection.columns(:testings).find { |c| c.name == "foo" }
 
             assert_equal :datetime, column.type
-            assert_equal "timestamp with time zone", column.sql_type
+            assert_equal "timestamp(6) with time zone", column.sql_type
           end
         end
       end
@@ -341,7 +341,7 @@ def test_change_column_with_timestamp_type
         elsif current_adapter?(:OracleAdapter)
           assert_equal "TIMESTAMP(6)", column.sql_type
         else
-          assert_equal connection.type_to_sql("datetime"), column.sql_type
+          assert_equal connection.type_to_sql("datetime(6)"), column.sql_type
         end
       end
 

diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb
@@ -335,6 +335,58 @@ def migrate(x)
         assert connection.index_exists?(:testings, [:gizmo_type, :gizmo_id], name: :index_testings_on_gizmo_type_and_gizmo_id)
       end
 
+      def test_datetime_doesnt_set_precision_on_create_table
+        migration = Class.new(ActiveRecord::Migration[4.2]) {
+          def migrate(x)
+            create_table :more_testings do |t|
+              t.datetime :published_at
+            end
+          end
+        }.new
+
+        ActiveRecord::Migrator.new(:up, [migration], @schema_migration).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_table
+        create_migration = Class.new(ActiveRecord::Migration[4.2]) {
+          def migrate(x)
+            create_table :more_testings do |t|
+              t.datetime :published_at
+            end
+          end
+        }.new
+
+        change_migration = Class.new(ActiveRecord::Migration[4.2]) {
+          def migrate(x)
+            change_table :more_testings do |t|
+              t.datetime :published_at, default: Time.now
+            end
+          end
+        }.new
+
+        ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration).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
+        migration = Class.new(ActiveRecord::Migration[4.2]) {
+          def migrate(x)
+            add_column :testings, :published_at, :datetime, default: Time.now
+          end
+        }.new
+
+        ActiveRecord::Migrator.new(:up, [migration], @schema_migration).migrate
+
+        assert connection.column_exists?(:testings, :published_at, **precision_implicit_default)
+      end
+
       private
         def precision_implicit_default
           if current_adapter?(:Mysql2Adapter)

diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb
@@ -837,7 +837,7 @@ def test_schema_dump_defaults_with_universally_supported_types
 
     assert_match %r{t\.string\s+"string_with_default",.*?default: "Hello!"}, output
     assert_match %r{t\.date\s+"date_with_default",\s+default: "2014-06-05"}, output
-    assert_match %r{t\.datetime\s+"datetime_with_default",\s+default: "2014-06-05 07:17:04"}, output
+    assert_match %r{t\.datetime\s+"datetime_with_default",\s+precision: 6,\s+default: "2014-06-05 07:17:04"}, output
     assert_match %r{t\.time\s+"time_with_default",\s+default: "2000-01-01 07:17:04"}, output
     assert_match %r{t\.decimal\s+"decimal_with_default",\s+precision: 20,\s+scale: 10,\s+default: "1234567890.0123456789"}, output
   end
@@ -847,8 +847,8 @@ def test_schema_dump_with_column_infinity_default
     output = dump_table_schema("infinity_defaults")
     assert_match %r{t\.float\s+"float_with_inf_default",\s+default: ::Float::INFINITY}, output
     assert_match %r{t\.float\s+"float_with_nan_default",\s+default: ::Float::NAN}, output
-    assert_match %r{t\.datetime\s+"beginning_of_time",\s+default: -::Float::INFINITY}, output
-    assert_match %r{t\.datetime\s+"end_of_time",\s+default: ::Float::INFINITY}, output
+    assert_match %r{t\.datetime\s+"beginning_of_time",\s+precision: 6,\s+default: -::Float::INFINITY}, output
+    assert_match %r{t\.datetime\s+"end_of_time",\s+precision: 6,\s+default: ::Float::INFINITY}, output
     assert_match %r{t\.date\s+"date_with_neg_inf_default",\s+default: -::Float::INFINITY}, output
     assert_match %r{t\.date\s+"date_with_pos_inf_default",\s+default: ::Float::INFINITY}, output
   end

diff --git a/activerecord/test/schema/mysql2_specific_schema.rb b/activerecord/test/schema/mysql2_specific_schema.rb
@@ -3,13 +3,13 @@
 ActiveRecord::Schema.define do
   if supports_datetime_with_precision?
     create_table :datetime_defaults, force: true do |t|
-      t.datetime :modified_datetime, default: -> { "CURRENT_TIMESTAMP" }
-      t.datetime :precise_datetime, precision: 6, default: -> { "CURRENT_TIMESTAMP(6)" }
+      t.datetime :modified_datetime, precision: nil, default: -> { "CURRENT_TIMESTAMP" }
+      t.datetime :precise_datetime, default: -> { "CURRENT_TIMESTAMP(6)" }
     end
 
     create_table :timestamp_defaults, force: true do |t|
       t.timestamp :nullable_timestamp
-      t.timestamp :modified_timestamp, default: -> { "CURRENT_TIMESTAMP" }
+      t.timestamp :modified_timestamp, precision: nil, default: -> { "CURRENT_TIMESTAMP" }
       t.timestamp :precise_timestamp, precision: 6, default: -> { "CURRENT_TIMESTAMP(6)" }
     end
   end