Refactor `type_to_sql` to handle converting `limit` to `size` in itself

Also, improving an argument error message for `limit`, extracting around
`type_to_sql` code into schema statements, and more exercise tests.

6 files changed, 91 insertions, 76 deletions

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 246c6b5123..37506a97e2 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -431,30 +431,6 @@ module ActiveRecord
         table_options
       end
 
-      # Maps logical Rails types to MySQL-specific data types.
-      def type_to_sql(type, limit: nil, precision: nil, scale: nil, unsigned: nil, **) # :nodoc:
-        sql = \
-          case type.to_s
-          when "integer"
-            integer_to_sql(limit)
-          when "text"
-            text_to_sql(limit)
-          when "blob"
-            binary_to_sql(limit)
-          when "binary"
-            if (0..0xfff) === limit
-              "varbinary(#{limit})"
-            else
-              binary_to_sql(limit)
-            end
-          else
-            super
-          end
-
-        sql = "#{sql} unsigned" if unsigned && type != :primary_key
-        sql
-      end
-
       # SHOW VARIABLES LIKE 'name'
       def show_variable(name)
         query_value("SELECT @@#{name}", "SCHEMA")
@@ -818,37 +794,6 @@ module ActiveRecord
           MismatchedForeignKey.new(options)
         end
 
-        def integer_to_sql(limit) # :nodoc:
-          case limit
-          when 1; "tinyint"
-          when 2; "smallint"
-          when 3; "mediumint"
-          when nil, 4; "int"
-          when 5..8; "bigint"
-          else raise(ActiveRecordError, "No integer type has byte size #{limit}. Use a decimal with scale 0 instead.")
-          end
-        end
-
-        def text_to_sql(limit) # :nodoc:
-          case limit
-          when 0..0xff;               "tinytext"
-          when nil, 0x100..0xffff;    "text"
-          when 0x10000..0xffffff;     "mediumtext"
-          when 0x1000000..0xffffffff; "longtext"
-          else raise(ActiveRecordError, "No text type has byte length #{limit}")
-          end
-        end
-
-        def binary_to_sql(limit) # :nodoc:
-          case limit
-          when 0..0xff;               "tinyblob"
-          when nil, 0x100..0xffff;    "blob"
-          when 0x10000..0xffffff;     "mediumblob"
-          when 0x1000000..0xffffffff; "longblob"
-          else raise(ActiveRecordError, "No binary type has byte length #{limit}")
-          end
-        end
-
         def version_string
           full_version.match(/^(?:5\.5\.5-)?(\d+\.\d+\.\d+)/)[1]
         end
diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb
index 8cb6b64fec..d21535a709 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb
@@ -64,16 +64,6 @@ module ActiveRecord
           case type
           when :virtual
             type = options[:type]
-          when :text, :blob, :binary
-            case (size = options[:size])&.to_s
-            when "tiny", "medium", "long"
-              sql_type = @conn.native_database_types[type][:name]
-              type = "#{size}#{sql_type}"
-            else
-              raise ArgumentError, <<~MSG unless size.nil?
-                #{size.inspect} is invalid :size value. Only :tiny, :medium, and :long are allowed.
-              MSG
-            end
           when :primary_key
             type = :integer
             options[:limit] ||= 8
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 e9484a08de..4018f0815c 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
@@ -97,6 +97,30 @@ module ActiveRecord
           MySQL::SchemaDumper.create(self, options)
         end
 
+        # Maps logical Rails types to MySQL-specific data types.
+        def type_to_sql(type, limit: nil, precision: nil, scale: nil, size: limit_to_size(limit, type), unsigned: nil, **)
+          sql =
+            case type.to_s
+            when "integer"
+              integer_to_sql(limit)
+            when "text"
+              type_with_size_to_sql("text", size)
+            when "blob"
+              type_with_size_to_sql("blob", size)
+            when "binary"
+              if (0..0xfff) === limit
+                "varbinary(#{limit})"
+              else
+                type_with_size_to_sql("blob", size)
+              end
+            else
+              super
+            end
+
+          sql = "#{sql} unsigned" if unsigned && type != :primary_key
+          sql
+        end
+
         private
           CHARSETS_OF_4BYTES_MAXLEN = ["utf8mb4", "utf16", "utf16le", "utf32"]
 
@@ -197,6 +221,40 @@ module ActiveRecord
             schema, name = nil, schema unless name
             [schema, name]
           end
+
+          def type_with_size_to_sql(type, size)
+            case size&.to_s
+            when nil, "tiny", "medium", "long"
+              "#{size}#{type}"
+            else
+              raise ArgumentError,
+                "#{size.inspect} is invalid :size value. Only :tiny, :medium, and :long are allowed."
+            end
+          end
+
+          def limit_to_size(limit, type)
+            case type.to_s
+            when "text", "blob", "binary"
+              case limit
+              when 0..0xff;               "tiny"
+              when nil, 0x100..0xffff;    nil
+              when 0x10000..0xffffff;     "medium"
+              when 0x1000000..0xffffffff; "long"
+              else raise ActiveRecordError, "No #{type} type has byte size #{limit}"
+              end
+            end
+          end
+
+          def integer_to_sql(limit)
+            case limit
+            when 1; "tinyint"
+            when 2; "smallint"
+            when 3; "mediumint"
+            when nil, 4; "int"
+            when 5..8; "bigint"
+            else raise ActiveRecordError, "No integer type has byte size #{limit}. Use a decimal with scale 0 instead."
+            end
+          end
       end
     end
   end
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 946436f7f9..d694a4f47d 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -548,14 +548,14 @@ module ActiveRecord
               # The hard limit is 1GB, because of a 32-bit size field, and TOAST.
               case limit
               when nil, 0..0x3fffffff; super(type)
