Skip to content

Commit

Permalink
[patch] Fixed binded parameters in limits and offsets
Browse files Browse the repository at this point in the history
  • Loading branch information
Ygor Castor committed Jul 19, 2023
1 parent e7c606a commit 7b36421
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 34 deletions.
26 changes: 22 additions & 4 deletions lib/ravix_ecto/parsers/query_params.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ defmodule Ravix.Ecto.Parser.QueryParams do

alias Ecto.Query, as: EctoQuery

def parse(%EctoQuery{wheres: wheres} = query, params, pk) do
def parse_conditions(%EctoQuery{wheres: wheres} = query, params, pk) do
wheres
|> Enum.map(fn %EctoQuery.BooleanExpr{expr: expr} ->
pair(expr, params, pk, query, "where clause")
Expand All @@ -13,16 +13,26 @@ defmodule Ravix.Ecto.Parser.QueryParams do
|> map_unless_empty
end

def parse([{_, _} | _] = fields, keys, pk) do
def parse_conditions([{_, _} | _] = fields, keys, pk) do
fields
|> Keyword.take(keys)
|> parse(pk)
|> parse_conditions(pk)
end

def parse(filter, pk) do
def parse_conditions(filter, pk) do
filter |> value(pk, "where clause") |> map_unless_empty
end

def parse_limit_and_offset(%EctoQuery{limit: limit, offset: offset}, params) do
case limit == nil and offset == nil do
true ->
nil

false ->
[limit: offset_limit(limit, params), offset: offset_limit(offset, params)]
end
end

def parse_update(%EctoQuery{updates: updates} = query, params, pk) do
updates
|> Enum.flat_map(fn %EctoQuery.QueryExpr{expr: expr} ->
Expand All @@ -32,6 +42,14 @@ defmodule Ravix.Ecto.Parser.QueryParams do
|> merge_keys(query, "update clause")
end

defp offset_limit(nil, _), do: 0

defp offset_limit(%{expr: {:^, [], [pos]}}, params),
do: elem(params, pos)

defp offset_limit(%{expr: value}, _),
do: value

defp merge_keys(keyword, query, place) do
Enum.reduce(keyword, %{}, fn {key, value}, acc ->
Map.update(acc, key, value, fn
Expand Down
63 changes: 33 additions & 30 deletions lib/ravix_ecto/parsers/query_parser.ex
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ defmodule Ravix.Ecto.Parser.QueryParser do

{coll, model, raven_query, pk} = from(ecto_query)
params = List.to_tuple(params)
query_params = QueryParams.parse(ecto_query, params, pk)
query_conditions = QueryParams.parse_conditions(ecto_query, params, pk)
limit_params = QueryParams.parse_limit_and_offset(ecto_query, params)

case Projection.project(ecto_query, params, {coll, model, pk}) do
{:find, projection, fields} ->
Expand All @@ -28,7 +29,15 @@ defmodule Ravix.Ecto.Parser.QueryParser do
fields: fields,
pk: pk,
raven_query:
find_all_query(raven_query, ecto_query, query_params, projection, pk, model)
find_all_query(
raven_query,
ecto_query,
query_conditions,
limit_params,
projection,
pk,
model
)
}
end
end
Expand Down Expand Up @@ -167,7 +176,7 @@ defmodule Ravix.Ecto.Parser.QueryParser do

{_coll, _model, raven_query, pk} = from(ecto_query)
params = List.to_tuple(params)
query_params = QueryParams.parse(ecto_query, params, pk)
query_params = QueryParams.parse_conditions(ecto_query, params, pk)

%QueryInfo{
kind: :delete,
Expand All @@ -181,7 +190,7 @@ defmodule Ravix.Ecto.Parser.QueryParser do

{_coll, _model, raven_query, pk} = from(ecto_query)
params = List.to_tuple(params)
query_params = QueryParams.parse(ecto_query, params, pk)
query_params = QueryParams.parse_conditions(ecto_query, params, pk)
updates = QueryParams.parse_update(ecto_query, params, pk)

%QueryInfo{
Expand All @@ -191,14 +200,22 @@ defmodule Ravix.Ecto.Parser.QueryParser do
}
end

defp find_all_query(raven_query, ecto_query, query_params, projection, pk, model) do
defp find_all_query(
raven_query,
ecto_query,
query_conditions,
limit_params,
projection,
pk,
model
) do
raven_query
|> append_conditions(query_params)
|> append_conditions(query_conditions)
|> append_default_where_if_missing()
|> parse_projections(projection)
|> parse_order(ecto_query, model, pk)
|> parse_grouping(ecto_query, pk)
|> limit_skip(ecto_query, query_params, pk)
|> limit_skip(limit_params)
end

defp delete_all_query(raven_query, query_params) do
Expand Down Expand Up @@ -352,30 +369,16 @@ defmodule Ravix.Ecto.Parser.QueryParser do

defp limit_skip(
%RavenQuery{} = raven_query,
%EctoQuery{limit: limit, offset: offset} = query,
params,
pk
) do
case limit == nil and offset == nil do
true ->
raven_query

false ->
RavenQuery.limit(
raven_query,
offset_limit(offset, params, pk, query, "offset clause"),
offset_limit(limit, params, pk, query, "limit clause")
)
end
end

defp offset_limit(nil, _params, _pk, _query, _where), do: 0
nil
),
do: raven_query

defp offset_limit(%EctoQuery.LimitExpr{expr: expr}, params, pk, query, where),
do: value(expr, params, pk, query, where)

defp offset_limit(%EctoQuery.QueryExpr{expr: expr}, params, pk, query, where),
do: value(expr, params, pk, query, where)
defp limit_skip(
%RavenQuery{} = raven_query,
limit: limit,
offset: offset
),
do: RavenQuery.limit(raven_query, offset, limit)

defp order_by_expr({:asc, expr}, model, pk, query),
do: order_by_expr(expr, :asc, model, pk, query)
Expand Down
10 changes: 10 additions & 0 deletions test/ecto/repo_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,16 @@ defmodule Ecto.Integration.RepoTest do
assert post2 == query |> limit(1) |> offset(1) |> TestRepo.one()
end

test "limit and offset with pinned vars" do
post = TestRepo.insert!(%Post{title: "1"})

limit = 1
offset = 0

query = from(p in Post)
assert post == query |> limit(^limit) |> offset(^offset) |> TestRepo.one()
end

test "exists? should work" do
TestRepo.insert!(%Post{title: "1", visits: 2})
TestRepo.insert!(%Post{title: "2", visits: 1})
Expand Down

0 comments on commit 7b36421

Please sign in to comment.