Repository: elixir-ecto/ecto
Pull request #4266 (2023-08-19 23:55:45)

Update changelog
As an aside:

I'm not too sure what the plans are for releasing 3.11. But I was wondering if it could wait a little bit for a couple things:

1. To see if I can come up with a decent proposal for the derived fields
2. I believe I thought of a simple way to support `values`. The other PR has stalled for over a year so I thought maybe I could submit something if my idea works out.

josevalim: @greg-rychlewski I don't think there is any pressure for releasing, so take your time. @whatyouhide may want a release, but they are still working on improving the adapter.

Also, I would be very glad to give you owner access on hex.pm if you want to publish the next releases. Just let me know your hex.pm email (you can email it to me). :)

In terms of features, the feature I see most asked is automatic migrations support, as in Django: https://docs.djangoproject.com/en/4.2/topics/migrations/#workflow - so if you are interested in bringing the projects forward, it may be something worth working on. It would be added to EctoSQL and it would require at least two new callbacks in SQL adapters: one for listing all fields for a table and another that checks the field compatibility (the ecto field with the db field).
greg-rychlewski: > Also, I would be very glad to give you owner access on hex.pm if you want to publish the next releases. Just let me know your hex.pm email (you can email it to me). :)

Oh thanks very much. I will send you an email. 

> In terms of features, the feature I see most asked is automatic migrations support, as in Django: https://docs.djangoproject.com/en/4.2/topics/migrations/#workflow - so if you are interested in bringing the projects forward, it may be something worth working on. It would be added to EctoSQL and it would require at least two new callbacks in SQL adapters: one for listing all fields for a table and another that checks the field compatibility (the ecto field with the db field).

This looks interesting I will give it a shot!

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d1551b97..311d59c4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,9 +2,33 @@
 
 ## v3.11.0-dev
 
+### Enhancements
+
+  * [Ecto.Association] Allow `preload_order` to take MFAs for `many_to_many` associations. This allows ordering by the join table
+  * [Ecto.Query] Add `:operation` option to `with_cte/3`. This allows CTEs to perform updates and deletes
+  * [Ecto.Query] Support `splice(^...)` in `fragment`
+  * [Ecto.Query] Add `prepend_order_by/3`
+  * [Ecto.Query] Allow `selected_as/1` and `selected_as/2` to take interpolated names
+  * [Ecto.Query] Allow map update syntax to work with `nil` values in `select`
+  * [Ecto.Query] Allow hints to inject SQL using `unsafe_fragment`
+  * [Ecto.Repo] Add `:on_preload_spawn` option to `preload/3`
+  * [Ecto.Schema] Support `:load_in_query` option for embeds
+
+### Bug fixes
+
+  * [Ecto.Association] Ensure parent prefix is passed to `on_delete` queries
+  * [Ecto.Changeset] Ensure duplicate primary keys are always detected for embeds
+  * [Ecto.Embedded] Raise `ArgumentError` when specifying an autogenerated `:id` primary key
+  * [Ecto.Query] Ensure subquery selects generate unique cache keys
+
 ### Adapter changes
 
   * Adapters now receive `nil` for encoding/decoding
+  * Adapters now receive `type` instead of `{:maybe, type}` as the first argument to `loaders/2`
+
+### Deprecations
+
+  * [Ecto.Query] Keyword hints are no longer supported. Please use `unsafe_fragment` inside of hints instead
 
 ## v3.10.3 (2023-07-07)
 
diff --git a/lib/ecto/query.ex b/lib/ecto/query.ex
index 1a52be65..311efdb1 100644
--- a/lib/ecto/query.ex
+++ b/lib/ecto/query.ex
@@ -736,7 +736,7 @@ defmodule Ecto.Query do
 
       windows: [ordered_names: [order_by: e.name]]
 
-  It works exactly as the keyword query version of `order_by/4`.
+  It works exactly as the keyword query version of `order_by/3`.
 
   ### :frame
 
