Add support for index directions

As discussed [on the mailing list], this PR add support for specifying the index
direction when creating a new index.

[on the mailing list]:
https://groups.google.com/g/elixir-ecto/c/uYFE2evwsW4/m/2q5QwQH2BAAJ

Most of the changes are pretty straightforward, but in the Tds and Postgres
adapters, I did have to split up the old `index_expr/1` into two different
functions: one to compute the index expression and one to compute the include
expression. Previously they shared the same function, but now that the domains
of the functions are different I don't think they should.

I also added two more types to `Ecto.Migration` to make sure the type of the
`columns` field is correct.

Finally, I chose to ignore the sorting direction when generating the index name,
so that functionality also had to be slightly amended.

Author: Munksgaard

lib/ecto/adapters/myxql/connection.ex

@@ -1312,11 +1312,34 @@ if Code.ensure_loaded?(MyXQL) do
     defp default_expr(:error),
       do: []
 
+    defp index_expr({dir, literal})
+         when is_binary(literal),
+         do: index_dir(dir, literal)
+
+    defp index_expr({dir, literal}),
+      do: index_dir(dir, quote_name(literal))
+
     defp index_expr(literal) when is_binary(literal),
       do: literal
 
     defp index_expr(literal), do: quote_name(literal)
 
+    defp index_dir(dir, str)
+         when dir in [
+                :asc,
+                :asc_nulls_first,
+                :asc_nulls_last,
+                :desc,
+                :desc_nulls_first,
+                :desc_nulls_last
+              ] do
+      case dir do
+        :asc -> [str | " ASC"]
+        :desc -> [str | " DESC"]
+        _ -> error!(nil, "#{dir} is not supported in indexes in MySQL")
+      end
+    end
+
     defp engine_expr(storage_engine),
       do: [" ENGINE = ", String.upcase(to_string(storage_engine || "INNODB"))]
 

lib/ecto/adapters/postgres/connection.ex

@@ -1305,7 +1305,7 @@ if Code.ensure_loaded?(Postgrex) do
 
     def execute_ddl({command, %Index{} = index}) when command in @creates do
       fields = Enum.map_intersperse(index.columns, ", ", &index_expr/1)
-      include_fields = Enum.map_intersperse(index.include, ", ", &index_expr/1)
+      include_fields = Enum.map_intersperse(index.include, ", ", &include_expr/1)
 
       maybe_nulls_distinct =
         case index.nulls_distinct do
@@ -1704,12 +1704,43 @@ if Code.ensure_loaded?(Postgrex) do
             ":default may be a string, number, boolean, list of strings, list of integers, map (when type is Map), or a fragment(...)"
         )
 
+    defp index_expr({dir, literal}) when is_binary(literal),
+      do: index_dir(dir, literal)
+
+    defp index_expr({dir, literal}),
+      do: index_dir(dir, quote_name(literal))
+
     defp index_expr(literal) when is_binary(literal),
       do: literal
 
     defp index_expr(literal),
       do: quote_name(literal)
 
+    defp index_dir(dir, str)
+         when dir in [
+                :asc,
+                :asc_nulls_first,
+                :asc_nulls_last,
+                :desc,
+                :desc_nulls_first,
+                :desc_nulls_last
+              ] do
+      case dir do
+        :asc -> [str | " ASC"]
+        :asc_nulls_first -> [str | " ASC NULLS FIRST"]
+        :asc_nulls_last -> [str | " ASC NULLS LAST"]
+        :desc -> [str | " DESC"]
+        :desc_nulls_first -> [str | " DESC NULLS FIRST"]
+        :desc_nulls_last -> [str | " DESC NULLS LAST"]
+      end
+    end
+
+    defp include_expr(literal) when is_binary(literal),
+      do: literal
+
+    defp include_expr(literal),
+      do: quote_name(literal)
+
     defp options_expr(nil),
       do: []
 

lib/ecto/adapters/tds/connection.ex

@@ -1196,7 +1196,7 @@ if Code.ensure_loaded?(Tds) do
       include =
         index.include
         |> List.wrap()
