Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support :prefix on index rename #573

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 29 additions & 32 deletions lib/ecto/adapters/postgres/connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ if Code.ensure_loaded?(Postgrex) do

[
"INSERT INTO ",
quote_table(prefix, table),
quote_name(prefix, table),
insert_as(on_conflict),
values,
on_conflict(on_conflict, header) | returning(returning)
Expand Down Expand Up @@ -332,7 +332,7 @@ if Code.ensure_loaded?(Postgrex) do

[
"UPDATE ",
quote_table(prefix, table),
quote_name(prefix, table),
" SET ",
fields,
" WHERE ",
Expand All @@ -351,7 +351,7 @@ if Code.ensure_loaded?(Postgrex) do
{[quote_name(field), " = $" | Integer.to_string(acc)], acc + 1}
end)

["DELETE FROM ", quote_table(prefix, table), " WHERE ", filters | returning(returning)]
["DELETE FROM ", quote_name(prefix, table), " WHERE ", filters | returning(returning)]
end

@impl true
Expand Down Expand Up @@ -1136,7 +1136,7 @@ if Code.ensure_loaded?(Postgrex) do

{table, schema, prefix} ->
name = as_prefix ++ [create_alias(table) | Integer.to_string(pos)]
{quote_table(prefix, table), name, schema}
{quote_name(prefix, table), name, schema}

%Ecto.SubQuery{} ->
{nil, as_prefix ++ [?s | Integer.to_string(pos)], nil}
Expand All @@ -1160,7 +1160,7 @@ if Code.ensure_loaded?(Postgrex) do

@impl true
def execute_ddl({command, %Table{} = table, columns}) when command in @creates do
table_name = quote_table(table.prefix, table.name)
table_name = quote_name(table.prefix, table.name)

query = [
"CREATE TABLE ",
Expand All @@ -1184,14 +1184,14 @@ if Code.ensure_loaded?(Postgrex) do
[
"DROP TABLE ",
if_do(command == :drop_if_exists, "IF EXISTS "),
quote_table(table.prefix, table.name),
quote_name(table.prefix, table.name),
drop_mode(mode)
]
]
end

def execute_ddl({:alter, %Table{} = table, changes}) do
table_name = quote_table(table.prefix, table.name)
table_name = quote_name(table.prefix, table.name)

query = [
"ALTER TABLE ",
Expand Down Expand Up @@ -1227,7 +1227,7 @@ if Code.ensure_loaded?(Postgrex) do
quote_name(index.name),
" ON ",
if_do(index.only, "ONLY "),
quote_table(index.prefix, index.table),
quote_name(index.prefix, index.table),
if_do(index.using, [" USING ", to_string(index.using)]),
?\s,
?(,
Expand All @@ -1239,7 +1239,7 @@ if Code.ensure_loaded?(Postgrex) do
]
]

queries ++ comments_on("INDEX", quote_table(index.prefix, index.name), index.comment)
queries ++ comments_on("INDEX", quote_name(index.prefix, index.name), index.comment)
end

def execute_ddl({command, %Index{} = index, mode}) when command in @drops do
Expand All @@ -1248,23 +1248,30 @@ if Code.ensure_loaded?(Postgrex) do
"DROP INDEX ",
if_do(index.concurrently, "CONCURRENTLY "),
if_do(command == :drop_if_exists, "IF EXISTS "),
quote_table(index.prefix, index.name),
quote_name(index.prefix, index.name),
drop_mode(mode)
]
]
end

def execute_ddl({:rename, %Index{} = current_index, new_name}) do
[["ALTER INDEX ", quote_name(current_index.name), " RENAME TO ", quote_name(new_name)]]
[
[
"ALTER INDEX ",
quote_name(current_index.prefix, current_index.name),
" RENAME TO ",
quote_name(new_name)
]
]
end

