From a886fecfd6f987aa49a7c43e6c2147549ecbf69c Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Thu, 31 Dec 2020 18:38:57 -0500 Subject: [PATCH] improvement: optimize not-in and fix dialyzer --- lib/ash/filter/filter.ex | 1 + lib/ash/query/expression.ex | 100 ++++++++++++++++++++++++++++++++++- lib/ash/query/operator/in.ex | 7 ++- lib/sat_solver.ex | 9 ++-- 4 files changed, 111 insertions(+), 6 deletions(-) diff --git a/lib/ash/filter/filter.ex b/lib/ash/filter/filter.ex index 94b45659..95963d58 100644 --- a/lib/ash/filter/filter.ex +++ b/lib/ash/filter/filter.ex @@ -1,4 +1,5 @@ defmodule Ash.Filter do + @dialyzer {:nowarn_function, do_map: 2, map: 2} alias Ash.Actions.SideLoad alias Ash.Engine.Request diff --git a/lib/ash/query/expression.ex b/lib/ash/query/expression.ex index 092824b2..8ea78392 100644 --- a/lib/ash/query/expression.ex +++ b/lib/ash/query/expression.ex @@ -1,7 +1,8 @@ defmodule Ash.Query.Expression do @moduledoc "Represents a boolean expression" + @dialyzer {:nowarn_function, optimized_new: 4} - alias Ash.Query.Operator.{Eq, In} + alias Ash.Query.Operator.{Eq, In, NotEq} alias Ash.Query.Ref defstruct [:op, :left, :right] @@ -44,6 +45,10 @@ defmodule Ash.Query.Expression do optimized_new(op, right, left, current_op) end + def optimized_new(op, %In{} = left, %NotEq{} = right, current_op) do + optimized_new(op, right, left, current_op) + end + def optimized_new(op, %In{} = left, %Eq{} = right, current_op) do optimized_new(op, right, left, current_op) end @@ -56,6 +61,14 @@ defmodule Ash.Query.Expression do do_new(op, left, right) end + def optimized_new(op, %NotEq{right: %Ref{}} = left, right, _) do + do_new(op, left, right) + end + + def optimized_new(op, left, %NotEq{right: %Ref{}} = right, _) do + do_new(op, left, right) + end + def optimized_new( :or, %Eq{left: left, right: value}, @@ -65,6 +78,21 @@ defmodule Ash.Query.Expression do %{right | right: MapSet.put(mapset, value)} end + def optimized_new( + :or, + %NotEq{left: left, right: value}, + %In{left: left, right: %{__struct__: MapSet} = mapset} = right, + _ + ) do + without = MapSet.delete(mapset, value) + + case MapSet.size(without) do + 0 -> false + 1 -> %Eq{left: left, right: Enum.at(without, 0)} + _ -> %{right | right: without} + end + end + def optimized_new( :and, %Eq{left: left, right: value} = left_expr, @@ -78,6 +106,28 @@ defmodule Ash.Query.Expression do end end + def optimized_new( + :and, + %NotEq{left: left, right: value}, + %In{left: left, right: %{__struct__: MapSet} = mapset} = right_expr, + _ + ) do + if MapSet.member?(mapset, value) do + false + else + right_expr + end + end + + def optimized_new( + op, + %NotEq{} = left, + %Eq{} = right, + current_op + ) do + optimized_new(op, right, left, current_op) + end + def optimized_new( :or, %Eq{left: left, right: left_value}, @@ -87,6 +137,26 @@ defmodule Ash.Query.Expression do %In{left: left, right: MapSet.new([left_value, right_value])} end + def optimized_new( + :or, + %NotEq{left: left, right: left_value}, + %Eq{left: left, right: right_value} = right_expr, + _ + ) + when left_value != right_value do + right_expr + end + + def optimized_new( + :or, + %NotEq{left: left, right: left_value}, + %Eq{left: left, right: right_value}, + _ + ) + when left_value == right_value do + true + end + def optimized_new( :and, %Eq{left: left, right: left_value} = left_expr, @@ -100,6 +170,16 @@ defmodule Ash.Query.Expression do end end + def optimized_new( + :and, + %NotEq{left: left, right: left_value} = left_expr, + %NotEq{left: left, right: right_value}, + _ + ) + when left_value == right_value do + left_expr + end + def optimized_new( :or, %In{left: left, right: left_values}, @@ -164,18 +244,36 @@ defmodule Ash.Query.Expression do do_new(op, left, right) end + defp simplify?(%NotEq{} = left, %In{} = right), do: simplify?(right, left) + defp simplify?(%NotEq{} = left, %Eq{} = right), do: simplify?(right, left) defp simplify?(%Eq{} = left, %In{} = right), do: simplify?(right, left) defp simplify?(%Eq{right: %Ref{}}, _), do: false defp simplify?(_, %Eq{right: %Ref{}}), do: false defp simplify?(%Eq{left: left}, %Eq{left: left}), do: true + defp simplify?(%NotEq{right: %Ref{}}, _), do: false + defp simplify?(_, %NotEq{right: %Ref{}}), do: false + defp simplify?(%NotEq{left: left}, %NotEq{left: left}), do: true + defp simplify?( %Eq{left: left}, %In{left: left, right: %MapSet{}} ), do: true + defp simplify?( + %NotEq{left: left}, + %In{left: left, right: %MapSet{}} + ), + do: true + + defp simplify?( + %Eq{left: left}, + %NotEq{left: left, right: %MapSet{}} + ), + do: true + defp simplify?(_, _), do: false defp do_new(op, left, right) do diff --git a/lib/ash/query/operator/in.ex b/lib/ash/query/operator/in.ex index 86a0a0ba..835e801d 100644 --- a/lib/ash/query/operator/in.ex +++ b/lib/ash/query/operator/in.ex @@ -13,6 +13,7 @@ defmodule Ash.Query.Operator.In do types: [[{:ref, :any}, {:array, :same}], [{:array, :same}, {:ref, :any}]] @inspect_items_limit 10 + @dialyzer {:nowarn_function, compare: 2} def new(_, []), do: {:known, false} @@ -46,8 +47,10 @@ defmodule Ash.Query.Operator.In do end end - def compare(%__MODULE__{}, %Ash.Query.Operator.Eq{right: %Ref{}}), - do: false + def compare(%__MODULE__{}, %Ash.Query.Operator.Eq{ + right: %Ref{} + }), + do: false def compare(%__MODULE__{left: left, right: %MapSet{} = left_right}, %Ash.Query.Operator.Eq{ left: left, diff --git a/lib/sat_solver.ex b/lib/sat_solver.ex index eeeb7873..7cb801f3 100644 --- a/lib/sat_solver.ex +++ b/lib/sat_solver.ex @@ -4,6 +4,8 @@ defmodule Ash.SatSolver do alias Ash.Filter alias Ash.Query.{Expression, Not, Ref} + @dialyzer {:nowarn_function, overlap?: 2} + defmacro b(statement) do value = Macro.prewalk( @@ -374,6 +376,7 @@ defmodule Ash.SatSolver do def fully_simplify(expression) do expression + |> do_fully_simplify() |> lift_equals_out_of_in() |> do_fully_simplify() end @@ -478,8 +481,8 @@ defmodule Ash.SatSolver do def split_in_expressions(other, _), do: other def overlap?( - %Ash.Query.Operator.In{left: left, right: %MapSet{} = left_right}, - %Ash.Query.Operator.In{left: left, right: %MapSet{} = right_right} + %Ash.Query.Operator.In{left: left, right: %{__struct__: MapSet} = left_right}, + %Ash.Query.Operator.In{left: left, right: %{__struct__: MapSet} = right_right} ) do if MapSet.equal?(left_right, right_right) do false @@ -506,7 +509,7 @@ defmodule Ash.SatSolver do def overlap?( %Ash.Query.Operator.Eq{left: left, right: left_right}, - %Ash.Query.Operator.In{left: left, right: %MapSet{} = right_right} + %Ash.Query.Operator.In{left: left, right: %{__struct__: MapSet} = right_right} ) do MapSet.member?(right_right, left_right) end