-        |> Enum.map_intersperse(", ", &index_expr/1)
+        |> Enum.map_intersperse(", ", &include_expr/1)
 
       [
         [
@@ -1570,9 +1570,35 @@ if Code.ensure_loaded?(Tds) do
     defp constraint_name(type, table, name),
       do: quote_name("#{type}_#{table.prefix}_#{table.name}_#{name}")
 
+    defp index_expr({dir, literal})
+         when is_binary(literal),
+         do: index_dir(dir, literal)
+
+    defp index_expr({dir, literal}),
+      do: index_dir(dir, quote_name(literal))
+
     defp index_expr(literal) when is_binary(literal), do: literal
     defp index_expr(literal), do: quote_name(literal)
 
+    defp index_dir(dir, str)
+         when dir in [
+                :asc,
+                :asc_nulls_first,
+                :asc_nulls_last,
+                :desc,
+                :desc_nulls_first,
+                :desc_nulls_last
+              ] do
+      case dir do
+        :asc -> [str | " ASC"]
+        :desc -> [str | " DESC"]
+        _ -> error!(nil, "#{dir} is not supported in indexes in Tds adapter")
+      end
+    end
+
+    defp include_expr(literal) when is_binary(literal), do: literal
+    defp include_expr(literal), do: quote_name(literal)
+
     defp engine_expr(_storage_engine), do: [""]
 
     defp options_expr(nil), do: []

lib/ecto/migration.ex

@@ -393,11 +393,15 @@ defmodule Ecto.Migration do
               comment: nil,
               options: nil
 
+    @type column :: atom | String.t() | {index_dir(), atom | String.t()}
+
+    @type index_dir :: :asc_nulls_first | :asc_nulls_last | :desc_nulls_first | :desc_nulls_last
+
     @type t :: %__MODULE__{
             table: String.t(),
             prefix: String.t() | nil,
             name: String.t() | atom,
-            columns: [atom | String.t()],
+            columns: [column()],
             unique: boolean,
             concurrently: boolean,
             using: atom | String.t(),
@@ -891,6 +895,24 @@ defmodule Ecto.Migration do
       create index("products", [:sku, :category_id], unique: true)
       create index("products", [:sku], unique: true, where: "category_id IS NULL")
 
+  ## Sorting direction
+
+  You can specify the sorting direction of the index by using a keyword list:
+
+      create index("products", [desc: sku])
+
+  The following keywords are supported:
+
+    * `:asc`
+    * `:asc_nulls_last`
+    * `:asc_nulls_first`
+    * `:desc`
+    * `:desc_nulls_last`
+    * `:desc_nulls_first`
+
+  The `*_nulls_first` and `*_nulls_last` variants are not supported by all
+  databases.
+
   ## Examples
 
       # With no name provided, the name of the below index defaults to
@@ -957,18 +979,19 @@ defmodule Ecto.Migration do
   defp default_index_name(index) do
     [index.table, index.columns, "index"]
     |> List.flatten()
-    |> Enum.map_join(
-      "_",
-      fn item ->
-        item
-        |> to_string()
-        |> String.replace(~r"[^\w]", "_")
-        |> String.replace_trailing("_", "")
-      end
-    )
+    |> Enum.map_join("_", &column_name/1)
     |> String.to_atom()
   end
 
+  defp column_name({_dir, column}), do: column_name(column)
+
+  defp column_name(column) do
+    column
+    |> to_string()
+    |> String.replace(~r"[^\w]", "_")
+    |> String.replace_trailing("_", "")
+  end
+
   @doc """
   Executes arbitrary SQL, anonymous function or a keyword command.
 

test/ecto/adapters/myxql_test.exs

@@ -2147,6 +2147,25 @@ defmodule Ecto.Adapters.MyXQLTest do
              ]
   end
 
+  test "create index with direction" do
+    create =
+      {:create, index(:posts, [:category_id, desc: :permalink])}
+
+    assert execute_ddl(create) ==
+             [
+               ~s|CREATE INDEX `posts_category_id_permalink_index` ON `posts` (`category_id`, `permalink` DESC)|
+             ]
+  end
+
+  test "create index with invalid direction" do
+    create =
+      {:create, index(:posts, [:category_id, asc_nulls_first: :permalink])}
+
+    assert_raise ArgumentError, "asc_nulls_first is not supported in indexes in MySQL", fn ->
+      execute_ddl(create)
+    end
+  end
+
   test "create unique index" do
     create = {:create, index(:posts, [:permalink], unique: true)}
 

test/ecto/adapters/postgres_test.exs

@@ -2700,6 +2700,16 @@ defmodule Ecto.Adapters.PostgresTest do
            ]
   end
 
+  test "create index with direction" do
+    create =
+      {:create, index(:posts, [:category_id, desc_nulls_last: :permalink])}
+
+    assert execute_ddl(create) ==
+             [
+               ~s|CREATE INDEX "posts_category_id_permalink_index" ON "posts" ("category_id", "permalink" DESC NULLS LAST)|
+             ]
+  end
+
   test "create unique index" do
     create = {:create, index(:posts, [:permalink], unique: true)}
 

test/ecto/adapters/tds_test.exs

@@ -1856,6 +1856,27 @@ defmodule Ecto.Adapters.TdsTest do
              [~s|CREATE INDEX [posts$main] ON [posts] ([permalink]) WITH(ONLINE=ON);|]
   end
 
+  test "create index with direction" do
+    create =
+      {:create, index(:posts, [:category_id, desc: :permalink])}
+
+    assert execute_ddl(create) ==
+             [
+               ~s|CREATE INDEX [posts_category_id_permalink_index] ON [posts] ([category_id], [permalink] DESC);|
+             ]
+  end
+
+  test "create index with invalid direction" do
+    create =
+      {:create, index(:posts, [:category_id, asc_nulls_first: :permalink])}
+
+    assert_raise ArgumentError,
+                 "asc_nulls_first is not supported in indexes in Tds adapter",
+                 fn ->
+                   execute_ddl(create)
+                 end
+  end
+
   test "create unique index" do
     create = {:create, index(:posts, [:permalink], unique: true)}
 

test/ecto/migration_test.exs

@@ -592,6 +592,12 @@ defmodule Ecto.MigrationTest do
       assert {:create, %Index{}} = last_command()
     end
 
+    test "creates an index with desc_nulls_last" do
+      create index(:posts, desc_nulls_last: :title)
+      flush()
+      assert {:create, %Index{}} = last_command()
+    end
+
     test "creates a check constraint" do
       create constraint(:posts, :price, check: "price > 0")
       flush()