Unify binary handling in adapters

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog for v3.x
 
+## v3.4.2 (2020-04-02)
+
+### Bug fixes
+
+  * [myxql] A binary with size should be a varbinary
+  * [mssql] A binary without size should be a varbinary(max)
+
 ## v3.4.1 (2020-03-25)
 
 ### Bug fixes

lib/ecto/adapters/myxql/connection.ex

@@ -874,13 +874,12 @@ if Code.ensure_loaded?(MyXQL) do
       size      = Keyword.get(opts, :size)
       precision = Keyword.get(opts, :precision)
       scale     = Keyword.get(opts, :scale)
-      type_name = ecto_to_db(type)
 
       cond do
-        size            -> [type_name, ?(, to_string(size), ?)]
-        precision       -> [type_name, ?(, to_string(precision), ?,, to_string(scale || 0), ?)]
-        type == :string -> [type_name, "(255)"]
-        true            -> type_name
+        size -> [ecto_size_to_db(type), ?(, to_string(size), ?)]
+        precision -> [ecto_to_db(type), ?(, to_string(precision), ?,, to_string(scale || 0), ?)]
+        type == :string -> ["varchar(255)"]
+        true -> ecto_to_db(type)
       end
     end
 
@@ -995,6 +994,9 @@ if Code.ensure_loaded?(MyXQL) do
     defp ecto_cast_to_db(:naive_datetime_usec, _query), do: "datetime(6)"
     defp ecto_cast_to_db(type, query), do: ecto_to_db(type, query)
 
+    defp ecto_size_to_db(:binary), do: "varbinary"
+    defp ecto_size_to_db(type), do: ecto_to_db(type)
+
     defp ecto_to_db(type, query \\ nil)
     defp ecto_to_db({:array, _}, query),           do: error!(query, "Array type is not supported by MySQL")
     defp ecto_to_db(:id, _query),                  do: "integer"

lib/ecto/adapters/tds/connection.ex

@@ -1504,16 +1504,13 @@ if Code.ensure_loaded?(Tds) do
     defp ecto_to_db(:string, s, _, _, _) when s in 1..4_000, do: "nvarchar(#{s})"
     defp ecto_to_db(:float, nil, _, _, _), do: "float"
     defp ecto_to_db(:float, s, _, _, _) when s in 1..53, do: "float(#{s})"
-    defp ecto_to_db(:binary, nil, _, _, _), do: "varbinary(2000)"
-    defp ecto_to_db(:binary, :max, _, _, _), do: "varbinary(max)"
+    defp ecto_to_db(:binary, nil, _, _, _), do: "varbinary(max)"
     defp ecto_to_db(:binary, s, _, _, _) when s in 1..8_000, do: "varbinary(#{s})"
     defp ecto_to_db(:uuid, _, _, _, _), do: "uniqueidentifier"
-    defp ecto_to_db(:map, nil, _, _, _), do: "nvarchar(2000)"
-    defp ecto_to_db(:map, :max, _, _, _), do: "nvarchar(max)"
+    defp ecto_to_db(:map, nil, _, _, _), do: "nvarchar(max)"
     defp ecto_to_db(:map, s, _, _, _) when s in 0..4_000, do: "nvarchar(#{s})"
-    defp ecto_to_db({:map, _}, nil, _, _, _), do: "nvarchar(2000)"
+    defp ecto_to_db({:map, _}, nil, _, _, _), do: "nvarchar(max)"
     defp ecto_to_db({:map, _}, s, _, _, _) when s in 1..4_000, do: "nvarchar(#{s})"
-    defp ecto_to_db({:map, _}, :max, _, _, _), do: "nvarchar(max)"
     defp ecto_to_db(:time, _, _, _, _), do: "time(0)"
     defp ecto_to_db(:time_usec, _, p, _, _) when p in 0..7, do: "time(#{p})"
     defp ecto_to_db(:time_usec, _, _, _, _), do: "time(6)"

