Merge pull request #18914 from kamipo/format_time_according_to_precision

Format the time string according to the precision of the time column

11 files changed, 166 insertions, 42 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 57d135d1aa..b852391da2 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,11 @@
+*   Format the time string according to the precision of the time column.
+
+    *Ryuta Kamizono*
+
+*   Allow `:precision` option for time type columns.
+
+    *Ryuta Kamizono*
+
 *   Add `ActiveRecord::Base.suppress` to prevent the receiver from being saved
     during the given block.
 
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 503b09d1fc..0438c95bd7 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -851,6 +851,12 @@ module ActiveRecord
               raise ArgumentError, "Error adding decimal column: precision cannot be empty if scale is specified"
             end
 
+          elsif [:datetime, :time].include?(type) && precision ||= native[:precision]
+            if (0..6) === precision
+              column_type_sql << "(#{precision})"
+            else
+              raise(ActiveRecordError, "No #{native[:name]} type has precision of #{precision}. The allowed range of precision is from 0 to 6")
+            end
           elsif (type != :primary_key) && (limit ||= native.is_a?(Hash) && native[:limit])
             column_type_sql << "(#{limit})"
           end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index ee11c0efa4..c307189980 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -245,6 +245,11 @@ module ActiveRecord
         false
       end
 
+      # Does this adapter support datetime with precision?
+      def supports_datetime_with_precision?
+        false
+      end
+
       # This is meant to be implemented by the adapters that support extensions
       def disable_extension(name)
       end
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 0a1c66be1e..8db4bcd7e3 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -75,7 +75,7 @@ module ActiveRecord
 
       def prepare_column_options(column)
         spec = super
-        spec.delete(:precision) if column.type == :datetime && column.precision == 0
+        spec.delete(:precision) if /time/ === column.sql_type && column.precision == 0
         spec
       end
 
@@ -251,6 +251,10 @@ module ActiveRecord
         version[0] >= 5
       end
 
+      def supports_datetime_with_precision?
+        (version[0] == 5 && version[1] >= 6) || version[0] >= 6
+      end
+
       def native_database_types
         NATIVE_DATABASE_TYPES
       end
@@ -623,13 +627,6 @@ module ActiveRecord
           when 0x1000000..0xffffffff; 'longtext'
           else raise(ActiveRecordError, "No text type has character length #{limit}")
           end
-        when 'datetime'
-          return super unless precision
-
-          case precision
-            when 0..6; "datetime(#{precision})"
-            else raise(ActiveRecordError, "No datetime type has precision of #{precision}. The allowed range of precision is from 0 to 6.")
-          end
         else
           super
         end
@@ -736,7 +733,7 @@ module ActiveRecord
       end
 
       def extract_precision(sql_type)
-        if /datetime/ === sql_type
+        if /time/ === sql_type
           super || 0
         else
           super
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 76f568ac03..75b9d079bd 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -533,12 +533,6 @@ module ActiveRecord
             when 5..8; 'bigint'
             else raise(ActiveRecordError, "No integer type has byte size #{limit}. Use a numeric with precision 0 instead.")
             end
-          when 'datetime'
-            case precision
-            when nil; super(type, limit, precision, scale)
-            when 0..6; "timestamp(#{precision})"
-            else raise(ActiveRecordError, "No timestamp type has precision of #{precision}. The allowed range of precision is from 0 to 6")
-            end
           else
             super(type, limit, precision, scale)
           end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index b4fe7506f8..6d25b53b21 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -180,6 +180,10 @@ module ActiveRecord
         true
       end
 
+      def supports_datetime_with_precision?
+        true
+      end
+
       def index_algorithms
         { concurrently: 'CONCURRENTLY' }
       end
diff --git a/activerecord/lib/active_record/type/date_time.rb b/activerecord/lib/active_record/type/date_time.rb
index 05d2af3808..a5199959b9 100644
--- a/activerecord/lib/active_record/type/date_time.rb
+++ b/activerecord/lib/active_record/type/date_time.rb
@@ -10,24 +10,6 @@ module ActiveRecord
         :datetime
       end
 