def execute_ddl({:rename, %Table{} = current_table, %Table{} = new_table}) do
[
[
"ALTER TABLE ",
quote_table(current_table.prefix, current_table.name),
quote_name(current_table.prefix, current_table.name),
" RENAME TO ",
quote_table(nil, new_table.name)
quote_name(nil, new_table.name)
]
]
end
Expand All @@ -1273,7 +1280,7 @@ if Code.ensure_loaded?(Postgrex) do
[
[
"ALTER TABLE ",
quote_table(table.prefix, table.name),
quote_name(table.prefix, table.name),
" RENAME ",
quote_name(current_column),
" TO ",
Expand All @@ -1283,7 +1290,7 @@ if Code.ensure_loaded?(Postgrex) do
end

def execute_ddl({:create, %Constraint{} = constraint}) do
table_name = quote_table(constraint.prefix, constraint.table)
table_name = quote_name(constraint.prefix, constraint.table)
queries = [["ALTER TABLE ", table_name, " ADD ", new_constraint_expr(constraint)]]

queries ++ comments_on("CONSTRAINT", constraint.name, constraint.comment, table_name)
Expand All @@ -1293,7 +1300,7 @@ if Code.ensure_loaded?(Postgrex) do
[
[
"ALTER TABLE ",
quote_table(constraint.prefix, constraint.table),
quote_name(constraint.prefix, constraint.table),
" DROP CONSTRAINT ",
if_do(command == :drop_if_exists, "IF EXISTS "),
quote_name(constraint.name),
Expand Down Expand Up @@ -1672,7 +1679,7 @@ if Code.ensure_loaded?(Postgrex) do
"FOREIGN KEY (",
quote_names(current_columns),
") REFERENCES ",
quote_table(ref.prefix || table.prefix, ref.table),
quote_name(ref.prefix || table.prefix, ref.table),
?(,
quote_names(reference_columns),
?),
Expand Down Expand Up @@ -1762,27 +1769,17 @@ if Code.ensure_loaded?(Postgrex) do
Enum.map_intersperse(names, ?,, &quote_name/1)
end

defp quote_name(nil, name), do: quote_name(name)

defp quote_name(prefix, name), do: [quote_name(prefix), ?., quote_name(name)]

defp quote_name(name) when is_atom(name) do
quote_name(Atom.to_string(name))
end

defp quote_name(name) when is_binary(name) do
if String.contains?(name, "\"") do
error!(nil, "bad literal/field/table name #{inspect(name)} (\" is not permitted)")
end

[?", name, ?"]
end

defp quote_table(nil, name), do: quote_table(name)
defp quote_table(prefix, name), do: [quote_table(prefix), ?., quote_table(name)]

defp quote_table(name) when is_atom(name),
do: quote_table(Atom.to_string(name))

defp quote_table(name) do
if String.contains?(name, "\"") do
error!(nil, "bad table name #{inspect(name)}")
error!(nil, "bad literal/field/index/table name #{inspect(name)} (\" is not permitted)")
end

[?", name, ?"]
Expand Down
2 changes: 1 addition & 1 deletion lib/ecto/migration/runner.ex
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ defmodule Ecto.Migration.Runner do
do: "drop index if exists #{quote_name(index.prefix, index.name)}#{drop_mode(mode)}"

defp command({:rename, %Index{} = index_current, new_name}),
do: "rename index #{quote_name(index_current.name)} to #{new_name}"
do: "rename index #{quote_name(index_current.prefix, index_current.name)} to #{new_name}"

defp command({:rename, %Table{} = current_table, %Table{} = new_table}),
do:
Expand Down
10 changes: 10 additions & 0 deletions test/ecto/adapters/postgres_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2726,6 +2726,16 @@ defmodule Ecto.Adapters.PostgresTest do
]
end

test "rename index with prefix" do
rename =
{:rename, index(:people, [:name], name: "persons_name_index", prefix: :foo),
"people_name_index"}

assert execute_ddl(rename) == [
~s|ALTER INDEX "foo"."persons_name_index" RENAME TO "people_name_index"|
]
end

test "create check constraint" do
create = {:create, constraint(:products, "price_must_be_positive", check: "price > 0")}

Expand Down