@@ -1809,20 +1809,12 @@ defmodule Ecto.Query do
   "nulls first" or "nulls last" is specific to each database implementation.
 
   `order_by` may be invoked or listed in a query many times. New expressions
-  can be appended or preprended to the existing ones. The behaviour is controlled
-  through the `:mode` option. By default, new expressions are appended.
+  are appended to the existing ones.
 
   `order_by` also accepts a list of atoms where each atom refers to a field in
   source or a keyword list where the direction is given as key and the field
   to order as value.
 
-  ## Options
-
-    * `:mode` - where to place the order expression relative to the
-      ones that already exist. Can be `:append`, to place it after the
-      current orderings, or `:prepend`, to place it before the current
-      orderings. Defaults to `:append`
-
   ## Keywords examples
 
       from(c in City, order_by: c.name, order_by: c.population)
@@ -1871,8 +1863,15 @@ defmodule Ecto.Query do
       City |> order_by(^order_by_param) # Keyword list
 
   """
-  defmacro order_by(query, binding \\ [], expr, opts \\ []) do
-    Builder.OrderBy.build(query, binding, expr, opts, __CALLER__)
+  defmacro order_by(query, binding \\ [], expr) do
+    Builder.OrderBy.build(query, binding, expr, :append, __CALLER__)
+  end
+
+  @doc """
+  Same as `order_by/3` except new expressions will be prepended to existing ones.
+  """
+  defmacro prepend_order_by(query, binding \\ [], expr) do
+    Builder.OrderBy.build(query, binding, expr, :prepend, __CALLER__)
   end
 
   @doc """
diff --git a/lib/ecto/query/api.ex b/lib/ecto/query/api.ex
index dac1fa49..a9a14b8e 100644
--- a/lib/ecto/query/api.ex
+++ b/lib/ecto/query/api.ex
@@ -768,7 +768,7 @@ defmodule Ecto.Query.API do
   referencing it outside of select statements.
 
   This comes in handy when, for instance, you would like to use the calculated
-  value in `Ecto.Query.group_by/3` or `Ecto.Query.order_by/4`:
+  value in `Ecto.Query.group_by/3` or `Ecto.Query.order_by/3`:
 
       from p in Post,
         select: %{
diff --git a/lib/ecto/query/builder/order_by.ex b/lib/ecto/query/builder/order_by.ex
index f46b5f35..5d863344 100644
--- a/lib/ecto/query/builder/order_by.ex
+++ b/lib/ecto/query/builder/order_by.ex
@@ -14,9 +14,6 @@ defmodule Ecto.Query.Builder.OrderBy do
     :desc_nulls_first
   ]
 
-  @modes [:append, :prepend]
-  @default_mode :append
-
   @doc """
   Returns `true` if term is a valid order_by direction; otherwise returns `false`.
 
@@ -131,23 +128,6 @@ defmodule Ecto.Query.Builder.OrderBy do
 
   defp to_field(field), do: {{:., [], [{:&, [], [0]}, field]}, [], []}
 
-  @doc """
-  Called at runtime to verify the ordering mode
-  """
-  def mode!(order_opts) do
-    case order_opts[:mode] do
-      nil ->
-        @default_mode
-
-      mode when mode in @modes ->
-        mode
-
-      other ->
-        raise ArgumentError,
-              "expected `:mode` to be `:append` or `:prepend`, got: #{inspect(other)}"
-    end
-  end
-
   @doc """
   Shared between order_by and distinct.
   """
@@ -168,11 +148,10 @@ defmodule Ecto.Query.Builder.OrderBy do
   @doc """
   Called at runtime to assemble order_by.
   """
-  def order_by!(query, exprs, order_opts, file, line) do
-    mode = mode!(order_opts)
+  def order_by!(query, exprs, op, file, line) do
     {expr, params} = order_by_or_distinct!(:order_by, query, exprs, [])
     expr = %Ecto.Query.QueryExpr{expr: expr, params: Enum.reverse(params), line: line, file: file}
-    apply(query, expr, mode)
+    apply(query, expr, op)
   end
 
   defp dynamic_or_field!(kind, %Ecto.Query.DynamicExpr{} = dynamic, query, {params, count}) do
@@ -197,36 +176,35 @@ defmodule Ecto.Query.Builder.OrderBy do
   If possible, it does all calculations at compile time to avoid
   runtime work.
   """
