Skip to content

Commit

Permalink
Add with_ties/3 to Ecto.Query (#4090)
Browse files Browse the repository at this point in the history
  • Loading branch information
greg-rychlewski authored Feb 12, 2023
1 parent d739211 commit ab0771e
Showing 6 changed files with 215 additions and 16 deletions.
65 changes: 64 additions & 1 deletion lib/ecto/query.ex
Original file line number Diff line number Diff line change
@@ -435,6 +435,11 @@ defmodule Ecto.Query do
defstruct [recursive: false, queries: []]
end

defmodule LimitExpr do
@moduledoc false
defstruct [:expr, :file, :line, :with_ties, params: []]
end

defmodule Tagged do
@moduledoc false
# * value is the tagged value
@@ -961,6 +966,8 @@ defmodule Ecto.Query do
end
end

{t, quoted} = maybe_with_ties(type, t, quoted, binds)

from(t, env, count_bind, quoted, binds)
end

@@ -983,6 +990,10 @@ defmodule Ecto.Query do
from(t, env, count_bind, quoted, to_query_binds(binds))
end

defp from([{:with_ties, _value}|_], _env, _count_bind, _quoted, _binds) do
Builder.error! "`with_ties` keyword must immediately follow a limit"
end

defp from([{:on, _value}|_], _env, _count_bind, _quoted, _binds) do
Builder.error! "`on` keyword must immediately follow a join"
end
@@ -999,6 +1010,23 @@ defmodule Ecto.Query do
quoted
end

defp maybe_with_ties(:limit, t, quoted, binds) do
{t, with_ties} = collect_with_ties(t, nil)

quoted =
if with_ties != nil do
quote do
Ecto.Query.with_ties(unquote(quoted), unquote(binds), unquote(with_ties))
end
else
quoted
end

{t, quoted}
end

defp maybe_with_ties(_type, t, quoted, _binds), do: {t, quoted}

defp to_query_binds(binds) do
for {k, v} <- binds, do: {{k, [], nil}, v}
end
@@ -1013,6 +1041,13 @@ defmodule Ecto.Query do
defp join_qual(:left_lateral_join), do: :left_lateral
defp join_qual(:inner_lateral_join), do: :inner_lateral

defp collect_with_ties([{:with_ties, with_ties} | t], nil),
do: collect_with_ties(t, with_ties)
defp collect_with_ties([{:with_ties, _} | _], _),
do: Builder.error! "`with_ties` keyword was given more than once to the same limit"
defp collect_with_ties(t, with_ties),
do: {t, with_ties}

defp collect_on([{key, _} | _] = t, on, as, prefix, hints) when key in @from_join_opts do
{t, as, prefix, hints} = collect_as_and_prefix_and_hints(t, as, prefix, hints)
collect_on(t, on, as, prefix, hints)
@@ -1955,6 +1990,34 @@ defmodule Ecto.Query do
Builder.LimitOffset.build(:limit, query, binding, expr, __CALLER__)
end

@doc """
Enables or disables ties for limit expressions.
If there are multiple records tied for the last position in an ordered
limit result, setting this value to `true` will return all of the tied
records, even if the final result exceeds the specified limit.
Must be a boolean or evaluate to a boolean at runtime. Can only be applied
to queries with a `limit` expression or an error is raised. If `limit`
is redefined then `with_ties` must be reapplied.
Not all databases support this option and the ones that do might list it
under the `FETCH` command. Databases may require a corresponding `order_by`
statement to evaluate ties.
## Keywords example
from(p in Post, where: p.author_id == ^current_user, order_by: [desc: p.visits], limit: 10, with_ties: true)
## Expressions example
Post |> where([p], p.author_id == ^current_user) |> order_by([p], desc: p.visits) |> limit(10) |> with_ties(true)
"""
defmacro with_ties(query, binding \\ [], expr) do
Builder.LimitOffset.build(:with_ties, query, binding, expr, __CALLER__)
end

@doc """
An offset query expression.
@@ -2353,7 +2416,7 @@ defmodule Ecto.Query do
def last(queryable, key), do: last(order_by(queryable, ^key), nil)

defp limit do
%QueryExpr{expr: 1, params: [], file: __ENV__.file, line: __ENV__.line}
%LimitExpr{expr: 1, params: [], file: __ENV__.file, line: __ENV__.line}
end

defp field(ix, field) when is_integer(ix) and is_atom(field) do
89 changes: 79 additions & 10 deletions lib/ecto/query/builder/limit_offset.ex
Original file line number Diff line number Diff line change
@@ -5,39 +5,108 @@ defmodule Ecto.Query.Builder.LimitOffset do

alias Ecto.Query.Builder

@doc """
Validates `with_ties` at runtime.
"""
@spec with_ties!(any) :: boolean
def with_ties!(with_ties) when is_boolean(with_ties), do: with_ties

def with_ties!(with_ties),
do: raise("`with_ties` expression must evaluate to a boolean at runtime, got: `#{inspect(with_ties)}`")

@doc """
Builds a quoted expression.
The quoted expression should evaluate to a query at runtime.
If possible, it does all calculations at compile time to avoid
runtime work.
"""
@spec build(:limit | :offset, Macro.t, [Macro.t], Macro.t, Macro.Env.t) :: Macro.t
@spec build(:limit | :with_ties | :offset, Macro.t(), [Macro.t()], Macro.t(), Macro.Env.t()) ::
Macro.t()
def build(type, query, binding, expr, env) do
{query, binding} = Builder.escape_binding(query, binding, env)
{expr, {params, _acc}} = Builder.escape(expr, :integer, {[], %{}}, binding, env)
{query, vars} = Builder.escape_binding(query, binding, env)
{expr, {params, _acc}} = escape(type, expr, {[], %{}}, vars, env)
params = Builder.escape_params(params)
quoted = build_quoted(type, expr, params, env)

Builder.apply_query(query, __MODULE__, [type, quoted], env)
end

defp escape(type, expr, params_acc, vars, env) when type in [:limit, :offset] do
Builder.escape(expr, :integer, params_acc, vars, env)
end

defp escape(:with_ties, expr, params_acc, _vars, _env) when is_boolean(expr) do
{expr, params_acc}
end

defp escape(:with_ties, {:^, _, [expr]}, params_acc, _vars, _env) do
{quote(do: Ecto.Query.Builder.LimitOffset.with_ties!(unquote(expr))), params_acc}
end

defp escape(:with_ties, expr, _params_acc, _vars, _env) do
Builder.error!(
"`with_ties` expression must be a compile time boolean or an interpolated value using ^, got: `#{Macro.to_string(expr)}`"
)
end

limoff = quote do: %Ecto.Query.QueryExpr{
expr: unquote(expr),
params: unquote(params),
file: unquote(env.file),
line: unquote(env.line)}
defp build_quoted(:limit, expr, params, env) do
quote do: %Ecto.Query.LimitExpr{
expr: unquote(expr),
params: unquote(params),
with_ties: false,
file: unquote(env.file),
line: unquote(env.line)
}
end

Builder.apply_query(query, __MODULE__, [type, limoff], env)
defp build_quoted(:offset, expr, params, env) do
quote do: %Ecto.Query.QueryExpr{
expr: unquote(expr),
params: unquote(params),
file: unquote(env.file),
line: unquote(env.line)
}
end

defp build_quoted(:with_ties, expr, _params, _env), do: expr

@doc """
The callback applied by `build/4` to build the query.
"""
@spec apply(Ecto.Queryable.t, :limit | :offset, term) :: Ecto.Query.t
@spec apply(Ecto.Queryable.t(), :limit | :with_ties | :offset, term) :: Ecto.Query.t()
def apply(%Ecto.Query{} = query, :limit, expr) do
%{query | limit: expr}
end

def apply(%Ecto.Query{limit: limit} = query, :with_ties, expr) do
%{query | limit: apply_limit(limit, expr)}
end

def apply(%Ecto.Query{} = query, :offset, expr) do
%{query | offset: expr}
end

def apply(query, kind, expr) do
apply(Ecto.Queryable.to_query(query), kind, expr)
end

@doc """
Applies the `with_ties` value to the `limit` struct.
"""
def apply_limit(nil, _with_ties) do
Builder.error!("`with_ties` can only be applied to queries containing a `limit`")
end

# Runtime
def apply_limit(%_{} = limit, with_ties) do
%{limit | with_ties: with_ties}
end

# Compile
def apply_limit(limit, with_ties) do
quote do
Ecto.Query.Builder.LimitOffset.apply_limit(unquote(limit), unquote(with_ties))
end
end
end
14 changes: 12 additions & 2 deletions lib/ecto/query/inspect.ex
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Inspect.Algebra
import Kernel, except: [to_string: 1]

alias Ecto.Query.{DynamicExpr, JoinExpr, QueryExpr, WithExpr}
alias Ecto.Query.{DynamicExpr, JoinExpr, QueryExpr, WithExpr, LimitExpr}

defimpl Inspect, for: Ecto.Query.DynamicExpr do
def inspect(%DynamicExpr{binding: binding} = dynamic, opts) do
@@ -95,6 +95,7 @@ defimpl Inspect, for: Ecto.Query do
assocs = assocs(query.assocs, names)
windows = windows(query.windows, names)
combinations = combinations(query.combinations)
limit = limit(query.limit, names)

wheres = bool_exprs(%{and: :where, or: :or_where}, query.wheres, names)
group_bys = kw_exprs(:group_by, query.group_bys, names)
@@ -103,7 +104,6 @@ defimpl Inspect, for: Ecto.Query do
updates = kw_exprs(:update, query.updates, names)

lock = kw_inspect(:lock, query.lock)
limit = kw_expr(:limit, query.limit, names)
offset = kw_expr(:offset, query.offset, names)
select = kw_expr(:select, query.select, names)
distinct = kw_expr(:distinct, query.distinct, names)
@@ -191,6 +191,16 @@ defimpl Inspect, for: Ecto.Query do
Enum.map(combinations, fn {key, val} -> {key, "(" <> to_string(val) <> ")"} end)
end

defp limit(nil, _names), do: []

defp limit(%LimitExpr{with_ties: false} = limit, names) do
[{:limit, expr(limit, names)}]
end

defp limit(%LimitExpr{with_ties: with_ties} = limit, names) do
[{:limit, expr(limit, names)}] ++ kw_inspect(:with_ties, with_ties)
end

defp bool_exprs(keys, exprs, names) do
Enum.map(exprs, fn %{expr: expr, op: op} = part ->
{Map.fetch!(keys, op), expr(expr, names, part)}
3 changes: 2 additions & 1 deletion lib/ecto/query/planner.ex
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ defmodule Ecto.Query.Planner do
# Normalizes a query and its parameters.
@moduledoc false

alias Ecto.Query.{BooleanExpr, DynamicExpr, FromExpr, JoinExpr, QueryExpr, SelectExpr}
alias Ecto.Query.{BooleanExpr, DynamicExpr, FromExpr, JoinExpr, QueryExpr, SelectExpr, LimitExpr}

if map_size(%Ecto.Query{}) != 21 do
raise "Ecto.Query match out of date in builder"
@@ -773,6 +773,7 @@ defmodule Ecto.Query.Planner do
# Current strategy appends [{:subquery, i, cache}], where cache is the cache key for this subquery.
{op, expr, Enum.map(subqueries, fn %{cache: cache} -> {:subquery, cache} end)}
end
defp expr_to_cache(%LimitExpr{expr: expr, with_ties: with_ties}), do: {with_ties, expr}

@spec cast_and_merge_params(atom, Ecto.Query.t, any, list, module) :: {params :: list, cacheable? :: boolean}
defp cast_and_merge_params(kind, query, expr, params, adapter) do
54 changes: 54 additions & 0 deletions test/ecto/query/builder/limit_offset_test.exs
Original file line number Diff line number Diff line change
@@ -1,7 +1,18 @@
Code.require_file "../../../support/eval_helpers.exs", __DIR__

defmodule Ecto.Query.Builder.LimitOffsetTest do
use ExUnit.Case, async: true

import Ecto.Query
import Support.EvalHelpers

defmodule Post do
use Ecto.Schema

schema "posts" do
field :title
end
end

test "overrides on duplicated limit and offset" do
query = "posts" |> limit([], 1) |> limit([], 2)
@@ -10,4 +21,47 @@ defmodule Ecto.Query.Builder.LimitOffsetTest do
query = "posts" |> offset([], 1) |> offset([], 2) |> select([], 3)
assert query.offset.expr == 2
end

test "with_ties" do
# compile time query
query = from(p in Post) |> limit([], 1) |> with_ties(true)
assert query.limit.expr == 1
assert query.limit.with_ties == true

# runtime query
query = "posts" |> limit([], 2) |> with_ties(^true)
assert query.limit.expr == 2
assert query.limit.with_ties == true
end

test "with_ties is removed when a new limit is set" do
# compile time query
query = from(p in Post) |> limit([], 1) |> with_ties(true) |> limit([], 2)
assert query.limit.expr == 2
assert query.limit.with_ties == false

# runtime query
query = "posts" |> limit([], 3) |> with_ties(true) |> limit([], 4)
assert query.limit.expr == 4
assert query.limit.with_ties == false
end

test "with_ties must be a runtime or compile time boolean" do
msg = "`with_ties` expression must evaluate to a boolean at runtime, got: `1`"
assert_raise RuntimeError, msg, fn ->
with_ties("posts", ^1)
end

msg = "`with_ties` expression must be a compile time boolean or an interpolated value using ^, got: `1`"
assert_raise Ecto.Query.CompileError, msg, fn ->
quote_and_eval with_ties("posts", 1)
end
end

test "with_ties requires a limit" do
msg = "`with_ties` can only be applied to queries containing a `limit`"
assert_raise Ecto.Query.CompileError, msg, fn ->
with_ties("posts", true)
end
end
end
6 changes: 4 additions & 2 deletions test/ecto/query/planner_test.exs
Original file line number Diff line number Diff line change
@@ -476,13 +476,15 @@ defmodule Ecto.Query.PlannerTest do
on: true,
hints: ["join hint"],
prefix: "world",
limit: 1, with_ties: true,
preload: :comments
)

{_query, _cast_params, _dump_params, key} = plan(%{query | prefix: "foo"})
assert key == [:all,
{:lock, "foo"},
{:prefix, "foo"},
{:limit, {true, 1}},
{:where, [{:and, {:is_nil, [], [nil]}}, {:or, {:is_nil, [], [nil]}}]},
{:join, [{:inner, {"comments", Comment, 38292156, "world"}, true, ["join hint"]}]},
{:from, {"posts", Post, 32915161, "hello"}, ["hint"]},
@@ -749,7 +751,7 @@ defmodule Ecto.Query.PlannerTest do
:all,
{:prefix, "another"},
{:take, %{0 => {:any, [:id]}}},
{:limit, {:^, [], [0]}},
{:limit, {false, {:^, [], [0]}}},
{:order_by, [[desc: _]]},
{:from, {"comments", Comment, _, nil}, []},
{:select, {:&, [], [0]}}
@@ -781,7 +783,7 @@ defmodule Ecto.Query.PlannerTest do
:all,
{:prefix, "another"},
{:take, %{0 => {:any, [:id]}}},
{:limit, {:^, [], [0]}},
{:limit, {false, {:^, [], [0]}}},
{:order_by, [[desc: _]]},
{:from, {"comments", Comment, _, nil}, []},
{:select, {:&, [], [0]}}

0 comments on commit ab0771e

Please sign in to comment.