From 294ac74fb9c15b44a9afac06ba34755bfa6a6589 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 2 Apr 2020 10:27:41 +0200 Subject: [PATCH] Unify binary handling in adapters --- CHANGELOG.md | 7 +++++++ lib/ecto/adapters/myxql/connection.ex | 12 +++++++----- lib/ecto/adapters/tds/connection.ex | 9 +++------ lib/ecto/migration.ex | 21 ++++++++++++++++----- test/ecto/adapters/myxql_test.exs | 2 ++ test/ecto/adapters/tds_test.exs | 6 +++--- 6 files changed, 38 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e64e142..4a2341e4 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/ecto/adapters/myxql/connection.ex b/lib/ecto/adapters/myxql/connection.ex index 9f921c2b..a41bb45e 100644 --- a/lib/ecto/adapters/myxql/connection.ex +++ b/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" diff --git a/lib/ecto/adapters/tds/connection.ex b/lib/ecto/adapters/tds/connection.ex index 0f998067..a6e7d122 100644 --- a/lib/ecto/adapters/tds/connection.ex +++ b/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)" diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex index 43a1336b..19a4eb2d 100644 --- a/lib/ecto/migration.ex +++ b/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 diff --git a/test/ecto/adapters/myxql_test.exs b/test/ecto/adapters/myxql_test.exs index 311f6f63..8f0e24da 100644 --- a/test/ecto/adapters/myxql_test.exs +++ b/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, diff --git a/test/ecto/adapters/tds_test.exs b/test/ecto/adapters/tds_test.exs index 5dce2a99..adf88e5a 100644 --- a/test/ecto/adapters/tds_test.exs +++ b/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