diff --git a/config/config.exs b/config/config.exs index 4783528e..6c147662 100644 --- a/config/config.exs +++ b/config/config.exs @@ -30,6 +30,7 @@ if Mix.env() == :test do config :ash, :custom_expressions, [Ash.Test.Expressions.JaroDistance] config :ash, :sat_testing, true + config :ash, :no_join_mnesia_ets, :dynamic config :ash, :validate_domain_resource_inclusion?, false config :ash, :validate_domain_config_inclusion?, false diff --git a/lib/ash/data_layer/ets/ets.ex b/lib/ash/data_layer/ets/ets.ex index 86f14914..7086c35a 100644 --- a/lib/ash/data_layer/ets/ets.ex +++ b/lib/ash/data_layer/ets/ets.ex @@ -218,10 +218,24 @@ defmodule Ash.DataLayer.Ets do def can?(_, :transact), do: false def can?(_, {:filter_expr, _}), do: true - def can?(resource, {:join, other_resource}) do - # See the comment in can?/2 in mnesia data layer to explain this - not (Ash.DataLayer.Ets.Info.private?(resource) and - Ash.DataLayer.data_layer(other_resource) == Ash.DataLayer.Mnesia) + case Application.compile_env(:ash, :no_join_mnesia_ets) || false do + false -> + def can?(_, {:join, _resource}) do + # we synthesize all filters under the hood using `Ash.Filter.Runtime` + true + end + + true -> + def can?(_, {:join, _resource}) do + # we synthesize all filters under the hood using `Ash.Filter.Runtime` + false + end + + :dynamic -> + def can?(_, {:join, resource}) do + Ash.Resource.Info.data_layer(resource) == __MODULE__ || + Application.get_env(:ash, :mnesia_ets_join?, true) + end end def can?(_, :nested_expressions), do: true diff --git a/lib/ash/data_layer/mnesia/mnesia.ex b/lib/ash/data_layer/mnesia/mnesia.ex index 825af13f..d318616e 100644 --- a/lib/ash/data_layer/mnesia/mnesia.ex +++ b/lib/ash/data_layer/mnesia/mnesia.ex @@ -112,12 +112,24 @@ defmodule Ash.DataLayer.Mnesia do def can?(_, {:aggregate, :exists}), do: true def can?(resource, {:query_aggregate, kind}), do: can?(resource, {:aggregate, kind}) - def can?(_, {:join, resource}) do - # This is to ensure that these can't join, which is necessary for testing - # if someone needs to use these both and *actually* needs real joins for private - # ets resources then we can talk about making this only happen in ash tests - not (Ash.DataLayer.data_layer(resource) == Ash.DataLayer.Ets && - Ash.DataLayer.Ets.Info.private?(resource)) + case Application.compile_env(:ash, :no_join_mnesia_ets) || false do + false -> + def can?(_, {:join, _resource}) do + # we synthesize all filters under the hood using `Ash.Filter.Runtime` + true + end + + true -> + def can?(_, {:join, _resource}) do + # we synthesize all filters under the hood using `Ash.Filter.Runtime` + false + end + + :dynamic -> + def can?(_, {:join, resource}) do + Ash.Resource.Info.data_layer(resource) == __MODULE__ || + Application.get_env(:ash, :mnesia_ets_join?, true) + end end def can?(_, {:filter_expr, _}), do: true diff --git a/lib/ash/filter/filter.ex b/lib/ash/filter/filter.ex index b2117fed..23acbca1 100644 --- a/lib/ash/filter/filter.ex +++ b/lib/ash/filter/filter.ex @@ -1383,37 +1383,7 @@ defmodule Ash.Filter do case Ash.Actions.Read.unpaginated_read(query, relationship.read_action) do {:ok, data} -> - pkey = Ash.Resource.Info.primary_key(related) - - expr = - Enum.reduce(data, nil, fn item, expr -> - new_expr = - Enum.reduce(pkey, nil, fn key, expr -> - {:ok, new_expr} = - Ash.Query.Operator.new( - Ash.Query.Operator.Eq, - %Ash.Query.Ref{ - attribute: key, - relationship_path: at_path - }, - Map.get(item, key) - ) - - if expr do - Ash.Query.BooleanExpression.new(:and, expr, new_expr) - else - new_expr - end - end) - - if expr do - Ash.Query.BooleanExpression.new(:or, expr, new_expr) - else - new_expr - end - end) - - {:ok, expr} + records_to_expression(data, relationship, at_path) {:error, error} -> {:error, error} @@ -1688,6 +1658,24 @@ defmodule Ash.Filter do defp records_to_expression([], _, _), do: {:ok, false} + defp records_to_expression(_, %{no_attributes?: true}, _), do: {:ok, true} + + defp records_to_expression([single_record], %{type: :many_to_many} = relationship, path) do + Ash.Query.Operator.new( + Eq, + %Ref{ + relationship_path: path ++ [relationship.join_relationship], + resource: relationship.through, + attribute: + Ash.Resource.Info.attribute( + relationship.through, + relationship.destination_attribute_on_join_resource + ) + }, + Map.get(single_record, relationship.destination_attribute) + ) + end + defp records_to_expression([single_record], relationship, path) do Ash.Query.Operator.new( Eq, @@ -1701,10 +1689,14 @@ defmodule Ash.Filter do end defp records_to_expression(records, relationship, path) do - Enum.reduce_while(records, {:ok, true}, fn record, {:ok, expression} -> + Enum.reduce_while(records, {:ok, nil}, fn record, {:ok, expression} -> case records_to_expression([record], relationship, path) do {:ok, operator} -> - {:cont, {:ok, BooleanExpression.optimized_new(:and, expression, operator)}} + if is_nil(expression) do + {:cont, {:ok, operator}} + else + {:cont, {:ok, BooleanExpression.optimized_new(:or, expression, operator)}} + end {:error, error} -> {:halt, {:error, error}} @@ -3021,6 +3013,9 @@ defmodule Ash.Filter do [] -> :ok + [{_data_layer, [_]}] -> + :ok + [{_data_layer, resources}] -> can_join? = Enum.all?(resources, fn resource -> diff --git a/test/filter/filter_interaction_test.exs b/test/filter/filter_interaction_test.exs index 94b3b8d7..a8256387 100644 --- a/test/filter/filter_interaction_test.exs +++ b/test/filter/filter_interaction_test.exs @@ -117,11 +117,15 @@ defmodule Ash.Test.Filter.FilterInteractionTest do end setup do + Application.put_env(:ash, :mnesia_ets_join?, false) + capture_log(fn -> Mnesia.start(Domain, [Post, PostLink]) end) on_exit(fn -> + Application.put_env(:ash, :mnesia_ets_join?, true) + capture_log(fn -> :mnesia.stop() :mnesia.delete_schema([node()]) @@ -175,6 +179,47 @@ defmodule Ash.Test.Filter.FilterInteractionTest do assert [%{id: ^post1_id}] = Ash.read!(query) end + test "it properly filters with a simple filter and multiple matches" do + author = + User + |> Ash.Changeset.for_create(:create, %{name: "best author"}) + |> Ash.create!() + + author2 = + User + |> Ash.Changeset.for_create(:create, %{name: "best author"}) + |> Ash.create!() + + author3 = + User + |> Ash.Changeset.for_create(:create, %{name: "worst author"}) + |> Ash.create!() + + post1 = + Post + |> Ash.Changeset.for_create(:create, %{title: "best"}) + |> Ash.Changeset.manage_relationship(:author, author, type: :append_and_remove) + |> Ash.create!() + + post2 = + Post + |> Ash.Changeset.for_create(:create, %{title: "best"}) + |> Ash.Changeset.manage_relationship(:author, author2, type: :append_and_remove) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "best"}) + |> Ash.Changeset.manage_relationship(:author, author3, type: :append_and_remove) + |> Ash.create!() + + query = + Post + |> Ash.Query.filter(author.name == "best author") + + assert query |> Ash.read!() |> Enum.map(& &1.id) |> Enum.sort() == + Enum.sort([post1.id, post2.id]) + end + test "parallelizable filtering of related resources with a data layer that cannot join" do post2 = Post @@ -262,9 +307,6 @@ defmodule Ash.Test.Filter.FilterInteractionTest do |> Ash.Changeset.manage_relationship(:related_posts, [post3], type: :append_and_remove) |> Ash.create!() - post2 - |> Ash.load!(:related_posts) - query = Post |> Ash.Query.filter(exists(related_posts, title == "two"))