Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions integration_test/index/filters_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,16 @@ defmodule Backpex.Integration.Index.FiltersTest do
|> refute_has("td", text: "New Post")
end

test "filters by a range beyond the range of an integer", %{conn: conn} do
params = fn likes -> %{"filters" => %{"published" => ["published"], "likes" => likes}} end

conn
|> visit(~p"/admin/posts?#{params.(%{"start" => "3000000000", "end" => ""})}")
|> refute_has("td", text: "Popular Post")
|> visit(~p"/admin/posts?#{params.(%{"start" => "", "end" => "100000000000000000000"})}")
|> assert_has("td", text: "Popular Post")
end

test "filters by a start value only", %{conn: conn} do
conn
|> visit(~p"/admin/posts")
Expand Down
42 changes: 40 additions & 2 deletions lib/backpex/adapters/ecto/where.ex
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ defmodule Backpex.Adapters.Ecto.Where do
alias Backpex.Expr.Ref

@comparisons [:==, :!=, :<, :<=, :>, :>=]
@integer_range -2_147_483_648..2_147_483_647//1
@bigint_min -9_223_372_036_854_775_808
@bigint_max 9_223_372_036_854_775_807
@flipped %{:== => :==, :!= => :!=, :< => :>, :<= => :>=, :> => :<, :>= => :<=}
@string_functions [:contains, :starts_with, :ends_with]

Expand Down Expand Up @@ -90,9 +93,17 @@ defmodule Backpex.Adapters.Ecto.Where do
end

defp to_dynamic(%Call{name: operator, args: [%Ref{} = ref, value]}, query, binding) when operator in @comparisons do
{query, {binding, name}} = resolve(ref, query, binding)
{query, {{_kind, schema} = binding, name}} = resolve(ref, query, binding)

# Postgres takes the type of a parameter from the column, so a parameter beyond the range of an integer column
# would fail to encode. A bigint parameter compares with every integer column.
if is_integer(value) and value not in @integer_range and integer_field?(schema, name) do
{operator, value} = limit_comparison(operator, value)

{query, compare(operator, binding, name, value)}
{query, compare_bigint(operator, field_dynamic(binding, name), value)}
else
{query, compare(operator, binding, name, value)}
end
end

defp to_dynamic(%Call{name: operator, args: [value, %Ref{} = ref]} = call, query, binding)
Expand Down Expand Up @@ -269,6 +280,33 @@ defmodule Backpex.Adapters.Ecto.Where do
end
end

defp compare_bigint(operator, field, value) do
case operator do
:== -> dynamic(^field == type(^value, :integer))
:!= -> dynamic(^field != type(^value, :integer))
:< -> dynamic(^field < type(^value, :integer))
:<= -> dynamic(^field <= type(^value, :integer))
:> -> dynamic(^field > type(^value, :integer))
:>= -> dynamic(^field >= type(^value, :integer))
end
end

defp integer_field?(nil = _schema, _name), do: false
defp integer_field?(schema, name), do: schema.__schema__(:type, name) in [:integer, :id]

# Every integer in the database is within the range of a bigint, so a comparison with an integer beyond it becomes a
# comparison with the bound of a bigint that keeps the same items. Like the comparison, it doesn't keep items whose
# field is `nil`.
defp limit_comparison(operator, value) when value > @bigint_max do
if operator in [:<, :<=, :!=], do: {:<=, @bigint_max}, else: {:>, @bigint_max}
end

defp limit_comparison(operator, value) when value < @bigint_min do
if operator in [:>, :>=, :!=], do: {:>=, @bigint_min}, else: {:<, @bigint_min}
end

defp limit_comparison(operator, value), do: {operator, value}

defp in_values({:root, _schema}, name, values), do: dynamic([item], field(item, ^name) in ^values)
defp in_values({binding, _schema}, name, values), do: dynamic([{^binding, item}], field(item, ^name) in ^values)

Expand Down
4 changes: 3 additions & 1 deletion lib/backpex/filters/filter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ defmodule Backpex.Filter do

def condition(_field, tag_ids, _assigns), do: expr(exists(tags, id in ^tag_ids))

