Skip to content

Commit 5750491

Browse files
authored
Fix issue with macro expansion in over clause's order_by (#4630)
1 parent 3637802 commit 5750491

File tree

2 files changed

+52
-2
lines changed

2 files changed

+52
-2
lines changed

lib/ecto/query/builder/order_by.ex

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,13 @@ defmodule Ecto.Query.Builder.OrderBy do
7575

7676
defp do_escape({dir, expr}, params_acc, kind, vars, env) do
7777
fun = &escape_expansion(kind, &1, &2, &3, &4, &5)
78-
{ast, params_acc} = Builder.escape(expr, :any, params_acc, vars, {env, fun})
78+
{ast, params_acc} = Builder.escape(expr, :any, params_acc, vars, {get_env(env), fun})
7979
{[{quoted_dir!(kind, dir), ast}], params_acc}
8080
end
8181

8282
defp do_escape(expr, params_acc, kind, vars, env) do
8383
fun = &escape_expansion(kind, &1, &2, &3, &4, &5)
84-
{ast, params_acc} = Builder.escape(expr, :any, params_acc, vars, {env, fun})
84+
{ast, params_acc} = Builder.escape(expr, :any, params_acc, vars, {get_env(env), fun})
8585

8686
if is_list(ast) do
8787
{ast, params_acc}
@@ -90,6 +90,9 @@ defmodule Ecto.Query.Builder.OrderBy do
9090
end
9191
end
9292

93+
defp get_env({env, _}), do: env
94+
defp get_env(env), do: env
95+
9396
defp escape_expansion(kind, expr, _type, params_acc, vars, env) when is_list(expr) do
9497
escape(kind, expr, params_acc, vars, env)
9598
end

test/ecto/query/builder/select_test.exs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,53 @@ defmodule Ecto.Query.Builder.SelectTest do
174174
assert %{alias: _} = query.select.aliases
175175
end
176176

177+
defmacro my_custom_field(p) do
178+
quote do
179+
fragment("lower(?)", unquote(p).title)
180+
end
181+
end
182+
183+
defmacro my_complex_order(p) do
184+
quote do
185+
[desc: unquote(p).id, asc: my_custom_field(unquote(p)), asc: nth_value(unquote(p).links, 1)]
186+
end
187+
end
188+
189+
test "supports macro expansion in over/2" do
190+
query = from p in "posts", select: %{row_number: over(row_number(), order_by: [desc: my_custom_field(p)])}
191+
192+
assert {:%{}, [],
193+
[
194+
row_number:
195+
{:over, [],
196+
[
197+
{:row_number, [], []},
198+
[
199+
order_by: [
200+
desc: {:fragment, [], [raw: "lower(", expr: _, raw: ")"]}
201+
]
202+
]
203+
]}
204+
]} = query.select.expr
205+
206+
query = from p in "posts", select: %{row_number: over(row_number(), order_by: my_complex_order(p))}
207+
assert {:%{}, [],
208+
[
209+
row_number:
210+
{:over, [],
211+
[
212+
{:row_number, [], []},
213+
[
214+
order_by: [
215+
desc: _,
216+
asc: {:fragment, [], [raw: "lower(", expr: _, raw: ")"]},
217+
asc: {:nth_value, [], _}
218+
]
219+
]
220+
]}
221+
]} = query.select.expr
222+
end
223+
177224
test "raises if name given to selected_as/2 is not an atom" do
178225
message = "expected literal atom or interpolated value in selected_as/2, got: `\"ident\"`"
179226

0 commit comments

Comments
 (0)