From 007e0fb081cc8791509e92105768619a9a2d1d24 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Tue, 21 Feb 2023 19:05:40 -0500 Subject: [PATCH] improvement: don't eager load sort data --- lib/ash/actions/read.ex | 79 ++++++++++++++------------ lib/ash/actions/sort.ex | 43 +++++++++++--- lib/ash/data_layer/ets/ets.ex | 2 +- lib/ash/data_layer/mnesia/mnesia.ex | 2 +- lib/ash/data_layer/simple/simple.ex | 2 +- lib/ash/embeddable_type.ex | 2 +- lib/ash/filter/runtime.ex | 87 ++++++++++++++++++++++++----- lib/ash/query/query.ex | 36 +++++++++++- lib/ash/sort/sort.ex | 2 +- test/calculation_test.exs | 66 ++++++++++++++++++++++ 10 files changed, 259 insertions(+), 62 deletions(-) diff --git a/lib/ash/actions/read.ex b/lib/ash/actions/read.ex index 9e80bfdb..9502c495 100644 --- a/lib/ash/actions/read.ex +++ b/lib/ash/actions/read.ex @@ -750,7 +750,13 @@ defmodule Ash.Actions.Read do end) |> Enum.concat(calc_selects) |> Enum.concat(query_selects) - |> remove_already_selected(request_opts[:initial_data]) + |> then(fn fields -> + if request_opts[:lazy?] do + remove_already_selected(fields, request_opts[:initial_data]) + else + fields + end + end) else [] end @@ -1043,44 +1049,44 @@ defmodule Ash.Actions.Read do defp loaded_query(query, calculations_at_runtime) do query = load_calc_requirements(query, calculations_at_runtime) - query = - Enum.reduce(query.sort || [], query, fn - {%Ash.Query.Calculation{name: name, module: module, opts: opts} = calculation, _}, - query -> - {resource_load, resource_select} = - if resource_calculation = Ash.Resource.Info.calculation(query.resource, name) do - {resource_calculation.load, resource_calculation.select} - else - {[], []} - end + # query = + # Enum.reduce(query.sort || [], query, fn + # {%Ash.Query.Calculation{name: name, module: module, opts: opts} = calculation, _}, + # query -> + # {resource_load, resource_select} = + # if resource_calculation = Ash.Resource.Info.calculation(query.resource, name) do + # {resource_calculation.load, resource_calculation.select} + # else + # {[], []} + # end - fields_to_select = - resource_select - |> Kernel.||([]) - |> Enum.concat(module.select(query, opts, calculation.context) || []) + # fields_to_select = + # resource_select + # |> Kernel.||([]) + # |> Enum.concat(module.select(query, opts, calculation.context) || []) - calculation = %{calculation | load: name, select: fields_to_select} + # calculation = %{calculation | load: name, select: fields_to_select} - query = - Ash.Query.load( - query, - module.load( - query, - opts, - Map.put(calculation.context, :context, query.context) - ) - |> Ash.Actions.Helpers.validate_calculation_load!(module) - ) + # query = + # Ash.Query.load( + # query, + # module.load( + # query, + # opts, + # Map.put(calculation.context, :context, query.context) + # ) + # |> Ash.Actions.Helpers.validate_calculation_load!(module) + # ) - Ash.Query.load(query, resource_load) + # Ash.Query.load(query, resource_load) - {key, _value}, query -> - if Ash.Resource.Info.aggregate(query.resource, key) do - Ash.Query.load(query, key) - else - query - end - end) + # {key, _value}, query -> + # if Ash.Resource.Info.aggregate(query.resource, key) do + # Ash.Query.load(query, key) + # else + # query + # end + # end) query.load |> Enum.reduce(query, fn @@ -1141,8 +1147,9 @@ defmodule Ash.Actions.Read do load_calc_requirements(query, new_loads, new_loads ++ checked) end - defp remove_already_selected(fields, %{results: results}), - do: remove_already_selected(fields, results) + defp remove_already_selected(fields, %struct{results: results}) + when struct in [Ash.Page.Keyset, Ash.Page.Offset], + do: remove_already_selected(fields, results) defp remove_already_selected(fields, record) when not is_list(record), do: remove_already_selected(fields, List.wrap(record)) diff --git a/lib/ash/actions/sort.ex b/lib/ash/actions/sort.ex index 60e7d774..11f53d13 100644 --- a/lib/ash/actions/sort.ex +++ b/lib/ash/actions/sort.ex @@ -243,20 +243,49 @@ defmodule Ash.Actions.Sort do end end - def runtime_sort([], _empty), do: [] - def runtime_sort(results, empty) when empty in [nil, []], do: results + @doc """ + Sort records at runtime - def runtime_sort([%resource{} | _] = results, [{field, direction}]) do - sort_by(results, &resolve_field(&1, field, resource), direction) - end + Opts - def runtime_sort([%resource{} | _] = results, [{field, direction} | rest]) do + * `:api` - The api to use if data needs to be loaded + * `:lazy?` - Wether to use already loaded values or to re-load them when necessary. Defaults to `false` + """ + def runtime_sort(results, sort, opts \\ []) + def runtime_sort([], _empty, _), do: [] + def runtime_sort(results, empty, _) when empty in [nil, []], do: results + def runtime_sort([single_result], _, _), do: [single_result] + + def runtime_sort([%resource{} | _] = results, [{field, direction} | rest], opts) do results + |> load_field(field, resource, opts) |> Enum.group_by(&resolve_field(&1, field, resource)) |> sort_by(fn {key, _value} -> key end, direction) |> Enum.flat_map(fn {_, records} -> - runtime_sort(records, rest) + runtime_sort(records, rest, Keyword.put(opts, :rekey?, false)) end) + |> tap(fn new_results -> + if opts[:rekey?] do + Enum.map(new_results, fn new_result -> + Enum.find(results, fn result -> + resource.primary_key_matches?(new_result, result) + end) + end) + end + end) + end + + defp load_field(records, field, resource, opts) do + if is_nil(opts[:api]) || (opts[:lazy?] && Ash.Resource.loaded?(records, field)) do + records + else + query = + resource + |> Ash.Query.load(field) + |> Ash.Query.set_context(%{private: %{internal?: true}}) + + opts[:api].load!(records, query) + end end defp resolve_field(record, %Ash.Query.Calculation{} = calc, resource) do diff --git a/lib/ash/data_layer/ets/ets.ex b/lib/ash/data_layer/ets/ets.ex index 26d79bda..d4d27411 100644 --- a/lib/ash/data_layer/ets/ets.ex +++ b/lib/ash/data_layer/ets/ets.ex @@ -337,7 +337,7 @@ defmodule Ash.DataLayer.Ets do do_add_calculations(records, resource, calculations) do offset_records = records - |> Sort.runtime_sort(sort) + |> Sort.runtime_sort(sort, api: api) |> Enum.drop(offset || 0) if limit do diff --git a/lib/ash/data_layer/mnesia/mnesia.ex b/lib/ash/data_layer/mnesia/mnesia.ex index 15161bfa..281cc444 100644 --- a/lib/ash/data_layer/mnesia/mnesia.ex +++ b/lib/ash/data_layer/mnesia/mnesia.ex @@ -219,7 +219,7 @@ defmodule Ash.DataLayer.Mnesia do {:ok, filtered} -> offset_records = filtered - |> Sort.runtime_sort(sort) + |> Sort.runtime_sort(sort, api: api) |> Enum.drop(offset || 0) limited_records = diff --git a/lib/ash/data_layer/simple/simple.ex b/lib/ash/data_layer/simple/simple.ex index 13a9abf6..06db7eb6 100644 --- a/lib/ash/data_layer/simple/simple.ex +++ b/lib/ash/data_layer/simple/simple.ex @@ -61,7 +61,7 @@ defmodule Ash.DataLayer.Simple do {:ok, results} -> {:ok, results - |> Ash.Actions.Sort.runtime_sort(sort) + |> Ash.Actions.Sort.runtime_sort(sort, api: api) |> then(fn data -> if limit do Enum.take(data, limit) diff --git a/lib/ash/embeddable_type.ex b/lib/ash/embeddable_type.ex index 80a7a1cc..30ed6cd2 100644 --- a/lib/ash/embeddable_type.ex +++ b/lib/ash/embeddable_type.ex @@ -289,7 +289,7 @@ defmodule Ash.EmbeddableType do def apply_constraints(nil, _), do: {:ok, nil} def apply_constraints(term, constraints) do - ShadowApi.load(term, constraints[:load] || []) + ShadowApi.load(term, constraints[:load] || [], lazy?: true) end def handle_change(nil, new_value, _constraints) do diff --git a/lib/ash/filter/runtime.ex b/lib/ash/filter/runtime.ex index 460ac18e..cb3e5a72 100644 --- a/lib/ash/filter/runtime.ex +++ b/lib/ash/filter/runtime.ex @@ -36,12 +36,30 @@ defmodule Ash.Filter.Runtime do resource end + {refs_to_load, refs} = + filter + |> Ash.Filter.list_refs() + |> Enum.reject(&match?(%{attribute: %Ash.Resource.Attribute{}}, &1)) + |> Enum.split_with(&(&1.relationship_path == [])) + + refs_to_load = + refs_to_load + |> Enum.map(fn + %{attribute: %Ash.Resource.Calculation{load: nil} = calc} -> + {calc.name, calc} + + %{attribute: %{name: name}} -> + name + end) + + records = api.load!(records, refs_to_load) + filter |> Ash.Filter.relationship_paths(true) |> Enum.reject(&(&1 == [])) |> Enum.uniq() |> Enum.reject(&Ash.Resource.loaded?(records, &1)) - |> Enum.map(&path_to_load(resource, &1)) + |> Enum.map(&path_to_load(resource, &1, refs)) |> case do [] -> Enum.reduce_while(records, {:ok, []}, fn record, {:ok, records} -> @@ -85,20 +103,24 @@ defmodule Ash.Filter.Runtime do def load_parent_requirements(api, expression, %resource{} = parent) do expression |> Ash.Filter.flat_map(fn %Ash.Query.Parent{expr: expr} -> - expr - |> Ash.Filter.relationship_paths(true) - |> Enum.reject(&(&1 == [])) + Ash.Filter.list_refs(expr) end) + |> Enum.reject(&match?(%{attribute: %Ash.Resource.Attribute{}}, &1)) |> Enum.uniq() - |> Enum.reject(&Ash.Resource.loaded?(parent, &1)) |> case do [] -> {:ok, parent} - requirements -> + refs -> + to_load = + refs + |> Enum.map(& &1.relationship_path) + |> Enum.uniq() + |> Enum.map(&path_to_load(resource, &1, refs)) + query = resource - |> Ash.Query.load(requirements) + |> Ash.Query.load(to_load) |> Ash.Query.set_context(%{private: %{internal?: true}}) api.load(parent, query) @@ -134,7 +156,7 @@ defmodule Ash.Filter.Runtime do end) need_to_load -> - {:load, Enum.map(need_to_load, &path_to_load(resource, &1))} + {:load, Enum.map(need_to_load, &path_to_load(resource, &1, []))} end end @@ -626,14 +648,53 @@ defmodule Ash.Filter.Runtime do defp or_default(other, _), do: other - defp path_to_load(resource, [first]) do - related = Ash.Resource.Info.related(resource, first) - {first, Ash.Query.set_context(related, %{private: %{internal?: true}})} + defp path_to_load(resource, [first], refs) do + to_load = + refs + |> Enum.filter(&(&1.relationship_path == [])) + |> Enum.map(fn + %{attribute: %Ash.Resource.Calculation{load: nil} = calc} -> + {calc.name, calc} + + %{attribute: %{name: name}} -> + name + end) + + query = + resource + |> Ash.Resource.Info.related(first) + |> Ash.Query.set_context(%{private: %{internal?: true}}) + |> Ash.Query.load(to_load) + + {first, query} end - defp path_to_load(resource, [first | rest]) do + defp path_to_load(resource, [first | rest], refs) do related = Ash.Resource.Info.related(resource, first) - {first, [path_to_load(related, rest)]} + + to_load = + refs + |> Enum.filter(&(&1.relationship_path == [])) + |> Enum.map(fn + %{attribute: %Ash.Resource.Calculation{load: nil} = calc} -> + {calc.name, calc} + + %{attribute: %{name: name}} -> + name + end) + + further_refs = + refs + |> Enum.filter(fn ref -> + ref.relationship_path + |> Enum.at(0) + |> Kernel.==(first) + end) + |> Enum.map(fn ref -> + %{ref | relationship_path: Enum.drop(ref.relationship_path, 1)} + end) + + {first, [path_to_load(related, rest, further_refs)] ++ to_load} end defp expression_matches(:and, left, right, record, parent) do diff --git a/lib/ash/query/query.ex b/lib/ash/query/query.ex index c389fd7c..8a2082ba 100644 --- a/lib/ash/query/query.ex +++ b/lib/ash/query/query.ex @@ -834,7 +834,14 @@ defmodule Ash.Query do ``` """ - @spec load(t() | Ash.Resource.t(), atom | list(atom) | Keyword.t()) :: t() + @spec load( + t() | Ash.Resource.t(), + atom + | Ash.Query.Calculation.t() + | list(atom | Ash.Query.Calculation.t()) + | list({atom | Ash.Query.Calculation.t(), term}) + ) :: + t() def load(query, fields) when not is_list(fields) do load(query, List.wrap(fields)) end @@ -905,6 +912,33 @@ defmodule Ash.Query do defp do_load(query, field) do cond do + match?(%Ash.Query.Calculation{}, field) -> + calculation = field + + fields_to_select = + calculation.module.select(query, calculation.opts, calculation.context) + |> Kernel.||([]) + |> Enum.uniq() + |> Enum.filter(&Ash.Resource.Info.attribute(query.resource, &1)) + + loads = + calculation.module.load( + query, + calculation.opts, + Map.put(calculation.context, :context, query.context) + ) + |> Ash.Actions.Helpers.validate_calculation_load!(calculation.module) + |> Enum.reject(&Ash.Resource.Info.attribute(query.resource, &1)) + + calculation = %{ + calculation + | load: field, + required_loads: loads, + select: fields_to_select + } + + Map.update!(query, :calculations, &Map.put(&1, calculation.name, calculation)) + Ash.Resource.Info.attribute(query.resource, field) -> Ash.Query.ensure_selected(query, field) diff --git a/lib/ash/sort/sort.ex b/lib/ash/sort/sort.ex index 3330fb33..37cf3334 100644 --- a/lib/ash/sort/sort.ex +++ b/lib/ash/sort/sort.ex @@ -231,5 +231,5 @@ defmodule Ash.Sort do collation that affects their sorting, making it unpredictable from the perspective of a tool using the database: https://www.postgresql.org/docs/current/collation.html """ - defdelegate runtime_sort(results, sort), to: Ash.Actions.Sort + defdelegate runtime_sort(results, sort, api \\ nil), to: Ash.Actions.Sort end diff --git a/test/calculation_test.exs b/test/calculation_test.exs index 407c7ce4..a0ec300a 100644 --- a/test/calculation_test.exs +++ b/test/calculation_test.exs @@ -2,6 +2,8 @@ defmodule Ash.Test.CalculationTest do @moduledoc false use ExUnit.Case, async: true + require Ash.Query + defmodule Concat do # An example concatenation calculation, that accepts the delimiter as an argument use Ash.Calculation @@ -66,6 +68,32 @@ defmodule Ash.Test.CalculationTest do end end + defmodule NamesOfBestFriendsOfMe do + use Ash.Calculation + + def load(_query, _opts, args) do + if args[:only_special] do + query = + __MODULE__.User + |> Ash.Query.filter(special == true) + |> Ash.Query.ensure_selected(:full_name) + + [best_friends_of_me: query] + else + [best_friends_of_me: :full_name] + end + end + + def calculate(records, _opts, _) do + Enum.map(records, fn record -> + record.best_friends_of_me + |> Enum.map(& &1.full_name) + |> Enum.sort() + |> Enum.join(" - ") + end) + end + end + defmodule BestFriendsFirstNamePlusStuff do use Ash.Calculation @@ -96,6 +124,7 @@ defmodule Ash.Test.CalculationTest do uuid_primary_key :id attribute :first_name, :string attribute :last_name, :string + attribute :special, :boolean end calculations do @@ -130,6 +159,8 @@ defmodule Ash.Test.CalculationTest do calculate :best_friends_name, :string, BestFriendsName + calculate :names_of_best_friends_of_me, :string, NamesOfBestFriendsOfMe + calculate :conditional_full_name, :string, expr( @@ -172,6 +203,10 @@ defmodule Ash.Test.CalculationTest do relationships do belongs_to :best_friend, __MODULE__ + + has_many :best_friends_of_me, __MODULE__ do + destination_attribute :best_friend_id + end end end @@ -428,4 +463,35 @@ defmodule Ash.Test.CalculationTest do assert full_names == ["bob", "brian cranston", "zach daniel"] end + + # test "loading calculations with different relationship dependencies won't collide", %{ + # user1: %{id: user1_id} = user1 + # } do + # user3 = + # User + # |> Ash.Changeset.new(%{first_name: "chidi", last_name: "anagonye", special: true}) + # |> Ash.Changeset.manage_relationship(:best_friend, user1, type: :append_and_remove) + # |> Api.create!() + + # assert %{ + # calculations: %{ + # names_of_best_friends_of_me: "brian cranston - chidi anagonye", + # names_of_special_best_friends_of_me: "chidi anagonye" + # } + # } = + # User + # |> Ash.Query.filter(id == ^user1_id) + # |> Ash.Query.load_calculation_as( + # :names_of_best_friends_of_me, + # :names_of_special_best_friends_of_me, + # %{ + # special: true + # } + # ) + # |> Ash.Query.load_calculation_as( + # :names_of_best_friends_of_me, + # :names_of_best_friends_of_me + # ) + # |> Api.read_one!() + # end end