feat: boolean filter refactor (#78)

feat: predicate behaviour

.credo.exs

@@ -117,7 +117,7 @@
         ## Refactoring Opportunities
         #
         {Credo.Check.Refactor.CondStatements, []},
-        {Credo.Check.Refactor.CyclomaticComplexity, [max_complexity: 12]},
+        {Credo.Check.Refactor.CyclomaticComplexity, [max_complexity: 13]},
         {Credo.Check.Refactor.FunctionArity, []},
         {Credo.Check.Refactor.LongQuoteBlocks, []},
         {Credo.Check.Refactor.MapInto, []},

lib/ash.ex

@@ -175,6 +175,15 @@ defmodule Ash do
     |> Enum.find(&(&1.name == name))
   end
 
+  def related(resource, []), do: resource
+
+  def related(resource, [path | rest]) do
+    case relationship(resource, path) do
+      %{destination: destination} -> related(destination, rest)
+      nil -> nil
+    end
+  end
+
   @doc "The data layer of the resource, or nil if it does not have one"
   @spec data_layer(resource()) :: data_layer()
   def data_layer(resource) do

lib/ash/actions/read.ex

@@ -3,6 +3,7 @@ defmodule Ash.Actions.Read do
   alias Ash.Actions.SideLoad
   alias Ash.Engine
   alias Ash.Engine.Request
+  alias Ash.Filter
   require Logger
 
   def run(query, _action, opts \\ []) do
@@ -40,31 +41,37 @@ defmodule Ash.Actions.Read do
   end
 
   defp requests(query, action, opts) do
+    filter_requests =
+      if Keyword.has_key?(opts, :actor) || opts[:authorize?] do
+        Filter.read_requests(query.filter)
+      else
+        []
+      end
+
     request =
       Request.new(
         resource: query.resource,
         api: query.api,
         query: query,
         action: action,
-        data: data_field(opts, query.filter, query.resource, query.data_layer_query),
+        data: data_field(opts, filter_requests, query.resource, query.data_layer_query),
         path: [:data],
         name: "#{action.type} - `#{action.name}`"
       )
 
-    [request | Map.get(query.filter || %{}, :requests, [])]
+    [request | filter_requests]
   end
 
-  defp data_field(params, filter, resource, query) do
+  defp data_field(params, filter_requests, resource, query) do
     if params[:initial_data] do
       List.wrap(params[:initial_data])
     else
-      Request.resolve(
+      relationship_filter_paths = Enum.map(filter_requests, &[&1.path, :authorization_filter])
-        [[:data, :query]],
-        Ash.Filter.optional_paths(filter),
-        fn %{data: %{query: ash_query}} = data ->
-          fetch_filter = Ash.Filter.request_filter_for_fetch(ash_query.filter, data)
 
-          with {:ok, query} <- Ash.DataLayer.filter(query, fetch_filter, resource),
+      Request.resolve(
+        [[:data, :query] | relationship_filter_paths],
+        fn %{data: %{query: ash_query}} ->
+          with {:ok, query} <- Ash.DataLayer.filter(query, ash_query.filter, resource),
                {:ok, query} <- Ash.DataLayer.limit(query, ash_query.limit, resource),
                {:ok, query} <- Ash.DataLayer.offset(query, ash_query.offset, resource) do
             Ash.DataLayer.run_query(query, resource)

lib/ash/actions/relationships.ex

@@ -170,20 +170,20 @@ defmodule Ash.Actions.Relationships do
        ) do
     relationship_name = relationship.name
 
-    filter =
+    {possible?, filter} =
       case identifiers do
         [single_identifier] ->
           if Keyword.keyword?(single_identifier) do
-            single_identifier
+            {true, single_identifier}
           else
-            [single_identifier]
+            {true, [single_identifier]}
           end
 
         [] ->
-          [__impossible__: true]
+          {false, []}
 
         many ->
-          [or: many]
+          {true, [or: many]}
       end
 
     query =
@@ -198,10 +198,15 @@ defmodule Ash.Actions.Relationships do
         action: Ash.primary_action!(relationship.destination, :read),
         query: query,
         path: [:relationships, relationship_name, type],
+        authorize?: possible?,
         data:
           Request.resolve(fn _data ->
-            query
+            if possible? do
-            |> api.read()
+              query
+              |> api.read()
+            else
+              []
+            end
           end),
         name: "read prior to write related #{relationship.name}"
       )
@@ -270,7 +275,7 @@ defmodule Ash.Actions.Relationships do
         {:ok, %{replace: identifier}}
 
       {:error, _} ->
-        {:error, "Relationship change invalid for #{relationship.name} 2"}
+        {:error, "Relationship change invalid map for #{relationship.name}"}
     end
   end
 
@@ -283,7 +288,7 @@ defmodule Ash.Actions.Relationships do
         {:ok, %{replace: identifiers}}
 
       {:error, _} ->
-        {:error, "Relationship change invalid for #{relationship.name} 2"}
+        {:error, "Relationship change invalid list for #{relationship.name}"}
     end
   end
 

lib/ash/actions/side_load.ex

@@ -432,7 +432,7 @@ defmodule Ash.Actions.SideLoad do
       root_filter =
         case data do
           [] ->
-            [__impossible__: true]
+            false
 
           [%resource{} = item] ->
             item
@@ -588,8 +588,8 @@ defmodule Ash.Actions.SideLoad do
   end
 
   defp put_nested_relationship(request_filter, path, value, records? \\ true)
-  defp put_nested_relationship(_, _, [], true), do: [__impossible__: true]
+  defp put_nested_relationship(_, _, [], true), do: false
-  defp put_nested_relationship(_, _, nil, true), do: [__impossible__: true]
+  defp put_nested_relationship(_, _, nil, true), do: false
   defp put_nested_relationship(_, _, [], false), do: []
   defp put_nested_relationship(_, _, nil, false), do: []
 

lib/ash/data_layer/data_layer.ex

@@ -10,12 +10,10 @@ defmodule Ash.DataLayer do
   to ensure that the engine only ever tries to interact with the data layer in ways
   that it supports.
   """
-  @type filter_type :: :eq | :in
   @type feature() ::
           :transact
           | :async_engine
-          | {:filter, filter_type}
+          | {:filter_predicate, struct}
-          | {:filter_related, Ash.relationship_cardinality()}
           | :upsert
           | :composite_primary_key
 

lib/ash/data_layer/ets/ets.ex

@@ -5,7 +5,8 @@ defmodule Ash.DataLayer.Ets do
   This is used for testing. *Do not use this data layer in production*
   """
 
-  alias Ash.Filter.{And, Eq, In, NotEq, NotIn, Or}
+  alias Ash.Filter.{Expression, Not, Predicate}
+  alias Ash.Filter.Predicate.{Eq, In}
 
   @behaviour Ash.DataLayer
 
@@ -40,18 +41,10 @@ defmodule Ash.DataLayer.Ets do
     not private?(resource)
   end
 
-  def can?(_, :transact), do: false
-
   def can?(_, :composite_primary_key), do: true
   def can?(_, :upsert), do: true
-  def can?(_, {:filter, :in}), do: true
+  def can?(_, {:filter_predicate, %In{}}), do: true
-  def can?(_, {:filter, :not_in}), do: true
+  def can?(_, {:filter_predicate, %Eq{}}), do: true
-  def can?(_, {:filter, :not_eq}), do: true
-  def can?(_, {:filter, :eq}), do: true
-  def can?(_, {:filter, :and}), do: true
-  def can?(_, {:filter, :or}), do: true
-  def can?(_, {:filter, :not}), do: true
-  def can?(_, {:filter_related, _}), do: true
   def can?(_, _), do: false
 
   @impl true
@@ -80,21 +73,13 @@ defmodule Ash.DataLayer.Ets do
     {:ok, %{query | sort: sort}}
   end
 
-  @impl true
-  def run_query(%Query{filter: %Ash.Filter{impossible?: true}}, _), do: {:ok, []}
-
   @impl true
   def run_query(
         %Query{resource: resource, filter: filter, offset: offset, limit: limit, sort: sort},
         _resource
       ) do
-    with {:ok, table} <- wrap_or_create_table(resource),
+    with {:ok, records} <- get_records(resource),
-         {:ok, records} <- ETS.Set.to_list(table) do
+         filtered_records <- filter_matches(records, filter) do
-      filtered_records =
-        records
-        |> Enum.map(&elem(&1, 1))
-        |> filter_matches(filter)
-
       offset_records =
         filtered_records
         |> do_sort(sort)
@@ -113,110 +98,106 @@ defmodule Ash.DataLayer.Ets do
     end
   end
 
-  defp filter_matches(records, filter) do
+  defp get_records(resource) do
-    Enum.filter(records, &matches_filter?(&1, filter))
+    with {:ok, table} <- wrap_or_create_table(resource),
-  end
+         {:ok, record_tuples} <- ETS.Set.to_list(table) do
-
+      {:ok, Enum.map(record_tuples, &elem(&1, 1))}
-  defp matches_filter?(record, %{ands: [first | rest]} = filter) do
-    matches_filter?(record, first) and matches_filter?(record, %{filter | ands: rest})
-  end
-
-  defp matches_filter?(record, %{not: not_filter} = filter) when not is_nil(not_filter) do
-    not matches_filter?(record, not_filter) and matches_filter?(record, %{filter | not: nil})
-  end
-
-  defp matches_filter?(record, %{ors: [first | rest]} = filter) do
-    matches_filter?(record, first) or matches_filter?(record, %{filter | ors: rest})
-  end
-
-  defp matches_filter?(record, %{resource: resource} = filter) do
-    resource
-    |> relationships_to_attribute_filters(filter)
-    |> Map.get(:attributes)
-    |> Enum.all?(fn {key, predicate} ->
-      matches_predicate?(Map.get(record, key), predicate)
-    end)
-  end
-
-  # alias Ash.Filter.{In, NotIn}
-
-  defp matches_predicate?(value, %Eq{value: predicate_value}) do
-    value == predicate_value
-  end
-
-  defp matches_predicate?(value, %NotEq{value: predicate_value}) do
-    value != predicate_value
-  end
-
-  defp matches_predicate?(value, %In{values: predicate_value}) do
-    value in predicate_value
-  end
-
-  defp matches_predicate?(value, %NotIn{values: predicate_value}) do
-    value not in predicate_value
-  end
-
-  defp matches_predicate?(value, %And{left: left, right: right}) do
-    matches_predicate?(value, left) and matches_predicate?(value, right)
-  end
-
-  defp matches_predicate?(value, %Or{left: left, right: right}) do
-    matches_predicate?(value, left) or matches_predicate?(value, right)
-  end
-
-  defp relationships_to_attribute_filters(_, %{relationships: relationships} = filter)
-       when relationships in [nil, %{}] do
-    filter
-  end
-
-  defp relationships_to_attribute_filters(resource, %{relationships: relationships} = filter) do
-    Enum.reduce(relationships, filter, fn {rel, related_filter}, filter ->
-      relationship = Ash.relationship(resource, rel)
-
-      {field, parsed_related_filter} = related_ids_filter(relationship, related_filter)
-
-      Ash.Filter.add_to_filter(filter, [{field, parsed_related_filter}])
-    end)
-  end
-
-  defp related_ids_filter(%{type: :many_to_many} = rel, filter) do
-    destination_query = %Query{
-      resource: rel.destination,
-      filter: filter
-    }
-
-    with {:ok, results} <- run_query(destination_query, rel.destination),
-         destination_values <- Enum.map(results, &Map.get(&1, rel.destination_field)),
-         %{errors: []} = through_filter <-
-           Ash.Filter.parse(
-             rel.through,
-             [
-               {rel.destination_field_on_join_table, [in: destination_values]}
-             ],
-             filter.api
-           ),
-         {:ok, join_results} <-
-           run_query(%Query{resource: rel.through, filter: through_filter}, rel.through) do
-      {rel.source_field,
-       [in: Enum.map(join_results, &Map.get(&1, rel.source_field_on_join_table))]}
-    else
-      %{errors: errors} -> {:error, errors}
-      {:error, error} -> {:error, error}
     end
   end
 
-  defp related_ids_filter(rel, filter) do
+  defp filter_matches(records, nil), do: records
-    query = %Query{
-      resource: rel.destination,
-      filter: filter
-    }
 
-    case run_query(query, rel.destination) do
+  defp filter_matches(records, filter) do
-      {:ok, results} ->
+    Enum.filter(records, &matches_filter?(&1, filter.expression))
-        {rel.source_field, [in: Enum.map(results, &Map.get(&1, rel.destination_field))]}
+  end
 
-      {:error, error} ->
+  defp matches_filter?(
-        {:error, error}
+         record,
+         %Predicate{
+           predicate: predicate,
+           attribute: %{name: name},
+           relationship_path: []
+         }
+       ) do
+    matches_predicate?(record, name, predicate)
+  end
+
+  defp matches_filter?(
+         record,
+         %Predicate{
+           predicate: predicate,
+           attribute: %{name: name},
+           relationship_path: path
+         }
+       ) do
+    record
+    |> get_related(path)
+    |> Enum.any?(&matches_predicate?(&1, name, predicate))
+  end
+
+  defp matches_filter?(record, %Expression{op: :and, left: left, right: right}) do
+    matches_filter?(record, left) && matches_filter?(record, right)
+  end
+
+  defp matches_filter?(record, %Expression{op: :or, left: left, right: right}) do
+    matches_filter?(record, left) || matches_filter?(record, right)
+  end
+
+  defp matches_filter?(record, %Not{expression: expression}) do
+    not matches_filter?(record, expression)
+  end
+
+  defp get_related(record_or_records, []), do: List.wrap(record_or_records)
+
+  defp get_related(%resource{} = record, [first | rest]) do
+    relationship = Ash.relationship(resource, first)
+    source_value = Map.get(record, relationship.source_field)
+
+    related =
+      if is_nil(Map.get(record, relationship.source_field)) do
+        []
+      else
+        case Ash.relationship(resource, first) do
+          %{type: :many_to_many} = relationship ->
+            {:ok, through_records} = get_records(relationship.through)
+            {:ok, destination_records} = get_records(relationship.destination)
+
+            through_records
+            |> Enum.reject(&is_nil(Map.get(&1, relationship.destination_field_on_join_table)))
+            |> Enum.flat_map(fn through_record ->
+              if Map.get(through_record, relationship.source_field_on_join_table) ==
+                   source_value do
+                Enum.filter(destination_records, fn destination_record ->
+                  Map.get(through_record, relationship.destination_field_on_join_table) ==
+                    Map.get(destination_record, relationship.destination_field)
+                end)
+              else
+                []
+              end
+            end)
+
+          relationship ->
+            {:ok, destination_records} = get_records(relationship.destination)
+
+            Enum.filter(destination_records, fn destination_record ->
+              Map.get(destination_record, relationship.destination_field) == source_value
+            end)
+        end
+      end
+
+    related
+    |> List.wrap()
+    |> Enum.flat_map(&get_related(&1, rest))
+  end
+
+  defp matches_predicate?(record, field, %Eq{value: predicate_value}) do
+    Map.fetch(record, field) == {:ok, predicate_value}
+  end
+
+  defp matches_predicate?(record, field, %In{values: predicate_values}) do
+    case Map.fetch(record, field) do
+      {:ok, value} -> value in predicate_values
+      :error -> false
     end
   end
 

lib/ash/engine/engine.ex

@@ -30,13 +30,6 @@ defmodule Ash.Engine do
     authorize? = opts[:authorize?] || Keyword.has_key?(opts, :actor)
     actor = opts[:actor]
 
-    requests =
-      if authorize? do
-        requests
-      else
-        Enum.reject(requests, & &1.skip_unless_authorize?)
-      end
-
     case Request.validate_requests(requests) do
       :ok ->
         requests =

lib/ash/engine/request.ex

@@ -45,7 +45,7 @@ defmodule Ash.Engine.Request do
     :api,
     :query,
     :write_to_data?,
-    :skip_unless_authorize?,
+    :strict_check_only?,
     :verbose?,
     :state,
     :actor,
@@ -106,7 +106,7 @@ defmodule Ash.Engine.Request do
       query: query,
       api: opts[:api],
       name: opts[:name],
-      skip_unless_authorize?: opts[:skip_unless_authorize?],
+      strict_check_only?: opts[:strict_check_only?],
       state: :strict_check,
       actor: opts[:actor],
       verbose?: opts[:verbose?] || false,
@@ -392,6 +392,8 @@ defmodule Ash.Engine.Request do
   end
 
   defp do_strict_check(authorizer, request, notifications \\ []) do
+    strict_check_only? = request.strict_check_only?
+
     case missing_strict_check_dependencies?(authorizer, request) do
       [] ->
         case strict_check_authorizer(authorizer, request) do
@@ -404,6 +406,9 @@ defmodule Ash.Engine.Request do
             |> set_authorizer_state(authorizer, :authorized)
             |> try_resolve([request.path ++ [:query]], false, false)
 
+          {:filter_and_continue, _, _} when strict_check_only? ->
+            {:error, "Request must pass strict check"}
+
           {:filter_and_continue, filter, new_authorizer_state} ->
             new_request =
               request
@@ -412,6 +417,9 @@ defmodule Ash.Engine.Request do
 
             {:ok, new_request}
 
+          {:continue, _} when strict_check_only? ->
+            {:error, "Request must pass strict check"}
+
           {:continue, authorizer_state} ->
             {:ok, set_authorizer_state(request, authorizer, authorizer_state)}
 

lib/ash/error.ex

@@ -45,11 +45,20 @@ defmodule Ash.Error do
   end
 
   def choose_error(errors) do
-    [error | _other_errors] = Enum.sort_by(errors, &Map.get(@error_class_indices, &1.class))
+    [error | other_errors] =
+      Enum.sort_by(errors, fn error ->
+        # the second element here sorts errors that are already parent errors
+        {Map.get(@error_class_indices, error.class),
+         @error_modules[error.class] != error.__struct__}
+      end)
 
     parent_error_module = @error_modules[error.class]
 
-    parent_error_module.exception(errors: errors)
+    if parent_error_module == error.__struct__ do
+      parent_error_module.exception(errors: (error.errors || []) ++ other_errors)
+    else
+      parent_error_module.exception(errors: errors)
+    end
   end
 
   def error_messages(errors) do

lib/ash/error/filter/invalid_filter_value.ex

@@ -11,12 +11,12 @@ defmodule Ash.Error.Filter.InvalidFilterValue do
 
     def class(_), do: :invalid
 
-    def message(%{field: field, value: value, filter: filter}) do
+    def message(%{value: value, filter: filter}) do
-      "Invalid filter value #{inspect(value)} supplied for #{inspect(field)}#{inspect(filter)}"
+      "Invalid filter value `#{inspect(value)}` supplied in: `#{inspect(filter)}`"
     end
 
-    def description(%{field: field, filter: filter, value: value}) do
+    def description(%{filter: filter, value: value}) do
-      "Invalid filter value #{inspect(value)} supplied for #{inspect(field)}#{inspect(filter)}"
+      "Invalid filter value `#{inspect(value)}` supplied in: `#{inspect(filter)}`"
     end
 
     def stacktrace(_), do: nil

lib/ash/filter/and.ex

@@ -1,54 +0,0 @@
-defmodule Ash.Filter.And do
-  @moduledoc false
-  defstruct [:left, :right]
-
-  def new(resource, attr_name, attr_type, [first | [last | []]]) do
-    with {:ok, first} <- Ash.Filter.parse_predicates(resource, first, attr_name, attr_type),
-         {:ok, right} <- Ash.Filter.parse_predicates(resource, last, attr_name, attr_type) do
-      if first == right do
-        {:ok, first}
-      else
-        {:ok, %__MODULE__{left: first, right: right}}
-      end
-    end
-  end
-
-  def new(resource, attr_name, attr_type, [first | rest]) do
-    case Ash.Filter.parse_predicates(resource, first, attr_name, attr_type) do
-      {:ok, first} ->
-        case new(resource, attr_name, attr_type, rest) do
-          {:ok, right} when right == first ->
-            {:ok, right}
-
-          {:ok, right} ->
-            {:ok, %__MODULE__{left: first, right: right}}
-
-          {:error, error} ->
-            {:error, error}
-        end
-
-      {:error, error} ->
-        {:error, error}
-    end
-  end
-
-  def new(resource, attr_name, attr_type, {left, right}) do
-    new(resource, attr_name, attr_type, [left, right])
-  end
-
-  def prebuilt_new(left, right) do
-    if left == right do
-      left
-    else
-      %__MODULE__{left: left, right: right}
-    end
-  end
-end
-
-defimpl Inspect, for: Ash.Filter.And do
-  import Inspect.Algebra
-
-  def inspect(%{left: left, right: right}, opts) do
-    concat([to_doc(left, opts), " and ", to_doc(right, opts)])
-  end
-end

lib/ash/filter/eq.ex

@@ -1,30 +0,0 @@
-defmodule Ash.Filter.Eq do
-  @moduledoc false
-  defstruct [:value]
-
-  alias Ash.Error.Filter.InvalidFilterValue
-
-  def new(_resource, attr_name, attr_type, value) do
-    case Ash.Type.cast_input(attr_type, value) do
-      {:ok, value} ->
-        {:ok, %__MODULE__{value: value}}
-
-      :error ->
-        {:error,
-         InvalidFilterValue.exception(
-           filter: %__MODULE__{value: value},
-           value: value,
-           field: attr_name
-         )}
-    end
-  end
-end
-
-defimpl Inspect, for: Ash.Filter.Eq do
-  import Inspect.Algebra
-  import Ash.Filter.InspectHelpers
-
-  def inspect(%{value: value}, opts) do
-    concat([attr(opts), " == ", to_doc(value, opts)])
-  end
-end

lib/ash/filter/expression.ex

@@ -0,0 +1,44 @@
+defmodule Ash.Filter.Expression do
+  @moduledoc "Represents a boolean expression"
+
+  defstruct [:op, :left, :right]
+
+  def new(_, nil, nil), do: nil
+  def new(_, nil, right), do: right
+  def new(_, left, nil), do: left
+
+  def new(op, %__MODULE__{op: left_op} = left, %__MODULE__{op: op} = right) when left_op != op do
+    %__MODULE__{op: op, left: right, right: left}
+  end
+
+  def new(op, left, right) do
+    %__MODULE__{op: op, left: left, right: right}
+  end
+end
+
+defimpl Inspect, for: Ash.Filter.Expression do
+  import Inspect.Algebra
+
+  def inspect(
+        %{left: left, right: right, op: op},
+        opts
+      ) do
+    container_type = container_type(opts)
+
+    opts = put_container_type(opts, op)
+
+    if container_type && op != container_type do
+      concat(["(", to_doc(left, opts), " ", to_string(op), " ", to_doc(right, opts), ")"])
+    else
+      concat([to_doc(left, opts), " ", to_string(op), " ", to_doc(right, opts)])
+    end
+  end
+
+  defp container_type(opts) do
+    opts.custom_options[:container_type]
+  end
+
+  defp put_container_type(opts, container_type) do
+    %{opts | custom_options: Keyword.put(opts.custom_options, :container_type, container_type)}
+  end
+end

lib/ash/filter/in.ex

@@ -1,56 +0,0 @@
-defmodule Ash.Filter.In do
-  @moduledoc false
-  defstruct [:values]
-
-  alias Ash.Error.Filter.InvalidFilterValue
-  alias Ash.Filter.Eq
-
-  def new(resource, attr_name, attr_type, [value]) do
-    Eq.new(resource, attr_name, attr_type, value)
-  end
-
-  def new(_resource, attr_name, attr_type, values) do
-    casted =
-      values
-      |> List.wrap()
-      |> Enum.reduce({:ok, []}, fn
-        value, {:ok, casted} ->
-          case Ash.Type.cast_input(attr_type, value) do
-            {:ok, value} ->
-              {:ok, [value | casted]}
-
-            :error ->
-              {:error,
-               InvalidFilterValue.exception(
-                 filter: %__MODULE__{values: values},
-                 value: value,
-                 field: attr_name
-               )}
-          end
-
-        _, {:error, error} ->
-          {:error, error}
-      end)
-
-    case casted do
-      {:error, error} ->
-        {:error, error}
-
-      {:ok, values} ->
-        {:ok, %__MODULE__{values: values}}
-    end
-    |> case do
-      {:ok, %{values: values} = in_operator} -> {:ok, %{in_operator | values: Enum.uniq(values)}}
-      {:error, error} -> {:error, error}
-    end
-  end
-end
-
-defimpl Inspect, for: Ash.Filter.In do
-  import Inspect.Algebra
-  import Ash.Filter.InspectHelpers
-
-  def inspect(%{values: values}, opts) do
-    concat([attr(opts), " in ", to_doc(values, opts)])
-  end
-end

lib/ash/filter/inspect.ex

@@ -1,43 +0,0 @@
-defmodule Ash.Filter.InspectHelpers do
-  @moduledoc false
-  def attr(opts) do
-    case path(opts) do
-      path when path in [nil, []] ->
-        to_string(get_attr(opts))
-
-      path ->
-        Enum.join(path || [], ".") <> "." <> to_string(get_attr(opts))
-    end
-  end
-
-  def root?(opts) do
-    path(opts) == nil && get_attr(opts) == nil
-  end
-
-  def put_attr(%{custom_options: custom} = opts, attr) do
-    %{opts | custom_options: Keyword.put(custom, :attr, attr)}
-  end
-
-  def make_non_root(%{custom_options: custom} = opts) do
-    new_options = Keyword.put(custom, :path, custom[:path] || [])
-
-    %{opts | custom_options: new_options}
-  end
-
-  def add_to_path(%{custom_options: custom} = opts, path_item) do
-    new_options =
-      Keyword.update(custom, :path, [to_string(path_item)], fn path ->
-        [path_item | path]
-      end)
-
-    %{opts | custom_options: new_options}
-  end
-
-  defp get_attr(opts) do
-    opts.custom_options[:attr]
-  end
-
-  defp path(opts) do
-    opts.custom_options[:path]
-  end
-end

lib/ash/filter/merge.ex

@@ -1,14 +0,0 @@
-defmodule Ash.Filter.Merge do
-  @moduledoc false
-  alias Ash.Filter.And
-
-  def merge(left, right) do
-    [left, right] = Enum.sort_by([left, right], fn %mod{} -> to_string(mod) end)
-
-    do_merge(left, right)
-  end
-
-  defp do_merge(left, right) do
-    And.prebuilt_new(left, right)
-  end
-end

lib/ash/filter/not.ex

@@ -0,0 +1,26 @@
+defmodule Ash.Filter.Not do
+  @moduledoc "Represents the negation of the contained expression"
+  defstruct [:expression]
+
+  alias Ash.Filter.Expression
+
+  def new(nil), do: nil
+
+  def new(expression) do
+    %__MODULE__{expression: expression}
+  end
+
+  defimpl Inspect do
+    import Inspect.Algebra
+
+    def inspect(%{expression: expression}, opts) do
+      case expression do
+        %Expression{op: :and} ->
+          concat(["not ", "(", to_doc(expression, opts), ")"])
+
+        _ ->
+          concat(["not ", to_doc(expression, opts)])
+      end
+    end
+  end
+end

lib/ash/filter/not_eq.ex

@@ -1,30 +0,0 @@
-defmodule Ash.Filter.NotEq do
-  @moduledoc false
-  defstruct [:value]
-
-  alias Ash.Error.Filter.InvalidFilterValue
-
-  def new(_resource, attr_name, attr_type, value) do
-    case Ash.Type.cast_input(attr_type, value) do
-      {:ok, value} ->
-        {:ok, %__MODULE__{value: value}}
-
-      :error ->
-        {:error,
-         InvalidFilterValue.exception(
-           filter: %__MODULE__{value: value},
-           value: value,
-           field: attr_name
-         )}
-    end
-  end
-end
-
-defimpl Inspect, for: Ash.Filter.NotEq do
-  import Inspect.Algebra
-  import Ash.Filter.InspectHelpers
-
-  def inspect(%{value: value}, opts) do
-    concat([attr(opts), " != ", to_doc(value, opts)])
-  end
-end

lib/ash/filter/not_in.ex

@@ -1,52 +0,0 @@
-defmodule Ash.Filter.NotIn do
-  @moduledoc false
-  defstruct [:values]
-
-  alias Ash.Error.Filter.InvalidFilterValue
-  alias Ash.Filter.NotEq
-
-  def new(resource, attr_name, attr_type, [value]) do
-    NotEq.new(resource, attr_name, attr_type, value)
-  end
-
-  def new(_resource, attr_name, attr_type, values) do
-    casted =
-      values
-      |> List.wrap()
-      |> Enum.reduce({:ok, []}, fn
-        value, {:ok, casted} ->
-          case Ash.Type.cast_input(attr_type, value) do
-            {:ok, value} ->
-              {:ok, [value | casted]}
-
-            :error ->
-              {:error,
-               InvalidFilterValue.exception(
-                 filter: %__MODULE__{values: values},
-                 value: value,
-                 field: attr_name
-               )}
-          end
-
-        _, {:error, error} ->
-          {:error, error}
-      end)
-
-    case casted do
-      {:error, error} ->
-        {:error, error}
-
-      {:ok, values} ->
-        {:ok, %__MODULE__{values: values}}
-    end
-  end
-end
-
-defimpl Inspect, for: Ash.Filter.NotIn do
-  import Inspect.Algebra
-  import Ash.Filter.InspectHelpers
-
-  def inspect(%{values: values}, opts) do
-    concat([attr(opts), " not in ", to_doc(values, opts)])
-  end
-end

lib/ash/filter/or.ex

@@ -1,41 +0,0 @@
-defmodule Ash.Filter.Or do
-  @moduledoc false
-  defstruct [:left, :right]
-
-  def new(resource, attr_name, attr_type, [first | [last | []]]) do
-    with {:ok, first} <- Ash.Filter.parse_predicates(resource, first, attr_name, attr_type),
-         {:ok, right} <- Ash.Filter.parse_predicates(resource, last, attr_name, attr_type) do
-      {:ok, %__MODULE__{left: first, right: right}}
-    end
-  end
-
-  def new(resource, attr_name, attr_type, [first | rest]) do
-    case Ash.Filter.parse_predicates(resource, first, attr_name, attr_type) do
-      {:ok, first} ->
-        {:ok, %__MODULE__{left: first, right: new(resource, attr_name, attr_type, rest)}}
-
-      {:error, error} ->
-        {:error, error}
-    end
-  end
-
-  def new(resource, attr_name, attr_type, {left, right}) do
-    new(resource, attr_name, attr_type, [left, right])
-  end
-
-  def prebuilt_new(left, right) do
-    if left == right do
-      left
-    else
-      %__MODULE__{left: left, right: right}
-    end
-  end
-end
-
-defimpl Inspect, for: Ash.Filter.Or do
-  import Inspect.Algebra
-
-  def inspect(%{left: left, right: right}, opts) do
-    concat([to_doc(left, opts), " or ", to_doc(right, opts)])
-  end
-end

lib/ash/filter/predicate.ex

@@ -0,0 +1,148 @@
+defmodule Ash.Filter.Predicate do
+  @moduledoc "Represents a filter predicate"
+
+  defstruct [:attribute, :relationship_path, :predicate]
+
+  alias Ash.Filter
+  alias Ash.Filter.{Expression, Not}
+
+  @type predicate :: struct
+
+  @type comparison ::
+          :unknown
+          | :right_excludes_left
+          | :left_excludes_right
+          | :right_includes_left
+          | :left_includes_right
+          | :mutually_inclusive
+          # A simplification value for the right term
+          | {:simplify, term}
+          | {:simplify, term, term}
+
+  @type t :: %__MODULE__{
+          attribute: Ash.attribute(),
+          relationship_path: list(atom),
+          predicate: predicate
+        }
+
+  @callback new(Ash.resource(), Ash.attribute(), term) :: {:ok, struct} | {:error, term}
+  @callback compare(predicate(), predicate()) :: comparison()
+
+  defmacro __using__(_opts) do
+    quote do
+      @behaviour Ash.Filter.Predicate
+
+      @spec compare(Ash.Filter.Predicate.predicate(), Ash.Filter.Predicate.predicate()) ::
+              Ash.Filter.Predicate.comparison() | :unknown
+      def compare(_, _), do: :unknown
+
+      defoverridable compare: 2
+    end
+  end
+
+  @spec compare(predicate(), predicate()) :: comparison
+  def compare(%__MODULE__{predicate: left} = pred, right) do
+    case compare(left, right) do
+      {:simplify, simplification} ->
+        simplification =
+          Filter.map(simplification, fn
+            %struct{} = expr when struct in [__MODULE__, Not, Expression] ->
+              expr
+
+            other ->
+              wrap_in_predicate(pred, other)
+          end)
+
+        {:simplify, simplification}
+
+      other ->
+        other
+    end
+  end
+
+  def compare(left, %__MODULE__{predicate: right}), do: compare(left, right)
+
+  def compare(left, right) do
+    if left.__struct__ == right.__struct__ do
+      with {:right_to_left, :unknown} <- {:right_to_left, left.__struct__.compare(left, right)},
+           {:left_to_right, :unknown} <- {:left_to_right, right.__struct__.compare(left, right)} do
+        :mutually_exclusive
+      else
+        {:right_to_left, {:simplify, left, _}} -> {:simplify, left}
+        {:left_to_right, {:simplify, _, right}} -> {:simplify, right}
+        {_, other} -> other
+      end
+    else
+      with {:right_to_left, :unknown} <- {:right_to_left, left.__struct__.compare(left, right)},
+           {:right_to_left, :unknown} <- {:right_to_left, right.__struct__.compare(left, right)},
+           {:left_to_right, :unknown} <- {:left_to_right, right.__struct__.compare(left, right)},
+           {:left_to_right, :unknown} <- {:left_to_right, left.__struct__.compare(left, right)} do
+        :mutually_exclusive
+      else
+        {:right_to_left, {:simplify, left, _}} -> {:simplify, left}
+        {:left_to_right, {:simplify, _, right}} -> {:simplify, right}
+        {_, other} -> other
+      end
+    end
+  end
+
+  defp wrap_in_predicate(predicate, %struct{} = other) do
+    if Ash.implements_behaviour?(struct, Ash.Filter.Predicate) do
+      %{predicate | predicate: other}
+    else
+      other
+    end
+  end
+
+  def new(resource, attribute, predicate, value, relationship_path) do
+    case predicate.new(resource, attribute, value) do
+      {:ok, predicate} ->
+        if Ash.data_layer_can?(resource, {:filter_predicate, predicate}) do
+          {:ok,
+           %__MODULE__{
+             attribute: attribute,
+             predicate: predicate,
+             relationship_path: relationship_path
+           }}
+        else
+          {:error, "Data layer does not support filtering with #{inspect(predicate)}"}
+        end
+
+      {:error, error} ->
+        {:error, error}
+    end
+  end
+
+  def add_inspect_path(inspect_opts, field) do
+    case inspect_opts.custom_options[:relationship_path] do
+      empty when empty in [nil, []] -> to_string(field)
+      path -> Enum.join(path, ".") <> "." <> to_string(field)
+    end
+  end
+
+  defimpl Inspect do
+    import Inspect.Algebra
+
+    def inspect(
+          %{relationship_path: relationship_path, predicate: predicate},
+          opts
+        ) do
+      opts = %{
+        opts
+        | syntax_colors: [
+            atom: :yellow,
+            binary: :green,
+            boolean: :pink,
+            list: :cyan,
+            map: :magenta,
+            number: :red,
+            regex: :violet,
+            tuple: :white
+          ],
+          custom_options: Keyword.put(opts.custom_options, :relationship_path, relationship_path)
+      }
+
+      to_doc(predicate, opts)
+    end
+  end
+end

lib/ash/filter/predicate/eq.ex

@@ -0,0 +1,41 @@
+defmodule Ash.Filter.Predicate.Eq do
+  @moduledoc false
+  defstruct [:field, :value]
+
+  use Ash.Filter.Predicate
+
+  alias Ash.Error.Filter.InvalidFilterValue
+  alias Ash.Filter.Predicate
+
+  def new(_resource, attribute, value) do
+    case Ash.Type.cast_input(attribute.type, value) do
+      {:ok, value} ->
+        {:ok, %__MODULE__{field: attribute.name, value: value}}
+
+      :error ->
+        {:error,
+         InvalidFilterValue.exception(
+           filter: %__MODULE__{field: attribute.name, value: value},
+           value: value,
+           field: attribute.name
+         )}
+    end
+  end
+
+  def compare(%__MODULE__{value: value}, %__MODULE__{value: value}), do: :mutually_inclusive
+  def compare(%__MODULE__{value: _}, %__MODULE__{value: _}), do: :mutually_exclusive
+
+  def compare(_, _), do: :unknown
+
+  defimpl Inspect do
+    import Inspect.Algebra
+
+    def inspect(predicate, opts) do
+      concat([
+        Predicate.add_inspect_path(opts, predicate.field),
+        " == ",
+        to_doc(predicate.value, opts)
+      ])
+    end
+  end
+end

lib/ash/filter/predicate/in.ex

@@ -0,0 +1,72 @@
+defmodule Ash.Filter.Predicate.In do
+  @moduledoc false
+  defstruct [:field, :values]
+
+  use Ash.Filter.Predicate
+
+  alias Ash.Error.Filter.InvalidFilterValue
+  alias Ash.Filter.Expression
+  alias Ash.Filter.Predicate
+  alias Ash.Filter.Predicate.Eq
+
+  def new(_resource, attribute, []),
+    do: {:ok, %__MODULE__{field: attribute.name, values: MapSet.new([])}}
+
+  def new(resource, attribute, [value]), do: {:ok, Eq.new(resource, attribute, value)}
+
+  def new(_resource, attribute, values) when is_list(values) do
+    Enum.reduce_while(values, {:ok, %__MODULE__{field: attribute.name, values: []}}, fn value,
+                                                                                        {:ok,
+                                                                                         predicate} ->
+      case Ash.Type.cast_input(attribute.type, value) do
+        {:ok, casted} ->
+          {:cont, {:ok, %{predicate | values: [casted | predicate.values]}}}
+
+        :error ->
+          {:error,
+           InvalidFilterValue.exception(
+             filter: %__MODULE__{field: attribute.name, values: values},
+             value: value,
+             field: attribute.name
+           )}
+      end
+    end)
+  end
+
+  def new(_resource, attribute, values) do
+    {:error,
+     InvalidFilterValue.exception(
+       filter: %__MODULE__{field: attribute.name, values: values},
+       value: values,
+       field: attribute.name
+     )}
+  end
+
+  def compare(%__MODULE__{} = left, %__MODULE__{} = right) do
+    {:simplify, in_to_or_equals(left), in_to_or_equals(right)}
+  end
+
+  def compare(%__MODULE__{} = in_expr, _) do
+    {:simplify, in_to_or_equals(in_expr)}
+  end
+
+  def compare(_, _), do: :unknown
+
+  defp in_to_or_equals(%{field: field, values: values}) do
+    Enum.reduce(values, nil, fn value, expression ->
+      Expression.new(:or, expression, %Eq{field: field, value: value})
+    end)
+  end
+
+  defimpl Inspect do
+    import Inspect.Algebra
+
+    def inspect(predicate, opts) do
+      concat([
+        Predicate.add_inspect_path(opts, predicate.field),
+        " in ",
+        to_doc(predicate.values, opts)
+      ])
+    end
+  end
+end

lib/ash/query.ex

@@ -24,6 +24,20 @@ defmodule Ash.Query do
     import Inspect.Algebra
 
     def inspect(query, opts) do
+      opts = %{
+        opts
+        | syntax_colors: [
+            atom: :yellow,
+            binary: :green,
+            boolean: :pink,
+            list: :cyan,
+            map: :magenta,
+            number: :red,
+            regex: :violet,
+            tuple: :white
+          ]
+      }
+
       error_doc =
         if Enum.empty?(query.errors) do
           empty()
@@ -59,7 +73,7 @@ defmodule Ash.Query do
       {:ok, resource} ->
         %__MODULE__{
           api: api,
-          filter: Ash.Filter.parse(resource, [], api),
+          filter: nil,
           resource: resource
         }
         |> set_data_layer_query()
@@ -67,7 +81,7 @@ defmodule Ash.Query do
       :error ->
         %__MODULE__{
           api: api,
-          filter: Ash.Filter.parse(resource, [], api),
+          filter: nil,
           resource: resource
         }
         |> add_error(:resource, "does not exist")
@@ -230,16 +244,19 @@ defmodule Ash.Query do
     new_filter =
       case query.filter do
         nil ->
-          filter
+          {:ok, filter}
 
         existing_filter ->
           Ash.Filter.add_to_filter(existing_filter, filter)
       end
 
-    new_filter.errors
+    case new_filter do
-    |> Enum.reduce(query, &add_error(&2, :filter, &1))
+      {:ok, filter} ->
-    |> Map.put(:filter, new_filter)
+        set_data_layer_query(%{query | filter: filter})
-    |> set_data_layer_query()
+
+      {:error, error} ->
+        add_error(query, :filter, error)
+    end
   end
 
   def filter(query, statement) do
@@ -247,46 +264,17 @@ defmodule Ash.Query do
       if query.filter do
         Ash.Filter.add_to_filter(query.filter, statement)
       else
-        Ash.Filter.parse(query.resource, statement, query.api)
+        Ash.Filter.parse(query.api, query.resource, statement)
       end
 
-    filter.errors
+    case filter do
-    |> Enum.reduce(query, &add_error(&2, :filter, &1))
+      {:ok, filter} ->
-    |> Map.put(:filter, filter)
-    |> set_data_layer_query()
-  end
-
-  def reject(query, statement) when is_list(statement) do
-    filter(query, not: statement)
-  end
-
-  def reject(query, %Ash.Filter{} = filter) do
-    case query.filter do
-      nil ->
-        new_filter =
-          query.resource
-          |> Ash.Filter.parse([], query.api)
-          |> Map.put(:not, filter)
-
         query
-        |> Map.put(:filter, new_filter)
+        |> Map.put(:filter, filter)
         |> set_data_layer_query()
 
-      existing_filter ->
+      {:error, error} ->
-        new_filter_not =
+        add_error(query, :filter, error)
-          case existing_filter.not do
-            nil ->
-              filter
-
-            existing_not_filter ->
-              %{existing_not_filter | ands: [filter | existing_not_filter.ands]}
-          end
-
-        new_filter = %{existing_filter | not: new_filter_not}
-
-        query
-        |> Map.put(:filter, new_filter)
-        |> set_data_layer_query()
     end
   end
 
@@ -349,6 +337,10 @@ defmodule Ash.Query do
     end
   end
 
+  defp maybe_filter(query, %{filter: nil}, _) do
+    {:ok, query}
+  end
+
   defp maybe_filter(query, ash_query, opts) do
     case Ash.DataLayer.filter(query, ash_query.filter, ash_query.resource) do
       {:ok, filtered} ->

lib/ash/type/type.ex

@@ -52,18 +52,6 @@ defmodule Ash.Type do
     value
   end
 
-  @spec supports_filter?(Ash.resource(), t(), Ash.DataLayer.filter_type(), Ash.data_layer()) ::
-          boolean
-  def supports_filter?(resource, type, filter_type, _data_layer) when type in @builtin_names do
-    Ash.data_layer_can?(resource, {:filter, filter_type}) and
-      filter_type in @builtins[type][:filters]
-  end
-
-  def supports_filter?(resource, type, filter_type, data_layer) do
-    Ash.data_layer_can?(resource, {:filter, filter_type}) and
-      filter_type in type.supported_filter_types(data_layer)
-  end
-
   @doc """
   Determines whether or not this value can be sorted.
   """

lib/sat_solver.ex

@@ -1,20 +1,36 @@
 defmodule Ash.SatSolver do
   @moduledoc false
 
+  alias Ash.Filter
+  alias Ash.Filter.{Expression, Not, Predicate}
+
   def strict_filter_subset(filter, candidate) do
-    filter_expr = filter_to_expr(filter)
+    case {filter, candidate} do
-    candidate_expr = filter_to_expr(candidate)
+      {%{expression: nil}, %{expression: nil}} ->
+        true
 
-    together = join_expr(filter_expr, candidate_expr, :and)
+      {%{expression: nil}, _candidate_expr} ->
+        true
 
-    separate = join_expr(negate(filter_expr), candidate_expr, :and)
+      {_filter_expr, %{expression: nil}} ->
+        false
 
-    case solve_expression(together) do
+      {filter, candidate} ->
+        do_strict_filter_subset(filter, candidate)
+    end
+  end
+
+  defp do_strict_filter_subset(filter, candidate) do
+    case add_comparisons_and_solve_expression(
+           Expression.new(:and, filter.expression, candidate.expression)
+         ) do
       {:error, :unsatisfiable} ->
         false
 
       {:ok, _} ->
-        case solve_expression(separate) do
+        case add_comparisons_and_solve_expression(
+               Expression.new(:and, Not.new(filter.expression), candidate.expression)
+             ) do
           {:error, :unsatisfiable} ->
             true
 
@@ -24,75 +40,107 @@ defmodule Ash.SatSolver do
     end
   end
 
-  defp negate(nil), do: nil
-  defp negate(expr), do: {:not, expr}
-
   defp filter_to_expr(nil), do: nil
-  defp filter_to_expr(%{impossible?: true}), do: false
+  defp filter_to_expr(false), do: false
+  defp filter_to_expr(true), do: true
+  defp filter_to_expr(%Filter{expression: expression}), do: filter_to_expr(expression)
+  defp filter_to_expr(%Predicate{} = predicate), do: predicate
+  defp filter_to_expr(%Not{expression: expression}), do: {:not, filter_to_expr(expression)}
 
-  defp filter_to_expr(%{
+  defp filter_to_expr(%Expression{op: op, left: left, right: right}) do
-         attributes: attributes,
+    {op, filter_to_expr(left), filter_to_expr(right)}
-         relationships: relationships,
+  end
-         not: not_filter,
+
-         ors: ors,
+  def add_comparisons_and_solve_expression(expression) do
-         ands: ands,
+    all_predicates =
-         path: path
+      Filter.reduce(expression, [], fn
-       }) do
+        %Predicate{} = predicate, predicates ->
-    expr =
+          [predicate | predicates]
-      Enum.reduce(attributes, nil, fn {attr, statement}, expr ->
+
-        join_expr(
+        _, predicates ->
-          expr,
+          predicates
-          tag_statement(statement_to_expr(statement), %{path: path, attr: attr}),
-          :and
-        )
       end)
 
-    expr =
+    simplified =
-      Enum.reduce(relationships, expr, fn {relationship, relationship_filter}, expr ->
+      Filter.map(expression, fn
-        join_expr(expr, {relationship, filter_to_expr(relationship_filter)}, :and)
+        %Predicate{} = predicate ->
+          predicate
+          |> find_simplification(all_predicates)
+          |> case do
+            nil ->
+              predicate
+
+            {:simplify, simplification} ->
+              simplification
+          end
+
+        other ->
+          other
       end)
 
-    expr = join_expr(negate(filter_to_expr(not_filter)), expr, :and)
+    if simplified == expression do
+      all_predicates =
+        Filter.reduce(expression, [], fn
+          %Predicate{} = predicate, predicates ->
+            [predicate | predicates]
 
-    expr =
+          _, predicates ->
-      Enum.reduce(ors, expr, fn or_filter, expr ->
+            predicates
-        join_expr(filter_to_expr(or_filter), expr, :or)
+        end)
-      end)
+        |> Enum.uniq()
 
-    Enum.reduce(ands, expr, fn and_filter, expr ->
+      comparison_expressions =
-      join_expr(filter_to_expr(and_filter), expr, :and)
+        all_predicates
+        |> Enum.reduce([], fn predicate, new_expressions ->
+          all_predicates
+          |> Enum.filter(fn other_predicate ->
+            other_predicate != predicate &&
+              other_predicate.relationship_path == predicate.relationship_path &&
+              other_predicate.attribute.name == predicate.attribute.name
+          end)
+          |> Enum.reduce(new_expressions, fn other_predicate, new_expressions ->
+            case Predicate.compare(predicate, other_predicate) do
+              inclusive when inclusive in [:right_includes_left, :mutually_inclusive] ->
+                [{:not, {:and, {:not, other_predicate}, predicate}} | new_expressions]
+
+              exclusive when exclusive in [:right_excludes_left, :mutually_exclusive] ->
+                [{:not, {:and, other_predicate, predicate}} | new_expressions]
+
+              {:simplify, _} ->
+                # Filter should be fully simplified here
+                raise "What"
+
+              _other ->
+                # If we can't tell, we assume they are exclusive statements
+                [{:not, {:and, other_predicate, predicate}} | new_expressions]
+            end
+          end)
+        end)
+        |> Enum.uniq()
+
+      expression = filter_to_expr(expression)
+
+      expression =
+        Enum.reduce(comparison_expressions, expression, fn comparison_expression, expression ->
+          {:and, comparison_expression, expression}
+        end)
+
+      solve_expression(expression)
+    else
+      add_comparisons_and_solve_expression(simplified)
+    end
+  end
+
+  defp find_simplification(predicate, predicates) do
+    predicates
+    |> Enum.find_value(fn other_predicate ->
+      case Predicate.compare(predicate, other_predicate) do
+        {:simplify, simplification} -> {:simplify, simplification}
+        _ -> false
+      end
     end)
   end
 
-  defp statement_to_expr(%Ash.Filter.NotIn{values: values}) do
-    {:not, %Ash.Filter.In{values: values}}
-  end
-
-  defp statement_to_expr(%Ash.Filter.NotEq{value: value}) do
-    {:not, %Ash.Filter.Eq{value: value}}
-  end
-
-  defp statement_to_expr(%Ash.Filter.And{left: left, right: right}) do
-    {:and, statement_to_expr(left), statement_to_expr(right)}
-  end
-
-  defp statement_to_expr(%Ash.Filter.Or{left: left, right: right}) do
-    {:or, statement_to_expr(left), statement_to_expr(right)}
-  end
-
-  defp statement_to_expr(statement), do: statement
-
-  defp tag_statement({:not, value}, tag), do: {:not, tag_statement(value, tag)}
-
-  defp tag_statement({joiner, left_value, right_value}, tag) when joiner in [:and, :or],
-    do: {joiner, tag_statement(left_value, tag), tag_statement(right_value, tag)}
-
-  defp tag_statement(statement, tag), do: {statement, tag}
-
-  defp join_expr(nil, right, _joiner), do: right
-  defp join_expr(left, nil, _joiner), do: left
-  defp join_expr(left, right, joiner), do: {joiner, left, right}
-
   def solve_expression(expression) do
     expression_with_constants = {:and, true, {:and, {:not, false}, expression}}
 

test/actions/read_test.exs

@@ -153,12 +153,16 @@ defmodule Ash.Test.Actions.ReadTest do
     end
 
     test "it raises on an error" do
-      assert_raise(Ash.Error.Invalid, ~r/Invalid filter value 10 supplied for :title == 10/, fn ->
+      assert_raise(
-        Post
+        Ash.Error.Invalid,
-        |> Api.query()
+        ~r/Invalid filter value `10` supplied in: `title == 10`/,
-        |> Ash.Query.filter(title: 10)
+        fn ->
-        |> Api.read!()
+          Post
-      end)
+          |> Api.query()
+          |> Ash.Query.filter(title: 10)
+          |> Api.read!()
+        end
+      )
     end
   end
 

test/filter/filter_test.exs

@@ -2,6 +2,8 @@ defmodule Ash.Test.Filter.FilterTest do
   @moduledoc false
   use ExUnit.Case, async: true
 
+  alias Ash.Filter
+
   defmodule Profile do
     @moduledoc false
     use Ash.Resource, data_layer: Ash.DataLayer.Ets
@@ -249,4 +251,61 @@ defmodule Ash.Test.Filter.FilterTest do
                |> Api.read!()
     end
   end
+
+  describe "filter subset logic" do
+    test "can detect a filter is a subset of itself" do
+      filter = Filter.parse!(Api, Post, %{points: 1})
+
+      assert Filter.strict_subset_of?(filter, filter)
+    end
+
+    test "can detect a filter is a subset of itself *and* something else" do
+      filter = Filter.parse!(Api, Post, points: 1)
+
+      candidate = Filter.add_to_filter!(filter, title: "Title")
+
+      assert Filter.strict_subset_of?(filter, candidate)
+    end
+
+    test "can detect a filter is not a subset of itself *or* something else" do
+      filter = Filter.parse!(Api, Post, points: 1)
+
+      candidate = Filter.add_to_filter!(filter, :or, title: "Title")
+
+      refute Filter.strict_subset_of?(filter, candidate)
+    end
+
+    test "can detect a filter is a subset based on a simplification" do
+      filter = Filter.parse!(Api, Post, points: [in: [1, 2]])
+
+      candidate = Filter.parse!(Api, Post, points: 1)
+
+      assert Filter.strict_subset_of?(filter, candidate)
+    end
+
+    test "can detect a filter is not a subset based on a simplification" do
+      filter = Filter.parse!(Api, Post, points: [in: [1, 2]])
+
+      candidate = Filter.parse!(Api, Post, points: 3)
+
+      refute Filter.strict_subset_of?(filter, candidate)
+    end
+
+    test "can detect a more complicated scenario" do
+      filter = Filter.parse!(Api, Post, or: [points: [in: [1, 2, 3]], points: 4, points: 5])
+
+      candidate = Filter.parse!(Api, Post, or: [points: 1, points: 3, points: 5])
+
+      assert Filter.strict_subset_of?(filter, candidate)
+    end
+
+    test "understands unrelated negations" do
+      filter = Filter.parse!(Api, Post, or: [points: [in: [1, 2, 3]], points: 4, points: 5])
+
+      candidate =
+        Filter.parse!(Api, Post, or: [points: 1, points: 3, points: 5], not: [points: 7])
+
+      assert Filter.strict_subset_of?(filter, candidate)
+    end
+  end
 end