lib/ecto/migration.ex

@@ -134,13 +134,24 @@ defmodule Ecto.Migration do
   ## Field Types
 
   The Ecto primitive types are mapped to the appropriate database
-  type by the various database adapters. For example, `:string` is converted to
-  `:varchar`, `:binary` to `:bits` or `:blob`, and so on.
+  type by the various database adapters. For example, `:string` is
+  converted to `:varchar`, `:binary` to `:bytea` or `:blob`, and so on.
 
   Similarly, you can pass any field type supported by your database
-  as long as it maps to an Ecto type. For instance, you can use `:text`,
-  `:varchar`, or `:char` in your migrations as `add :field_name, :text`.
-  In your Ecto schema, they will all map to the same `:string` type.
+  as long as it maps to an Ecto type. For instance, for an Ecto schema
+  with the field `:string`, the database migration type can be any of
+  `:text`, `:char` or `:varchar` (the default).
+
+  In particular, note that:
+
+    * the `:string` type in migrations by default has a limit of 255 characters.
+      If you need more or less characters, pass the `:size` option, such
+      as `add :field, :string, size: 10`. If you don't want to impose a limit,
+      most databases support a `:text` type or similar
+
+    * the `:binary` type in migrations by default has no size limit. If you want
+      to impose a limit, pass the `:size` option accordingly. In MySQL, passing
+      the size option changes the underlying field from "blob" to "varbinary"
 
   Remember, atoms can contain arbitrary characters by enclosing in
   double quotes the characters following the colon. So, if you want to use a

test/ecto/adapters/myxql_test.exs

@@ -981,6 +981,7 @@ defmodule Ecto.Adapters.MyXQLTest do
   test "create table" do
     create = {:create, table(:posts),
                [{:add, :name, :string, [default: "Untitled", size: 20, null: false]},
+                {:add, :token, :binary, [size: 20, null: false]},
                 {:add, :price, :numeric, [precision: 8, scale: 2, default: {:fragment, "expr"}]},
                 {:add, :on_hand, :integer, [default: 0, null: true]},
                 {:add, :likes, :"smallint unsigned", [default: 0, null: false]},
@@ -989,6 +990,7 @@ defmodule Ecto.Adapters.MyXQLTest do
 
     assert execute_ddl(create) == ["""
     CREATE TABLE `posts` (`name` varchar(20) DEFAULT 'Untitled' NOT NULL,
+    `token` varbinary(20) NOT NULL,
     `price` numeric(8,2) DEFAULT expr,
     `on_hand` integer DEFAULT 0 NULL,
     `likes` smallint unsigned DEFAULT 0 NOT NULL,

test/ecto/adapters/tds_test.exs

@@ -1094,16 +1094,16 @@ defmodule Ecto.Adapters.TdsTest do
 
     assert execute_ddl(create) ==
              [
-               "CREATE TABLE [blobs] ([blob] varbinary(2000) CONSTRAINT [DF__blobs_blob] DEFAULT (N'foo')); "
+               "CREATE TABLE [blobs] ([blob] varbinary(max) CONSTRAINT [DF__blobs_blob] DEFAULT (N'foo')); "
              ]
   end
 
   test "create table with binary column and hex bytea literal default" do
-    create = {:create, table(:blobs), [{:add, :blob, :binary, [default: "\\x666F6F"]}]}
+    create = {:create, table(:blobs), [{:add, :blob, :binary, [default: "\\x666F6F", size: 16]}]}
 
     assert execute_ddl(create) ==
              [
-               "CREATE TABLE [blobs] ([blob] varbinary(2000) CONSTRAINT [DF__blobs_blob] DEFAULT (N'\\x666F6F')); "
+               "CREATE TABLE [blobs] ([blob] varbinary(16) CONSTRAINT [DF__blobs_blob] DEFAULT (N'\\x666F6F')); "
              ]
   end