diff --git a/lib/ecto/adapters/postgres/connection.ex b/lib/ecto/adapters/postgres/connection.ex index b4d16e12..a8d7d5c1 100644 --- a/lib/ecto/adapters/postgres/connection.ex +++ b/lib/ecto/adapters/postgres/connection.ex @@ -1532,7 +1532,7 @@ if Code.ensure_loaded?(Postgrex) do [ quote_name(name), ?\s, - reference_column_type(ref.type, opts), + add_reference_column_type(ref.type, opts), column_options(ref.type, opts), ", ", reference_expr(ref, table, name) @@ -1540,7 +1540,7 @@ if Code.ensure_loaded?(Postgrex) do end defp column_definition(_table, {:add, name, type, opts}) do - [quote_name(name), ?\s, column_type(type, opts), column_options(type, opts)] + [quote_name(name), ?\s, add_column_type(type, opts), column_options(type, opts)] end defp column_changes(table, columns) do @@ -1552,7 +1552,7 @@ if Code.ensure_loaded?(Postgrex) do "ADD COLUMN ", quote_name(name), ?\s, - reference_column_type(ref.type, opts), + add_reference_column_type(ref.type, opts), column_options(ref.type, opts), ", ADD ", reference_expr(ref, table, name) @@ -1560,7 +1560,13 @@ if Code.ensure_loaded?(Postgrex) do end defp column_change(_table, {:add, name, type, opts}) do - ["ADD COLUMN ", quote_name(name), ?\s, column_type(type, opts), column_options(type, opts)] + [ + "ADD COLUMN ", + quote_name(name), + ?\s, + add_column_type(type, opts), + column_options(type, opts) + ] end defp column_change(table, {:add_if_not_exists, name, %Reference{} = ref, opts}) do @@ -1568,7 +1574,7 @@ if Code.ensure_loaded?(Postgrex) do "ADD COLUMN IF NOT EXISTS ", quote_name(name), ?\s, - reference_column_type(ref.type, opts), + add_reference_column_type(ref.type, opts), column_options(ref.type, opts), ", ADD ", reference_expr(ref, table, name) @@ -1580,7 +1586,7 @@ if Code.ensure_loaded?(Postgrex) do "ADD COLUMN IF NOT EXISTS ", quote_name(name), ?\s, - column_type(type, opts), + add_column_type(type, opts), column_options(type, opts) ] end @@ -1594,11 +1600,11 @@ if Code.ensure_loaded?(Postgrex) do quote_name(name), " TYPE ", reference_column_type(ref.type, opts), + collation_expr(collation), ", ADD ", reference_expr(ref, table, name), modify_null(name, opts), - modify_default(name, ref.type, opts), - collation_expr(collation) + modify_default(name, ref.type, opts) ] end @@ -1611,9 +1617,9 @@ if Code.ensure_loaded?(Postgrex) do quote_name(name), " TYPE ", modify_column_type(type, opts), + collation_expr(collation), modify_null(name, opts), modify_default(name, type, opts), - collation_expr(collation), modify_identity(name, type, opts) ] end @@ -1675,18 +1681,23 @@ if Code.ensure_loaded?(Postgrex) do defp column_options(type, opts) do default = Keyword.fetch(opts, :default) null = Keyword.get(opts, :null) - collation = Keyword.fetch(opts, :collation) - [default_expr(default, type), null_expr(null), collation_expr(collation)] + [default_expr(default, type), null_expr(null)] end defp null_expr(false), do: " NOT NULL" defp null_expr(true), do: " NULL" defp null_expr(_), do: [] - defp collation_expr({:ok, collation_name}), do: " COLLATE \"#{collation_name}\"" + defp collation_expr({:ok, name}), do: [" COLLATE ", quote_collation_name(name)] defp collation_expr(_), do: [] + defp quote_collation_name(name) when is_atom(name), + do: quote_collation_name(Atom.to_string(name)) + + defp quote_collation_name(name) when is_binary(name), + do: [?", String.replace(name, "\"", "\"\""), ?"] + defp new_constraint_expr(%Constraint{check: check} = constraint) when is_binary(check) do [ "CONSTRAINT ", @@ -1815,18 +1826,28 @@ if Code.ensure_loaded?(Postgrex) do defp options_expr(options), do: [?\s, options] - defp column_type(type, opts) do + defp add_column_type(type, opts) do + column_type(type, opts, collation_expr(Keyword.fetch(opts, :collation))) + end + + defp add_reference_column_type(type, opts) when type in [:serial, :bigserial, :identity] do + [reference_column_type(type, opts), collation_expr(Keyword.fetch(opts, :collation))] + end + + defp add_reference_column_type(type, opts), do: add_column_type(type, opts) + + defp column_type(type, opts, collation \\ []) do type_name = column_type_name(type, opts) case Keyword.get(opts, :generated) do nil when type == :identity -> - [type_name, identity_generated_expr(opts)] + [type_name, collation, identity_generated_expr(opts)] nil -> - type_name + [type_name, collation] expr when is_binary(expr) -> - [type_name, " GENERATED ", expr] + [type_name, collation, " GENERATED ", expr] other -> raise ArgumentError, diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex index 50a440c9..18c0e777 100644 --- a/lib/ecto/migration.ex +++ b/lib/ecto/migration.ex @@ -1450,7 +1450,9 @@ defmodule Ecto.Migration do specified. * `:scale` - the scale of a numeric type. Defaults to `0`. * `:comment` - adds a comment to the modified column. - * `:collation` - the collation of the text type. + * `:collation` - the collation of the text type. PostgreSQL resets the + collation to the type's default when modifying a column without this option, + even if the type is unchanged. Specify the current collation to preserve it. """ def modify(column, type, opts \\ []) when is_atom(column) and is_list(opts) do validate_column_opts!(opts, @modify_column_opts, "modify/3") diff --git a/test/ecto/adapters/postgres_test.exs b/test/ecto/adapters/postgres_test.exs index f3577fc7..deb551f9 100644 --- a/test/ecto/adapters/postgres_test.exs +++ b/test/ecto/adapters/postgres_test.exs @@ -2799,6 +2799,75 @@ defmodule Ecto.Adapters.PostgresTest do ] end + test "alter column collation stays with the type when modifying null or default" do + assert execute_ddl( + {:alter, table(:posts), [{:modify, :name, :text, collation: "C", null: false}]} + ) == + [ + ~s|ALTER TABLE "posts" ALTER COLUMN "name" TYPE text COLLATE "C", ALTER COLUMN "name" SET NOT NULL| + ] + + assert execute_ddl( + {:alter, table(:posts), [{:modify, :name, :text, collation: "C", default: "x"}]} + ) == + [ + ~s|ALTER TABLE "posts" ALTER COLUMN "name" TYPE text COLLATE "C", ALTER COLUMN "name" SET DEFAULT 'x'| + ] + + assert execute_ddl( + {:alter, table(:posts), + [ + {:modify, :name, %Reference{table: :names, type: :text}, + collation: "C", null: false, default: "x"} + ]} + ) == + [ + ~s|ALTER TABLE "posts" ALTER COLUMN "name" TYPE text COLLATE "C", ADD CONSTRAINT "posts_name_fkey" FOREIGN KEY ("name") REFERENCES "names"("id"), ALTER COLUMN "name" SET NOT NULL, ALTER COLUMN "name" SET DEFAULT 'x'| + ] + end + + test "column collation precedes defaults and constraints when adding columns" do + opts = [collation: "C", default: "x", null: false] + + assert execute_ddl({:create, table(:posts), [{:add, :name, :text, opts}]}) == [ + ~s|CREATE TABLE "posts" ("name" text COLLATE "C" DEFAULT 'x' NOT NULL)| + ] + + assert execute_ddl({:alter, table(:posts), [{:add, :name, :text, opts}]}) == [ + ~s|ALTER TABLE "posts" ADD COLUMN "name" text COLLATE "C" DEFAULT 'x' NOT NULL| + ] + + assert execute_ddl({:alter, table(:posts), [{:add_if_not_exists, :name, :text, opts}]}) == [ + ~s|ALTER TABLE "posts" ADD COLUMN IF NOT EXISTS "name" text COLLATE "C" DEFAULT 'x' NOT NULL| + ] + + assert execute_ddl( + {:alter, table(:posts), + [{:add, :name, %Reference{table: :names, type: :text}, opts}]} + ) == + [ + ~s|ALTER TABLE "posts" ADD COLUMN "name" text COLLATE "C" DEFAULT 'x' NOT NULL, ADD CONSTRAINT "posts_name_fkey" FOREIGN KEY ("name") REFERENCES "names"("id")| + ] + + assert execute_ddl( + {:alter, table(:posts), + [{:add, :computed, :text, collation: "C", generated: "ALWAYS AS (name) STORED"}]} + ) == + [ + ~s|ALTER TABLE "posts" ADD COLUMN "computed" text COLLATE "C" GENERATED ALWAYS AS (name) STORED| + ] + end + + test "collation strings are quoted as single identifiers" do + assert execute_ddl({:alter, table(:posts), [{:modify, :name, :text, collation: "odd\"name"}]}) == + [~s|ALTER TABLE "posts" ALTER COLUMN "name" TYPE text COLLATE "odd""name"|] + + assert execute_ddl( + {:alter, table(:posts), [{:add, :name, :text, collation: "name.with.dot"}]} + ) == + [~s|ALTER TABLE "posts" ADD COLUMN "name" text COLLATE "name.with.dot"|] + end + test "alter table with comments on table and columns" do alter = {:alter, table(:posts, comment: "table comment"),