-              else raise(ActiveRecordError, "No binary type has byte size #{limit}.")
+              else raise ActiveRecordError, "No binary type has byte size #{limit}. The limit on binary can be at most 1GB - 1byte."
               end
             when "text"
               # PostgreSQL doesn't support limits on text columns.
               # The hard limit is 1GB, according to section 8.3 in the manual.
               case limit
               when nil, 0..0x3fffffff; super(type)
-              else raise(ActiveRecordError, "The limit on text can be at most 1GB - 1byte.")
+              else raise ActiveRecordError, "No text type has byte size #{limit}. The limit on text can be at most 1GB - 1byte."
               end
             when "integer"
               case limit
diff --git a/activerecord/test/cases/migration/column_attributes_test.rb b/activerecord/test/cases/migration/column_attributes_test.rb
index 3022121f4c..6f9190c110 100644
--- a/activerecord/test/cases/migration/column_attributes_test.rb
+++ b/activerecord/test/cases/migration/column_attributes_test.rb
@@ -177,10 +177,8 @@ module ActiveRecord
       if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter)
         def test_out_of_range_limit_should_raise
           assert_raise(ActiveRecordError) { add_column :test_models, :integer_too_big, :integer, limit: 10 }
-
-          unless current_adapter?(:PostgreSQLAdapter)
-            assert_raise(ActiveRecordError) { add_column :test_models, :text_too_big, :text, limit: 0xfffffffff }
-          end
+          assert_raise(ActiveRecordError) { add_column :test_models, :text_too_big, :text, limit: 0xfffffffff }
+          assert_raise(ActiveRecordError) { add_column :test_models, :binary_too_big, :binary, limit: 0xfffffffff }
         end
       end
     end
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index 02031e51ef..0ecd93412e 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -600,6 +600,18 @@ class MigrationTest < ActiveRecord::TestCase
     end
   end
 
+  def test_decimal_scale_without_precision_should_raise
+    e = assert_raise(ArgumentError) do
+      Person.connection.create_table :test_decimal_scales, force: true do |t|
+        t.decimal :scaleonly, scale: 10
+      end
+    end
+
+    assert_equal "Error adding decimal column: precision cannot be empty if scale is specified", e.message
+  ensure
+    Person.connection.drop_table :test_decimal_scales, if_exists: true
+  end
+
   if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter)
     def test_out_of_range_integer_limit_should_raise
       e = assert_raise(ActiveRecord::ActiveRecordError, "integer limit didn't raise") do
@@ -608,13 +620,11 @@ class MigrationTest < ActiveRecord::TestCase
         end
       end
 
-      assert_match(/No integer type has byte size 10/, e.message)
+      assert_includes e.message, "No integer type has byte size 10"
     ensure
       Person.connection.drop_table :test_integer_limits, if_exists: true
     end
-  end
 
-  if current_adapter?(:Mysql2Adapter)
     def test_out_of_range_text_limit_should_raise
       e = assert_raise(ActiveRecord::ActiveRecordError, "text limit didn't raise") do
         Person.connection.create_table :test_text_limits, force: true do |t|
@@ -622,11 +632,25 @@ class MigrationTest < ActiveRecord::TestCase
         end
       end
 
-      assert_match(/No text type has byte length #{0xfffffffff}/, e.message)
+      assert_includes e.message, "No text type has byte size #{0xfffffffff}"
+    ensure
+      Person.connection.drop_table :test_text_limits, if_exists: true
+    end
+
+    def test_out_of_range_binary_limit_should_raise
+      e = assert_raise(ActiveRecord::ActiveRecordError) do
+        Person.connection.create_table :test_text_limits, force: true do |t|
+          t.binary :bigbinary, limit: 0xfffffffff
+        end
+      end
+
+      assert_includes e.message, "No binary type has byte size #{0xfffffffff}"
     ensure
       Person.connection.drop_table :test_text_limits, if_exists: true
     end
+  end
 
+  if current_adapter?(:Mysql2Adapter)
     def test_invalid_text_size_should_raise
       e = assert_raise(ArgumentError) do
         Person.connection.create_table :test_text_sizes, force: true do |t|
@@ -634,7 +658,7 @@ class MigrationTest < ActiveRecord::TestCase
         end
       end
 
-      assert_match(/#{0xfffffffff} is invalid :size value\. Only :tiny, :medium, and :long are allowed\./, e.message)
+      assert_equal "#{0xfffffffff} is invalid :size value. Only :tiny, :medium, and :long are allowed.", e.message
     ensure
       Person.connection.drop_table :test_text_sizes, if_exists: true
     end