-  @spec build(Macro.t, [Macro.t], Macro.t, Macro.t, Macro.Env.t) :: Macro.t
-  def build(query, _binding, {:^, _, [var]}, order_opts, env) do
+  @spec build(Macro.t, [Macro.t], Macro.t, :append | :prepend, Macro.Env.t) :: Macro.t
+  def build(query, _binding, {:^, _, [var]}, op, env) do
     quote do
-      Ecto.Query.Builder.OrderBy.order_by!(unquote(query), unquote(var), unquote(order_opts), unquote(env.file), unquote(env.line))
+      Ecto.Query.Builder.OrderBy.order_by!(unquote(query), unquote(var), unquote(op), unquote(env.file), unquote(env.line))
     end
   end
 
-  def build(query, binding, expr, order_opts, env) do
+  def build(query, binding, expr, op, env) do
     {query, binding} = Builder.escape_binding(query, binding, env)
     {expr, {params, _acc}} = escape(:order_by, expr, {[], %{}}, binding, env)
     params = Builder.escape_params(params)
-    mode = quote do: Ecto.Query.Builder.OrderBy.mode!(unquote(order_opts))
 
     order_by = quote do: %Ecto.Query.QueryExpr{
                            expr: unquote(expr),
                            params: unquote(params),
                            file: unquote(env.file),
                            line: unquote(env.line)}
-    Builder.apply_query(query, __MODULE__, [order_by, mode], env)
+    Builder.apply_query(query, __MODULE__, [order_by, op], env)
   end
 
   @doc """
   The callback applied by `build/4` to build the query.
   """
   @spec apply(Ecto.Queryable.t, term, term) :: Ecto.Query.t
-  def apply(%Ecto.Query{order_bys: orders} = query, expr, mode) do
-    %{query | order_bys: update_order_bys(orders, expr, mode)}
+  def apply(%Ecto.Query{order_bys: orders} = query, expr, op) do
+    %{query | order_bys: update_order_bys(orders, expr, op)}
   end