-      def serialize(value)
-        if precision && value.respond_to?(:usec)
-          number_of_insignificant_digits = 6 - precision
-          round_power = 10 ** number_of_insignificant_digits
-          value = value.change(usec: value.usec / round_power * round_power)
-        end
-
-        if value.acts_like?(:time)
-          zone_conversion_method = ActiveRecord::Base.default_timezone == :utc ? :getutc : :getlocal
-
-          if value.respond_to?(zone_conversion_method)
-            value = value.send(zone_conversion_method)
-          end
-        end
-
-        value
-      end
-
       private
 
       def cast_value(string)
diff --git a/activerecord/lib/active_record/type/helpers/time_value.rb b/activerecord/lib/active_record/type/helpers/time_value.rb
index 6e14c3a9b5..7eb41557cb 100644
--- a/activerecord/lib/active_record/type/helpers/time_value.rb
+++ b/activerecord/lib/active_record/type/helpers/time_value.rb
@@ -2,6 +2,24 @@ module ActiveRecord
   module Type
     module Helpers
       module TimeValue # :nodoc:
+        def serialize(value)
+          if precision && value.respond_to?(:usec)
+            number_of_insignificant_digits = 6 - precision
+            round_power = 10 ** number_of_insignificant_digits
+            value = value.change(usec: value.usec / round_power * round_power)
+          end
+
+          if value.acts_like?(:time)
+            zone_conversion_method = ActiveRecord::Base.default_timezone == :utc ? :getutc : :getlocal
+
+            if value.respond_to?(zone_conversion_method)
+              value = value.send(zone_conversion_method)
+            end
+          end
+
+          value
+        end
+
         def type_cast_for_schema(value)
           "'#{value.to_s(:db)}'"
         end
diff --git a/activerecord/test/cases/date_time_precision_test.rb b/activerecord/test/cases/date_time_precision_test.rb
index 4602ba6d0d..6a4e64b22c 100644
--- a/activerecord/test/cases/date_time_precision_test.rb
+++ b/activerecord/test/cases/date_time_precision_test.rb
@@ -1,7 +1,7 @@
 require 'cases/helper'
 require 'support/schema_dumping_helper'
 
-if mysql_56? || current_adapter?(:PostgreSQLAdapter)
+if ActiveRecord::Base.connection.supports_datetime_with_precision?
 class DateTimePrecisionTest < ActiveRecord::TestCase
   include SchemaDumpingHelper
   self.use_transactional_fixtures = false
@@ -71,9 +71,18 @@ class DateTimePrecisionTest < ActiveRecord::TestCase
     assert_equal 999900, foo.updated_at.usec
   end
 
+  def test_schema_dump_includes_datetime_precision
+    @connection.create_table(:foos, force: true) do |t|
+      t.timestamps precision: 6
+    end
+    output = dump_table_schema("foos")
+    assert_match %r{t\.datetime\s+"created_at",\s+precision: 6,\s+null: false$}, output
+    assert_match %r{t\.datetime\s+"updated_at",\s+precision: 6,\s+null: false$}, output
+  end
+
   if current_adapter?(:PostgreSQLAdapter)
     def test_datetime_precision_with_zero_should_be_dumped
-      @connection.create_table(:foos) do |t|
+      @connection.create_table(:foos, force: true) do |t|
         t.timestamps precision: 0
       end
       output = dump_table_schema("foos")
diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb
index 0413984729..7d8d6421a9 100644
--- a/activerecord/test/cases/schema_dumper_test.rb
+++ b/activerecord/test/cases/schema_dumper_test.rb
@@ -232,13 +232,6 @@ class SchemaDumperTest < ActiveRecord::TestCase
     end
   end
 
-  if mysql_56?
-    def test_schema_dump_includes_datetime_precision
-      output = standard_dump
-      assert_match %r{t\.datetime\s+"written_on",\s+precision: 6$}, output
-    end
-  end
-
   def test_schema_dump_includes_decimal_options
     output = dump_all_table_schema([/^[^n]/])
     assert_match %r{precision: 3,[[:space:]]+scale: 2,[[:space:]]+default: 2\.78}, output
