WIP

README.md

@@ -170,3 +170,4 @@ end
 - check if preparations have been done on a superset filter of a request and, if so, use it
 - without transactions, we can't ensure that all changes are rolled back in the case that relationship updates are included. Don't think there is really anything to do about that, but something worth considering.
 - perhaps have auth steps express which fields need to be present, so we can avoid loading things unnecessarily
+- lift `or` filters over the same field equaling a value into a single `in` filter, for performance (potentially)

lib/ash/actions/create.ex

@@ -3,21 +3,7 @@ defmodule Ash.Actions.Create do
   alias Ash.Actions.{Attributes, Relationships, SideLoad}
   require Logger
 
-  @spec run(Ash.api(), Ash.resource(), Ash.action(), Ash.params()) ::
-          {:ok, Ash.record()} | {:error, Ecto.Changeset.t()} | {:error, Ash.error()}
   def run(api, resource, action, params) do
-    transaction_result =
-      Ash.DataLayer.transact(resource, fn ->
-        do_run(api, resource, action, params)
-      end)
-
-    case transaction_result do
-      {:ok, value} -> value
-      {:error, error} -> {:error, error}
-    end
-  end
-
-  defp do_run(api, resource, action, params) do
     attributes = Keyword.get(params, :attributes, %{})
     side_loads = Keyword.get(params, :side_load, [])
     side_load_filter = Keyword.get(params, :side_load_filter)

lib/ash/actions/destroy.ex

@@ -4,18 +4,6 @@ defmodule Ash.Actions.Destroy do
   @spec run(Ash.api(), Ash.record(), Ash.action(), Ash.params()) ::
           {:ok, Ash.record()} | {:error, Ecto.Changeset.t()} | {:error, Ash.error()}
   def run(api, %resource{} = record, action, params) do
