Fix macros inside of order_by #4578
Conversation
|
Hello, Unfortunately, it seems that this PR may have introduced a regression. Given the following setup: defmodule MyQueryUtils do
defmacro foo(table) do
quote(do: unquote(table).priority > 0)
end
end
defmodule Post do
use Ecto.Schema
@primary_key false
schema "posts" do
field :title, :string
field :priority, :integer
end
endThis query no longer compiles: import MyQueryUtils
import Ecto.Query
select(Post, [o], %{row_number: over(row_number(), order_by: [desc: foo(o)])})With the following error: A possible fix could be to handle this specific case here by adding: defp do_escape(expr, params_acc, kind, vars, {env, _}) do
do_escape(expr, params_acc, kind, vars, env)
endLet me know what you think, I’d be happy to help test and open a pull request for this. Thanks again! Edit: I've simplified the test query |
|
@Gigitsu Thanks for finding this. I think it might be a bit safer to restrict it to the places where we are setting But besides that what you said looks like the right fix to me! |
I’m not sure I’ve understood correctly. Do you mean something like this? defp do_escape({dir, expr}, params_acc, kind, vars, env) do
env = with {env, _} <- env, do: env
fun = &escape_expansion(kind, &1, &2, &3, &4, &5)
{ast, params_acc} = Builder.escape(expr, :any, params_acc, vars, {env, fun})
{[{quoted_dir!(kind, dir), ast}], params_acc}
end
defp do_escape(expr, params_acc, kind, vars, env) do
env = with {env, _} <- env, do: env
fun = &escape_expansion(kind, &1, &2, &3, &4, &5)
{ast, params_acc} = Builder.escape(expr, :any, params_acc, vars, {env, fun})
if is_list(ast) do
{ast, params_acc}
else
{[{:asc, ast}], params_acc}
end
end |
|
@Gigitsu Yeah basically that. In other places like the main builder function we define a defp get_env({env, _}), do: env
defp get_env(env), do: env |
|
@greg-rychlewski I've created the #4630 pull request. I'm unsure about the test setup. Specifically, I'm not confident whether importing macros from If you agree, we can move the discussion to the pull request |
Closes #4575