-  def apply(query, expr, mode) do
-    apply(Ecto.Queryable.to_query(query), expr, mode)
+  def apply(query, expr, op) do
+    apply(Ecto.Queryable.to_query(query), expr, op)
   end
 
   @doc """
diff --git a/lib/ecto/repo/preloader.ex b/lib/ecto/repo/preloader.ex
index 8ca2e28e..fd8ed695 100644
--- a/lib/ecto/repo/preloader.ex
+++ b/lib/ecto/repo/preloader.ex
@@ -340,7 +340,7 @@ defmodule Ecto.Repo.Preloader do
   defp add_preload_order([], query), do: query
 
   defp add_preload_order(order, query) when is_list(order) do
-    Ecto.Query.order_by(query, [q], ^order, mode: :prepend)
+    Ecto.Query.prepend_order_by(query, [q], ^order)
   end
 
   defp add_preload_order({m, f, a}, query) do
diff --git a/lib/ecto/schema.ex b/lib/ecto/schema.ex
index a47dd1c0..d846c3e4 100644
--- a/lib/ecto/schema.ex
+++ b/lib/ecto/schema.ex
@@ -831,7 +831,7 @@ defmodule Ecto.Schema do
       For example, if you set `Post.has_many :comments, preload_order: [asc: :content]`,
       whenever the `:comments` associations is preloaded,
       the comments will be order by the `:content` field.
-      See `Ecto.Query.order_by/4` for more examples.
+      See `Ecto.Query.order_by/3` for more examples.
 
   ## Examples
 
@@ -1371,7 +1371,7 @@ defmodule Ecto.Schema do
 
     * `:preload_order` - Sets the default `order_by` when preloading the association.
       It may be a keyword list/list of fields or an MFA tuple, such as `{Mod, fun, []}`.
-      Both cases must resolve to a valid `order_by` expression. See `Ecto.Query.order_by/4`
+      Both cases must resolve to a valid `order_by` expression. See `Ecto.Query.order_by/3`
       to learn more about about ordering expressions.
       See the [preload order](#many_to_many/3-preload-order) section below to learn how
       this option can be utilized


Test patch:
diff --git a/test/ecto/query/builder/order_by_test.exs b/test/ecto/query/builder/order_by_test.exs
index 333c6a05..81656994 100644
--- a/test/ecto/query/builder/order_by_test.exs
+++ b/test/ecto/query/builder/order_by_test.exs
@@ -89,34 +89,14 @@ defmodule Ecto.Query.Builder.OrderByTest do
       end
     end
 
-    test "accepts :mode option" do
+    test "prepend_order_by" do
       query = order_by("q", [q], [:title])
-      opts = [mode: :prepend]
-      %{order_bys: prepend_order_bys} = order_by(query, [q], [:prepend], opts)
-      %{order_bys: append_order_bys} = order_by(query, [q], [:append], mode: :append)
-      %{order_bys: default_order_bys} = order_by(query, [q], [:append])
+      %{order_bys: order_bys} = prepend_order_by(query, [q], [:prepend])
 
       assert [
                %{expr: [asc: {{:., _, [_, :prepend]}, _, _}]},
                %{expr: [asc: {{:., _, [_, :title]}, _, _}]}
-             ] = prepend_order_bys
-
-      assert [
-               %{expr: [asc: {{:., _, [_, :title]}, _, _}]},
-               %{expr: [asc: {{:., _, [_, :append]}, _, _}]}
-             ] = append_order_bys
-
-      assert [
-               %{expr: [asc: {{:., _, [_, :title]}, _, _}]},
-               %{expr: [asc: {{:., _, [_, :append]}, _, _}]}
-             ] = default_order_bys
-
-
-      error_msg = "expected `:mode` to be `:append` or `:prepend`, got: :invalid"
-
-      assert_raise ArgumentError, error_msg, fn ->
-        order_by(query, [q], [:invalid], mode: :invalid)
-      end
+             ] = order_bys
     end
   end
 
@@ -130,36 +110,6 @@ defmodule Ecto.Query.Builder.OrderByTest do
       assert order_by("q", [q], [{^dir, ^key}]).order_bys == order_by("q", [q], [desc: q.title]).order_bys
     end
 
-    test "accepts :mode option" do
-      query = order_by("q", [q], [:title])
-      opts = [mode: :prepend]
-      %{order_bys: prepend_order_bys} = order_by(query, [q], ^[:prepend], opts)
-      %{order_bys: append_order_bys} = order_by(query, [q], ^[:append], mode: :append)
-      %{order_bys: default_order_bys} = order_by(query, [q], ^[:append])
-
-      assert [
-               %{expr: [asc: {{:., _, [_, :prepend]}, _, _}]},
-               %{expr: [asc: {{:., _, [_, :title]}, _, _}]}
-             ] = prepend_order_bys
-
-      assert [
-               %{expr: [asc: {{:., _, [_, :title]}, _, _}]},
-               %{expr: [asc: {{:., _, [_, :append]}, _, _}]}
-             ] = append_order_bys
-
-      assert [
-               %{expr: [asc: {{:., _, [_, :title]}, _, _}]},
-               %{expr: [asc: {{:., _, [_, :append]}, _, _}]}
-             ] = default_order_bys
-
-
-      error_msg = "expected `:mode` to be `:append` or `:prepend`, got: :invalid"
-
-      assert_raise ArgumentError, error_msg, fn ->
-        order_by(query, [q], ^[:invalid], mode: :invalid)
-      end
-    end
-
     test "supports dynamic expressions" do
       order_by = [
         asc: dynamic([p], p.foo == ^1 and p.bar == ^"bar"),