-    transaction_result =
-      Ash.DataLayer.transact(resource, fn ->
-        do_authorized(api, params, action, record)
-      end)
-
-    case transaction_result do
-      {:ok, value} -> value
-      {:error, error} -> {:error, error}
-    end
-  end
-
-  defp do_authorized(api, params, action, %resource{} = record) do
     auth_request =
       Ash.Engine.Request.new(
         resource: resource,
@@ -24,7 +12,7 @@ defmodule Ash.Actions.Destroy do
         strict_access: false,
         path: [:data],
         data:
-          Ash.Engine.Request.UnresolvedField.data([], fn _request, _ ->
+          Ash.Engine.Request.UnresolvedField.data([], fn _ ->
             case Ash.data_layer(resource).destroy(record) do
               :ok -> {:ok, record}
               {:error, error} -> {:error, error}
@@ -34,15 +22,21 @@ defmodule Ash.Actions.Destroy do
         resolve_when_fetch_only?: true
       )
 
-    if params[:authorization] do
+    result =
-      Engine.run(
+      if params[:authorization] do
-        [auth_request],
+        Engine.run(
-        api,
+          [auth_request],
-        user: params[:authorization][:user],
+          api,
-        log_final_report?: params[:authorization][:log_final_report?]
+          user: params[:authorization][:user],
-      )
+          log_final_report?: params[:authorization][:log_final_report?]
-    else
+        )
-      Engine.run([auth_request], api, fetch_only?: true)
+      else
+        Engine.run([auth_request], api, fetch_only?: true)
+      end
+
+    case result do
+      %{errors: errors} when errors == %{} -> :ok
+      %{errors: errors} -> {:error, errors}
     end
   end
 end

lib/ash/actions/read.ex

@@ -5,18 +5,6 @@ defmodule Ash.Actions.Read do
   require Logger
 
   def run(api, resource, action, params) do
-    transaction_result =
-      Ash.DataLayer.transact(resource, fn ->
-        do_run(api, resource, action, params)
-      end)
-
-    case transaction_result do
-      {:ok, value} -> value
-      {:error, error} -> {:error, error}
-    end
-  end
-
-  defp do_run(api, resource, action, params) do
     filter = Keyword.get(params, :filter, [])
     sort = Keyword.get(params, :sort, [])
     side_loads = Keyword.get(params, :side_load, [])

lib/ash/actions/relationships/create.ex

@@ -1,43 +0,0 @@
-# defmodule Ash.Actions.Relationships.Create do
-#   alias Ash.Actions.Relationships.Change
-
-#   def changeset(changeset, api, relationships) do
-#     relationship_changes = relationship_changes(relationships)
-
-#     changeset
-#   end
-
-#   defp relationship_changes(relationships) do
-#     Enum.into(relationships, %{}, fn {key, value} ->
-#       {key, Change.from(value, :create)}
-#     end)
-#   end
-
-#   # def changeset(changeset, api, relationships) do
-#   #   if relationships == %{} do
-#   #     changeset
-#   #   else
-#   #     dependencies = Map.get(changeset, :__changes_depend_on__, [])
-
-#   #     Ash.Engine.Request.UnresolvedField.field(dependencies, fn data ->
-#   #       new_changeset =
-#   #         data
-#   #         |> Map.get(:relationships, %{})
-#   #         |> Enum.reduce(changeset, fn {relationship, relationship_data}, changeset ->
-#   #           relationship = Ash.relationship(changeset.data.__struct__, relationship)
-
-#   #           relationship_data =
-#   #             relationship_data
-#   #             |> Enum.into(%{}, fn {key, value} ->
-#   #               {key, value.data}
-#   #             end)
-#   #             |> Map.put_new(:current, [])
-
-#   #           add_relationship_to_changeset(changeset, api, relationship, relationship_data)
-#   #         end)
-
-#   #       {:ok, new_changeset}
-#   #     end)
-#   #   end
-#   # end
-# end

lib/ash/actions/relationships/relationships.ex

@@ -1,5 +0,0 @@
-# defmodule Ash.Actions.Relationships do
-#   defmodule Change do
-#     defstruct [:add, :remove, :current]
-#   end
-# end

lib/ash/actions/side_load.ex

@@ -91,16 +91,18 @@ defmodule Ash.Actions.SideLoad do
     end)
     |> Enum.reduce(data, fn {key, %{data: value}}, data ->
       last_relationship = last_relationship!(resource, key)
+      lead_path = :lists.droplast(key)
 
       case last_relationship do
         %{type: :many_to_many, name: name} ->
           # TODO: If we sort the relationships as we do them (doing the join assoc first)
           # then we can just use those linked assocs (maybe)
           join_association = String.to_existing_atom(to_string(name) <> "_join_assoc")
-          join_path = :lists.droplast(key) ++ [join_association]
+
+          join_path = lead_path ++ [join_association]
           join_data = Map.get(includes, join_path, [])
 
-          map_or_update(data, fn record ->
+          map_or_update(data, lead_path, fn record ->
             source_value = Map.get(record, last_relationship.source_field)
 
             join_values =
@@ -125,7 +127,7 @@ defmodule Ash.Actions.SideLoad do
         %{cardinality: :many} ->
           values = Enum.group_by(value, &Map.get(&1, last_relationship.destination_field))
 
-          map_or_update(data, fn record ->
+          map_or_update(data, lead_path, fn record ->
             source_key = Map.get(record, last_relationship.source_field)
             related_records = Map.get(values, source_key, [])
             Map.put(record, last_relationship.name, related_records)
@@ -137,7 +139,7 @@ defmodule Ash.Actions.SideLoad do
               {Map.get(item, last_relationship.destination_field), item}
             end)
 
-          map_or_update(data, fn record ->
+          map_or_update(data, lead_path, fn record ->
             source_key = Map.get(record, last_relationship.source_field)
             related_record = Map.get(values, source_key)
             Map.put(record, last_relationship.name, related_record)
@@ -156,12 +158,18 @@ defmodule Ash.Actions.SideLoad do
     data
   end
 
-  defp map_or_update(record, func) when not is_list(record), do: func.(record)
+  defp map_or_update(record, [], func) when not is_list(record), do: func.(record)
 
-  defp map_or_update(records, func) do
+  defp map_or_update(records, [], func) do
     Enum.map(records, func)
   end
 
+  defp map_or_update(records, [path | tail], func) do
+    map_or_update(records, [], fn record ->
+      Map.update!(record, path, &map_or_update(&1, tail, func))
+    end)
+  end
+
   defp last_relationship!(resource, [last]) do
     Ash.relationship(resource, last) || raise "Assumption Failed"
   end
@@ -177,7 +185,7 @@ defmodule Ash.Actions.SideLoad do
            {:rel, Ash.relationship(resource, key)},
          nested_path <- path ++ [relationship],
          {:ok, requests} <-
-           requests(api, relationship.destination, further, filters, nested_path) do
+           requests(api, relationship.destination, further, filters, root_filter, nested_path) do
       default_read =
         Ash.primary_action(relationship.destination, :read) ||
           raise "Must set default read for #{inspect(resource)}"
@@ -211,7 +219,7 @@ defmodule Ash.Actions.SideLoad do
           path: [:include, Enum.map(nested_path, &Map.get(&1, :name))],
           resolve_when_fetch_only?: true,
           filter:
-            side_load_filter2(
+            side_load_filter(
               relationship,
               Map.get(filters || %{}, source, []),
               nested_path,
@@ -237,6 +245,7 @@ defmodule Ash.Actions.SideLoad do
               # or for doing many to many joins, but can be slower.
               # If the relationship is already loaded, we should consider doing an in-memory filtering
               # Right now, we just use the original query
+
               with {:ok, filter} <-
                      true_side_load_filter(
                        relationship,
@@ -275,7 +284,7 @@ defmodule Ash.Actions.SideLoad do
               strict_access?: root_filter not in [:create, :update],
               resolve_when_fetch_only?: true,
               filter:
-                side_load_filter2(
+                side_load_filter(
                   Ash.relationship(resource, join_relationship.name),
                   [],
                   nested_path,
@@ -325,7 +334,7 @@ defmodule Ash.Actions.SideLoad do
     end
   end
 
-  defp side_load_filter2(
+  defp side_load_filter(
          %{reverse_relationship: nil, type: :many_to_many} = relationship,
          _request_filter,
          _prior_path,
@@ -338,7 +347,7 @@ defmodule Ash.Actions.SideLoad do
     end)
   end
 
-  defp side_load_filter2(
+  defp side_load_filter(
          relationship,
          request_filter,
          prior_path,
@@ -372,13 +381,13 @@ defmodule Ash.Actions.SideLoad do
         {:ok, reverse_path} ->
           Ash.Filter.parse(
             relationship.destination,
-            put_nested_relationship(request_filter, reverse_path, root_filter)
+            put_nested_relationship(request_filter, reverse_path, root_filter, false)
           )
       end
     end)
   end
 
-  defp side_load_filter2(
+  defp side_load_filter(
          relationship,
          request_filter,
          prior_path,
@@ -416,7 +425,7 @@ defmodule Ash.Actions.SideLoad do
       {:ok, reverse_path} ->
         Ash.Filter.parse(
           relationship.destination,
-          put_nested_relationship(request_filter, reverse_path, root_filter)
+          put_nested_relationship(request_filter, reverse_path, root_filter, false)
         )
 
       :error ->
@@ -450,17 +459,22 @@ defmodule Ash.Actions.SideLoad do
           Map.get(data, :data)
 
         path ->
-          Map.get(data, [:include, Enum.reverse(path)])
+          path_names = path |> Enum.reverse() |> Enum.map(& &1.name)
+
+          data
+          |> Map.get(:include, %{})
+          |> Map.get(path_names, %{})
       end
 
-    values = get_fields(source_data.data, pkey)
+    related_data = Map.get(source_data || %{}, :data, [])
 
     cond do
       reverse_relationship ->
+        values = get_fields(related_data, pkey)
         {:ok, put_nested_relationship(filter, [reverse_relationship], values)}
 
       true ->
-        ids = Enum.map(source_data.data, &Map.get(&1, relationship.source_field))
+        ids = Enum.map(related_data, &Map.get(&1, relationship.source_field))
 
         filter_value =
           case ids do
@@ -588,30 +602,33 @@ defmodule Ash.Actions.SideLoad do
   #   |> get_field(name, rest)
   # end
 
-  defp put_nested_relationship(_, _, []), do: [__impossible__: true]
+  defp put_nested_relationship(request_filter, path, value, records? \\ true)
-  defp put_nested_relationship(_, _, nil), do: [__impossible__: true]
+  defp put_nested_relationship(_, _, [], true), do: [__impossible__: true]
+  defp put_nested_relationship(_, _, nil, true), do: [__impossible__: true]
+  defp put_nested_relationship(_, _, [], false), do: []
+  defp put_nested_relationship(_, _, nil, false), do: []
 
-  defp put_nested_relationship(request_filter, path, value) when not is_list(value) do
+  defp put_nested_relationship(request_filter, path, value, records?) when not is_list(value) do
-    put_nested_relationship(request_filter, path, [value])
+    put_nested_relationship(request_filter, path, [value], records?)
   end
 
-  defp put_nested_relationship(request_filter, [rel | rest], values) do
+  defp put_nested_relationship(request_filter, [rel | rest], values, records?) do
     [
-      {rel, put_nested_relationship(request_filter, rest, values)}
+      {rel, put_nested_relationship(request_filter, rest, values, records?)}
     ]
   end
 
-  defp put_nested_relationship(request_filter, [], [[{field, _}] | _] = keys) do
+  defp put_nested_relationship(request_filter, [], [[{field, _}] | _] = keys, _) do
     add_relationship_id_filter(request_filter, field, Enum.map(keys, &elem(&1, 1)))
   end
 
-  defp put_nested_relationship(request_filter, [], [values]) do
+  defp put_nested_relationship(request_filter, [], [values], _) do
     Enum.reduce(values, request_filter, fn {field, value}, filter ->
       add_relationship_id_filter(filter, field, [value])
     end)
   end
 
-  defp put_nested_relationship(request_filter, [], values) do
+  defp put_nested_relationship(request_filter, [], values, _) do
     Keyword.update(request_filter, :or, values, &Kernel.++(&1, values))
   end
 

lib/ash/api/interface.ex

@@ -467,6 +467,7 @@ defmodule Ash.Api.Interface do
     end
   end
 
+  defp unwrap_or_raise!(:ok), do: :ok
   defp unwrap_or_raise!({:ok, result}), do: result
 
   defp unwrap_or_raise!({:error, error}) when is_bitstring(error) do

lib/ash/data_layer/ets.ex

@@ -69,6 +69,9 @@ 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},

lib/ash/filter/filter.ex

@@ -9,7 +9,7 @@ defmodule Ash.Filter do
     requests: [],
     path: [],
     errors: [],
-    impossible: false
+    impossible?: false
   ]
 
   alias Ash.Engine.Request
@@ -23,7 +23,7 @@ defmodule Ash.Filter do
           attributes: Keyword.t(),
           relationships: Map.t(),
           path: list(atom),
-          impossible: boolean,
+          impossible?: boolean,
           errors: list(String.t()),
           requests: list(Ash.Engine.Request.t())
         }
@@ -258,7 +258,7 @@ defmodule Ash.Filter do
           # TODO: We should probably include some kind of filter that *makes* it immediately impossible
           # that way, if the data layer doesn't check impossibility they will run the simpler query,
           # like for each pkey field say `[field: [in: []]]`
-          %{filter | impossible: true}
+          %{filter | impossible?: true}
         else
           filter
         end
@@ -293,14 +293,14 @@ defmodule Ash.Filter do
 
   defp lift_impossibility(filter) do
     with_related_impossibility =
-      if Enum.any?(filter.relationships || %{}, fn {_, val} -> Map.get(val, :impossible) end) do
+      if Enum.any?(filter.relationships || %{}, fn {_, val} -> Map.get(val, :impossible?) end) do
-        Map.put(filter, :impossible, true)
+        Map.put(filter, :impossible?, true)
       else
         filter
       end
 
     Map.update!(with_related_impossibility, :ors, fn ors ->
-      Enum.reject(ors, &Map.get(&1, :impossible))
+      Enum.reject(ors, &Map.get(&1, :impossible?))
     end)
   end
 
@@ -407,6 +407,9 @@ defmodule Ash.Filter do
     Enum.reduce(filter_statement, filter, fn
       {key, value}, filter ->
         cond do
+          key == :__impossible__ && value == true ->
+            %{filter | impossible?: true}
+
           key in [:or, :and, :not] ->
             new_filter = add_expression_level_boolean_filter(filter, resource, key, value)
 

lib/ash/filter/inspect.ex

@@ -70,7 +70,7 @@ defimpl Inspect, for: Ash.Filter do
           ors: ors,
           relationships: relationships,
           attributes: attributes,
-          impossible: impossible
+          impossible?: impossible
         },
         opts
       )
@@ -89,7 +89,7 @@ defimpl Inspect, for: Ash.Filter do
     end
   end
 
-  def inspect(%{impossible: impossible} = filter, opts) do
+  def inspect(%{impossible?: impossible} = filter, opts) do
     rels =
       filter
       |> Map.get(:relationships)

lib/ash/resource/relationships/relationships.ex

@@ -243,7 +243,7 @@ defmodule Ash.Resource.Relationships do
           has_many_name,
           unquote(config)[:through],
           destination_field: unquote(config)[:source_field_on_join_table],
-          source_field: unquote(config)[:source_field]
+          source_field: unquote(config)[:source_field] || :id
         )
 
       with {:many_to_many, {:ok, many_to_many}} <- {:many_to_many, many_to_many},

test/actions/destroy_test.exs

@@ -82,7 +82,7 @@ defmodule Ash.Test.Actions.DestroyTest do
     test "allows destroying a record" do
       post = Api.create!(Post, attributes: %{title: "foo", contents: "bar"})
 
-      assert Api.destroy!(post) == post
+      assert Api.destroy!(post) == :ok
 
       refute Api.get!(Post, post.id)
     end

test/resource/relationships/many_to_many_test.exs

@@ -12,7 +12,7 @@ defmodule Ash.Test.Resource.Relationships.ManyToManyTest do
   end
 
   describe "representation" do
-    test "it creates a relationship" do
+    test "it creates a relationship and a join relationship" do
       defposts do
         relationships do
           many_to_many :foobars, Foobar, through: SomeResource
@@ -20,16 +20,28 @@ defmodule Ash.Test.Resource.Relationships.ManyToManyTest do
       end
 
       assert [
+               %Ash.Resource.Relationships.HasMany{
+                 cardinality: :many,
+                 destination: SomeResource,
+                 destination_field: :posts_id,
+                 name: :foobars_join_assoc,
+                 source: Ash.Test.Resource.Relationships.ManyToManyTest.Post,
+                 source_field: :id,
+                 type: :has_many,
+                 write_rules: []
+               },
                %Ash.Resource.Relationships.ManyToMany{
                  cardinality: :many,
                  destination: Foobar,
                  destination_field: :id,
                  destination_field_on_join_table: :foobars_id,
                  name: :foobars,
+                 source: Ash.Test.Resource.Relationships.ManyToManyTest.Post,
                  source_field: :id,
                  source_field_on_join_table: :posts_id,
                  through: SomeResource,
-                 type: :many_to_many
+                 type: :many_to_many,
+                 write_rules: []
                }
              ] = Ash.relationships(Post)
     end