The adapter casts each value to the type of the field. See `Backpex.Expr` for all operators and functions, and for
The adapter casts each value to the type of the field. `Backpex.Adapters.Ecto` also compares an integer field with an
integer beyond the range of the column, e.g. `expr(likes < 3_000_000_000)` on an `integer` column keeps every item
with likes. See `Backpex.Expr` for all operators and functions, and for
expressions that only one adapter understands, such as a fragment of SQL.

A filter can't change the query itself, e.g. join a table or select distinct items, as the adapter decides how to
Expand Down
44 changes: 44 additions & 0 deletions test/adapters/ecto_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,42 @@ defmodule Backpex.Adapters.EctoTest do
assert [%{params: [{["1", "2"], {:in, {0, :age}}}]}] = filter_query(expr(age in ["1", "2"])).wheres
end

test "compares an integer field with an integer beyond the range of an integer as a bigint" do
huge = 10 ** 20

expected = [
{expr(age < 3_000_000_000), "&0.age() < type(^0, :integer)", [3_000_000_000]},
{expr(age >= ^huge), "&0.age() > type(^0, :integer)", [9_223_372_036_854_775_807]},
{expr(age <= ^huge), "&0.age() <= type(^0, :integer)", [9_223_372_036_854_775_807]},
{expr(age != ^huge), "&0.age() <= type(^0, :integer)", [9_223_372_036_854_775_807]},
{expr(not (age < 3_000_000_000)), "not (&0.age() < type(^0, :integer))", [3_000_000_000]},
{expr(id >= ^huge), "&0.id() > type(^0, :integer)", [9_223_372_036_854_775_807]},
{expr(age <= ^(-huge)), "&0.age() < type(^0, :integer)", [-9_223_372_036_854_775_808]},
{expr(age > ^(-huge)), "&0.age() >= type(^0, :integer)", [-9_223_372_036_854_775_808]},
{expr(age == ^(-huge)), "&0.age() < type(^0, :integer)", [-9_223_372_036_854_775_808]},
{expr(3_000_000_000 > age), "&0.age() < type(^0, :integer)", [3_000_000_000]}
]

assert_wheres(expected)
end

test "compares an integer field of a related item with an integer beyond the range of an integer as a bigint" do
query =
TestPost
|> from(as: :testpost)
|> EctoAdapter.apply_filters(%{age: expr(user.age < 3_000_000_000)}, [age: %{module: ConditionFilter}], %{})

assert [%{expr: expr, params: [{3_000_000_000, :integer}]}] = query.wheres
assert Macro.to_string(expr) == "&1.age() < type(^0, :integer)"
end

test "leaves integers within the range of an integer, other fields and other values to the cast of the field" do
assert [%{params: [{2_147_483_647, {0, :age}}]}] = filter_query(expr(age <= 2_147_483_647)).wheres
assert [%{params: [{3_000_000_000, {0, :title}}]}] = filter_query(expr(title >= 3_000_000_000)).wheres
assert [%{params: [{2.5, {0, :age}}]}] = filter_query(expr(age >= 2.5)).wheres
assert [%{params: [{"3000000000", {0, :age}}]}] = filter_query(expr(age >= "3000000000")).wheres
end

test "takes native Ecto dynamic expressions" do
native = Expr.native(EctoAdapter, dynamic([testuser: u], u.age > ^18))
query = filter_query(Expr.any([native, expr(is_nil(age))]))
Expand Down Expand Up @@ -693,6 +729,14 @@ defmodule Backpex.Adapters.EctoTest do
assert Macro.to_string(expr) == "&1.name() == ^0"
end

defp assert_wheres(expected) do
for {condition, expected_expr, expected_params} <- expected do
assert [%{expr: expr, params: params}] = filter_query(condition).wheres
assert {condition, Macro.to_string(expr)} == {condition, expected_expr}
assert Enum.map(params, &elem(&1, 0)) == expected_params
end
end

defp filter_query(condition) do
TestUser
|> from(as: ^EctoAdapter.name_by_schema(TestUser))
Expand Down
Loading