diff --git a/activerecord/test/cases/time_precision_test.rb b/activerecord/test/cases/time_precision_test.rb
new file mode 100644
index 0000000000..ff4e5ecec5
--- /dev/null
+++ b/activerecord/test/cases/time_precision_test.rb
@@ -0,0 +1,108 @@
+require 'cases/helper'
+require 'support/schema_dumping_helper'
+
+if ActiveRecord::Base.connection.supports_datetime_with_precision?
+class TimePrecisionTest < ActiveRecord::TestCase
+  include SchemaDumpingHelper
+  self.use_transactional_fixtures = false
+
+  class Foo < ActiveRecord::Base; end
+
+  setup do
+    @connection = ActiveRecord::Base.connection
+  end
+
+  teardown do
+    @connection.drop_table :foos, if_exists: true
+  end
+
+  def test_time_data_type_with_precision
+    @connection.create_table(:foos, force: true)
+    @connection.add_column :foos, :start,  :time, precision: 3
+    @connection.add_column :foos, :finish, :time, precision: 6
+    assert_equal 3, activerecord_column_option('foos', 'start',  'precision')
+    assert_equal 6, activerecord_column_option('foos', 'finish', 'precision')
+  end
+
+  def test_passing_precision_to_time_does_not_set_limit
+    @connection.create_table(:foos, force: true) do |t|
+      t.time :start,  precision: 3
+      t.time :finish, precision: 6
+    end
+    assert_nil activerecord_column_option('foos', 'start',  'limit')
+    assert_nil activerecord_column_option('foos', 'finish', 'limit')
+  end
+
+  def test_invalid_time_precision_raises_error
+    assert_raises ActiveRecord::ActiveRecordError do
+      @connection.create_table(:foos, force: true) do |t|
+        t.time :start,  precision: 7
+        t.time :finish, precision: 7
+      end
+    end
+  end
+
+  def test_database_agrees_with_activerecord_about_precision
+    @connection.create_table(:foos, force: true) do |t|
+      t.time :start,  precision: 2
+      t.time :finish, precision: 4
+    end
+    assert_equal 2, database_datetime_precision('foos', 'start')
+    assert_equal 4, database_datetime_precision('foos', 'finish')
+  end
+
+  def test_formatting_time_according_to_precision
+    @connection.create_table(:foos, force: true) do |t|
+      t.time :start,  precision: 0
+      t.time :finish, precision: 4
+    end
+    time = ::Time.utc(2000, 1, 1, 12, 30, 0, 999999)
+    Foo.create!(start: time, finish: time)
+    assert foo = Foo.find_by(start: time)
+    assert_equal 1, Foo.where(finish: time).count
+    assert_equal time.to_s, foo.start.to_s
+    assert_equal time.to_s, foo.finish.to_s
+    assert_equal 000000, foo.start.usec
+    assert_equal 999900, foo.finish.usec
+  end
+
+  def test_schema_dump_includes_time_precision
+    @connection.create_table(:foos, force: true) do |t|
+      t.time :start,  precision: 4
+      t.time :finish, precision: 6
+    end
+    output = dump_table_schema("foos")
+    assert_match %r{t\.time\s+"start",\s+precision: 4$}, output
+    assert_match %r{t\.time\s+"finish",\s+precision: 6$}, output
+  end
+
+  if current_adapter?(:PostgreSQLAdapter)
+    def test_time_precision_with_zero_should_be_dumped
+      @connection.create_table(:foos, force: true) do |t|
+        t.time :start,  precision: 0
+        t.time :finish, precision: 0
+      end
+      output = dump_table_schema("foos")
+      assert_match %r{t\.time\s+"start",\s+precision: 0$}, output
+      assert_match %r{t\.time\s+"finish",\s+precision: 0$}, output
+    end
+  end
+
+  private
+
+  def database_datetime_precision(table_name, column_name)
+    results = @connection.exec_query("SELECT column_name, datetime_precision FROM information_schema.columns WHERE table_name = '#{table_name}'")
+    result = results.find do |result_hash|
+      result_hash["column_name"] == column_name
+    end
+    result && result["datetime_precision"].to_i
+  end
+
+  def activerecord_column_option(tablename, column_name, option)
+    result = @connection.columns(tablename).find do |column|
+      column.name == column_name
+    end
+    result && result.send(option)
+  end
+end
+end