Fix virtual datetime default precision

Prior to this change

    t.virtual :column_name, type: :datetime

would erroneously produce the same result as

    t.virtual :column_name, type: :datetime, precision: nil

This is because the code path for `virtual` skipped the default lookup:

- `t.datetime` is delegated to the `column` method
  - `column` sees `type == :datetime` and sets a default `precision` is none was given
  - `column` calls `new_column_definition` to add the result to `@columns_hash`
- `t.virtual` is delegated to the `column` method
  - `column` sees `type == :virtual`, not `:datetime`, so skips the default lookup
  - `column` calls `new_column_definition` to add the result to `@columns_hash`
    - `new_column_definition` sees `type == :virtual`, so sets `type = options[:type]`

By moving the default lookup, we get consistent code paths:

- `t.datetime` is delegated to the `column` method
  - `column` calls `new_column_definition` to add the result to `@columns_hash`
    - `new_column_definition` sees `type == :datetime` and sets a default `precision` is none was given
- `t.virtual` is delegated to the `column` method
  - `column` calls `new_column_definition` to add the result to `@columns_hash`
    - `new_column_definition` sees `type == :virtual`, so sets `type = options[:type]`
    - `new_column_definition` sees `type == :datetime` and sets a default `precision` is none was given

Author: sambostock

activerecord/CHANGELOG.md

@@ -1,3 +1,17 @@
+*   Fix erroneous nil default precision on virtual datetime columns.
+
+    Prior to this change, virtual datetime columns did not have the same
+    default precision as regular datetime columns, resulting in the following
+    being erroneously equivalent:
+
+        t.virtual :name, type: datetime,                 as: "expression"
+        t.virtual :name, type: datetime, precision: nil, as: "expression"
+
+    This change fixes the default precision lookup, so virtual and regular
+    datetime column default precisions match.
+
+    *Sam Bostock*
+
 *   Use connection from `#with_raw_connection` in `#quote_string`.
 
     This ensures that the string quoting is wrapped in the reconnect and retry logic

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

@@ -475,12 +475,6 @@ 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
@@ -547,6 +541,13 @@ def new_column_definition(name, type, **options) # :nodoc:
           type = integer_like_primary_key_type(type, options)
         end
         type = aliased_types(type.to_s, type)
+
+        if @conn.supports_datetime_with_precision?
+          if type == :datetime && !options.key?(:precision)
+            options[:precision] = 6
+          end
+        end
+
         options[:primary_key] ||= type == :primary_key
         options[:null] = false if options[:primary_key]
         create_column_definition(name, type, options)

activerecord/test/cases/adapters/mysql2/virtual_column_test.rb

@@ -63,7 +63,7 @@ def test_schema_dumping
       assert_match(/t\.virtual\s+"name_length",\s+type: :integer,\s+as: "(?:octet_length|length)\(`name`\)",\s+stored: true$/i, output)
       assert_match(/t\.virtual\s+"name_octet_length",\s+type: :integer,\s+as: "(?:octet_length|length)\(`name`\)",\s+stored: true$/i, output)
       assert_match(/t\.virtual\s+"profile_email",\s+type: :string,\s+as: "json_extract\(`profile`,\w*?'\$\.email'\)", stored: true$/i, output)
-      assert_match(/t\.virtual\s+"time_mirror",\s+type: :datetime,\s+precision: nil,\s+as: "`time`"$/i, output[/^.*time_mirror.*$/])
+      assert_match(/t\.virtual\s+"time_mirror",\s+type: :datetime,\s+as: "`time`"$/i, output[/^.*time_mirror.*$/])
     end
   end
 end

activerecord/test/cases/migration/change_schema_test.rb

@@ -286,7 +286,7 @@ def test_add_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