From 1c34fb09bf97c391b6988a1c5095705de13cdb8f Mon Sep 17 00:00:00 2001 From: TravelCurry02 Date: Mon, 21 Sep 2026 11:47:47 -0600 Subject: [PATCH 1/5] fix: compile parent(rel.id) or-filters as EXISTS instead of joining both lists --- lib/expr.ex | 903 ++++++++++++++++++++++++++++++---------------------- lib/join.ex | 45 ++- 2 files changed, 563 insertions(+), 385 deletions(-) diff --git a/lib/expr.ex b/lib/expr.ex index c1c19d1..5cf6e3f 100644 --- a/lib/expr.ex +++ b/lib/expr.ex @@ -1823,318 +1823,45 @@ defmodule AshSql.Expr do {Ecto.Query.dynamic(-(^expr)), acc} end + # If this is parent(rel.id) == id, use EXISTS instead of a normal ==. + # Every other == must use the typed operator path (UUIDs, ltree, etc). defp default_dynamic_expr( query, - %mod{ - __predicate__?: _, - left: left, - right: right, - embedded?: pred_embedded?, - operator: operator - }, + %Ash.Query.Operator.Eq{left: left, right: right} = op, bindings, embedded?, acc, type ) do - {[left_type, right_type], type} = - case operator do - :/ -> - {types, result} = determine_types(bindings.sql_behaviour, mod, [left, right], type) - - {types, result} = - {Enum.map(types, fn - {Ash.Type.Float, _} -> {Ash.Type.Decimal, []} - other -> other - end), - case result do - {Ash.Type.Float, _} -> {Ash.Type.Decimal, []} - other -> other - end} - - case result do - {Ash.Type.Decimal, _} -> - {Enum.map(types, fn _ -> {Ash.Type.Decimal, []} end), result} - - _ -> - {types, result} - end - - _ -> - determine_types(bindings.sql_behaviour, mod, [left, right], type) - end - - bindings = - if no_cast_for_native_value?(left, left_type) or - no_cast_for_native_value?(right, right_type) do - Map.put(bindings, :skip_cast_for_ref?, true) - else - bindings - end - - {left_expr, acc} = - if left_type do - maybe_type_expr( - query, - left, - set_location(bindings, :sub_expr), - pred_embedded? || embedded?, - acc, - left_type - ) - else - do_dynamic_expr( - query, - left, - set_location(bindings, :sub_expr), - pred_embedded? || embedded?, - acc, - left_type - ) - end - - with :in <- operator, - {:ok, item_type} <- extract_multidimensional_array_type(right_type), - {:ok, right} <- extract_list_value(right) do - Enum.reduce(right, {nil, acc}, fn item, {expr, acc} -> - {elem_expr, acc} = - do_dynamic_expr( - query, - item, - set_location(bindings, :sub_expr), - pred_embedded? || embedded?, - acc, - item_type - ) + if bindings[:parent_bindings] do + case parent_rel_id_path(left, right) do + {:ok, path} -> + parent_rel_id_exists(query, path, bindings, acc) - if is_nil(expr) do - {Ecto.Query.dynamic(^left_expr == ^elem_expr), acc} - else - {Ecto.Query.dynamic(^expr or ^left_expr == ^elem_expr), acc} - end - end) - else - _ -> - if operator == :in do - get_path = strip_get_path_type(right) - - if match?(%Ash.Query.Function.GetPath{}, get_path) and get_path_array_type?(right_type) do - context_embedded? = pred_embedded? || embedded? - - {raw_right_expr, acc} = - get_untyped_get_path_expr( - query, - get_path, - bindings, - context_embedded?, - acc - ) - - {Ecto.Query.dynamic(fragment("(?::jsonb \\? ?)", ^raw_right_expr, ^left_expr)), acc} - else - {right_expr, acc} = - evaluate_right( - query, - right, - bindings, - pred_embedded? || embedded?, - acc, - right_type - ) - - {Ecto.Query.dynamic(^left_expr in ^right_expr), acc} - end - else - {right_expr, acc} = - evaluate_right( - query, - right, - bindings, - pred_embedded? || embedded?, - acc, - right_type - ) - - case operator do - :== -> - {Ecto.Query.dynamic(^left_expr == ^right_expr), acc} - - :!= -> - {Ecto.Query.dynamic(^left_expr != ^right_expr), acc} - - :> -> - {Ecto.Query.dynamic(^left_expr > ^right_expr), acc} - - :< -> - {Ecto.Query.dynamic(^left_expr < ^right_expr), acc} - - :>= -> - {Ecto.Query.dynamic(^left_expr >= ^right_expr), acc} - - :<= -> - {Ecto.Query.dynamic(^left_expr <= ^right_expr), acc} - - :+ -> - {Ecto.Query.dynamic(^left_expr + ^right_expr), acc} - - :- -> - {Ecto.Query.dynamic(^left_expr - ^right_expr), acc} - - :/ -> - {Ecto.Query.dynamic(^left_expr / ^right_expr), acc} - - :* -> - {Ecto.Query.dynamic(^left_expr * ^right_expr), acc} - - :<> -> - do_dynamic_expr( - query, - %Fragment{ - embedded?: pred_embedded?, - arguments: [ - raw: "(", - casted_expr: left_expr, - raw: " || ", - casted_expr: right_expr, - raw: ")" - ] - }, - bindings, - embedded?, - acc, - type - ) - - :|| -> - cond do - boolean_type?(left_type) and boolean_type?(right_type) and - cant_return_nil?(left) -> - {Ecto.Query.dynamic(^left_expr or ^right_expr), acc} - - boolean_type?(left_type) and boolean_type?(right_type) -> - {Ecto.Query.dynamic(coalesce(^left_expr or ^right_expr, false)), acc} - - cannot_be_boolean?(left_type) -> - {Ecto.Query.dynamic(coalesce(^left_expr, ^right_expr)), acc} - - true -> - if "ash-functions" in query.__ash_bindings__.sql_behaviour.repo( - query.__ash_bindings__.resource, - :mutate - ).installed_extensions() do - do_dynamic_expr( - query, - %Fragment{ - embedded?: pred_embedded?, - arguments: [ - raw: "ash_elixir_or(", - casted_expr: left_expr, - raw: ", ", - casted_expr: right_expr, - raw: ")" - ] - }, - bindings, - embedded?, - acc, - type - ) - else - if query.__ash_bindings__.sql_behaviour.require_ash_functions_for_or_and_and?() do - require_ash_functions!(query, "||") - end - - do_dynamic_expr( - query, - %Ash.Query.Function.Fragment{ - embedded?: pred_embedded?, - arguments: [ - raw: "(CASE WHEN (", - casted_expr: left_expr, - raw: " = FALSE OR ", - casted_expr: left_expr, - raw: " IS NULL) THEN ", - casted_expr: right_expr, - raw: " ELSE ", - casted_expr: left_expr, - raw: "END)" - ] - }, - bindings, - embedded?, - acc, - type - ) - end - end - - :&& -> - cond do - boolean_type?(left_type) and boolean_type?(right_type) and - cant_return_nil?(left) -> - {Ecto.Query.dynamic(^left_expr and ^right_expr), acc} - - boolean_type?(left_type) and boolean_type?(right_type) -> - {Ecto.Query.dynamic(coalesce(^left_expr and ^right_expr, false)), acc} - - true -> - if "ash-functions" in query.__ash_bindings__.sql_behaviour.repo( - query.__ash_bindings__.resource, - :mutate - ).installed_extensions() do - do_dynamic_expr( - query, - %Fragment{ - embedded?: pred_embedded?, - arguments: [ - raw: "ash_elixir_and(", - casted_expr: left_expr, - raw: ", ", - casted_expr: right_expr, - raw: ")" - ] - }, - bindings, - embedded?, - acc, - type - ) - else - if query.__ash_bindings__.sql_behaviour.require_ash_functions_for_or_and_and?() do - require_ash_functions!(query, "&&") - end - - do_dynamic_expr( - query, - %Fragment{ - embedded?: pred_embedded?, - arguments: [ - raw: "(CASE WHEN (", - casted_expr: left_expr, - raw: " = FALSE OR ", - casted_expr: left_expr, - raw: " IS NULL) THEN ", - casted_expr: left_expr, - raw: " ELSE ", - casted_expr: right_expr, - raw: "END)" - ] - }, - bindings, - embedded?, - acc, - type - ) - end - end - - other -> - raise "Operator not implemented #{other}" - end - end + _ -> + typed_binary_operator_expr(query, op, bindings, embedded?, acc, type) + end + else + typed_binary_operator_expr(query, op, bindings, embedded?, acc, type) end end + defp default_dynamic_expr( + query, + %_mod{ + __predicate__?: _, + left: _, + right: _, + operator: _ + } = op, + bindings, + embedded?, + acc, + type + ) do + typed_binary_operator_expr(query, op, bindings, embedded?, acc, type) + end + defp default_dynamic_expr(query, %MapSet{} = mapset, bindings, embedded?, acc, type) do do_dynamic_expr(query, Enum.to_list(mapset), bindings, embedded?, acc, type) end @@ -2419,7 +2146,7 @@ defmodule AshSql.Expr do new_field_name = query.__ash_bindings__.aggregate_names[field_name] - unless new_field_name do + if !new_field_name do raise "Unbound aggregate field: #{inspect(field_name)}" end @@ -2761,7 +2488,7 @@ defmodule AshSql.Expr do acc = %{acc | has_error?: true} - unless Keyword.keyword?(input) || is_map(input) do + if !(Keyword.keyword?(input) || is_map(input)) do raise "Input expression to `error` must be a map or keyword list" end @@ -2909,7 +2636,7 @@ defmodule AshSql.Expr do first_relationship = Ash.Resource.Info.relationship(resource, first) - unless first_relationship do + if !first_relationship do raise Ash.Error.Framework.AssumptionFailed, message: """ Unknown relationship #{inspect(bindings.resource)}.#{first} @@ -3301,98 +3028,410 @@ defmodule AshSql.Expr do {:ok, expr, acc} -> {expr, acc} - {:error, error} -> - raise "Error while building expression: #{error}" + {:error, error} -> + raise "Error while building expression: #{error}" + + :error -> + raise "Unsupported expression in #{bindings.sql_behaviour} query: #{inspect(other, structs: false)}" + end + end + else + handle_literal(query, other, bindings, true, acc, type) + end + end + end + + defp default_dynamic_expr(query, value, bindings, embedded?, acc, {:in, type}) + when is_list(value) do + list_expr(query, value, bindings, embedded?, acc, {:array, type}) + end + + defp default_dynamic_expr(query, value, bindings, embedded?, acc, type) + when not is_nil(value) and is_atom(value) and not is_boolean(value) do + do_dynamic_expr(query, to_string(value), bindings, embedded?, acc, type) + end + + defp default_dynamic_expr(_query, nil, bindings, _, acc, type) do + if type && type != :any do + param_type = parameterized_type(bindings.sql_behaviour, type, [], :expr) + + if param_type do + {Ecto.Query.dynamic(type(fragment("NULL"), ^param_type)), acc} + else + {Ecto.Query.dynamic(fragment("NULL")), acc} + end + else + {Ecto.Query.dynamic(fragment("NULL")), acc} + end + end + + defp default_dynamic_expr(query, value, bindings, false, acc, type) + when type == nil or type == :any do + if is_list(value) do + list_expr(query, value, bindings, false, acc, type) + else + handle_literal(query, value, bindings, true, acc, type) + end + end + + defp default_dynamic_expr(query, value, bindings, false, acc, type) do + if Ash.Expr.expr?(value) do + if is_list(value) do + list_expr(query, value, bindings, false, acc, type) + else + case bindings.sql_behaviour.expr(query, value, bindings, false, acc, type) do + {:ok, expr, acc} -> + {expr, acc} + + {:error, error} -> + raise "Error while building expression: #{error}" + + :error -> + raise "Unsupported expression in #{bindings.sql_behaviour} query: #{inspect(value, structs: false)}" + end + end + else + if bindings[:no_cast?] do + {value, acc} + else + case handle_literal(query, value, bindings, true, acc, type) do + {^value, acc} -> + if type do + type = + parameterized_type( + bindings.sql_behaviour, + type, + [], + :expr + ) + + if type do + validate_type!(query, type, value) + + {query.__ash_bindings__.sql_behaviour.type_expr(value, type), acc} + else + {value, acc} + end + else + {value, acc} + end + + {value, acc} -> + {value, acc} + end + end + end + end + + defp typed_binary_operator_expr( + query, + %mod{ + __predicate__?: _, + left: left, + right: right, + embedded?: pred_embedded?, + operator: operator + }, + bindings, + embedded?, + acc, + type + ) do + {[left_type, right_type], type} = + case operator do + :/ -> + {types, result} = determine_types(bindings.sql_behaviour, mod, [left, right], type) + + {types, result} = + {Enum.map(types, fn + {Ash.Type.Float, _} -> {Ash.Type.Decimal, []} + other -> other + end), + case result do + {Ash.Type.Float, _} -> {Ash.Type.Decimal, []} + other -> other + end} + + case result do + {Ash.Type.Decimal, _} -> + {Enum.map(types, fn _ -> {Ash.Type.Decimal, []} end), result} + + _ -> + {types, result} + end + + _ -> + determine_types(bindings.sql_behaviour, mod, [left, right], type) + end + + bindings = + if no_cast_for_native_value?(left, left_type) or + no_cast_for_native_value?(right, right_type) do + Map.put(bindings, :skip_cast_for_ref?, true) + else + bindings + end + + {left_expr, acc} = + if left_type do + maybe_type_expr( + query, + left, + set_location(bindings, :sub_expr), + pred_embedded? || embedded?, + acc, + left_type + ) + else + do_dynamic_expr( + query, + left, + set_location(bindings, :sub_expr), + pred_embedded? || embedded?, + acc, + left_type + ) + end + + with :in <- operator, + {:ok, item_type} <- extract_multidimensional_array_type(right_type), + {:ok, right} <- extract_list_value(right) do + Enum.reduce(right, {nil, acc}, fn item, {expr, acc} -> + {elem_expr, acc} = + do_dynamic_expr( + query, + item, + set_location(bindings, :sub_expr), + pred_embedded? || embedded?, + acc, + item_type + ) + + if is_nil(expr) do + {Ecto.Query.dynamic(^left_expr == ^elem_expr), acc} + else + {Ecto.Query.dynamic(^expr or ^left_expr == ^elem_expr), acc} + end + end) + else + _ -> + if operator == :in do + get_path = strip_get_path_type(right) + + if match?(%Ash.Query.Function.GetPath{}, get_path) and get_path_array_type?(right_type) do + context_embedded? = pred_embedded? || embedded? + + {raw_right_expr, acc} = + get_untyped_get_path_expr( + query, + get_path, + bindings, + context_embedded?, + acc + ) + + {Ecto.Query.dynamic(fragment("(?::jsonb \\? ?)", ^raw_right_expr, ^left_expr)), acc} + else + {right_expr, acc} = + evaluate_right( + query, + right, + bindings, + pred_embedded? || embedded?, + acc, + right_type + ) + + {Ecto.Query.dynamic(^left_expr in ^right_expr), acc} + end + else + {right_expr, acc} = + evaluate_right( + query, + right, + bindings, + pred_embedded? || embedded?, + acc, + right_type + ) + + case operator do + :== -> + {Ecto.Query.dynamic(^left_expr == ^right_expr), acc} + + :!= -> + {Ecto.Query.dynamic(^left_expr != ^right_expr), acc} + + :> -> + {Ecto.Query.dynamic(^left_expr > ^right_expr), acc} + + :< -> + {Ecto.Query.dynamic(^left_expr < ^right_expr), acc} + + :>= -> + {Ecto.Query.dynamic(^left_expr >= ^right_expr), acc} + + :<= -> + {Ecto.Query.dynamic(^left_expr <= ^right_expr), acc} + + :+ -> + {Ecto.Query.dynamic(^left_expr + ^right_expr), acc} - :error -> - raise "Unsupported expression in #{bindings.sql_behaviour} query: #{inspect(other, structs: false)}" - end - end - else - handle_literal(query, other, bindings, true, acc, type) - end - end - end + :- -> + {Ecto.Query.dynamic(^left_expr - ^right_expr), acc} - defp default_dynamic_expr(query, value, bindings, embedded?, acc, {:in, type}) - when is_list(value) do - list_expr(query, value, bindings, embedded?, acc, {:array, type}) - end + :/ -> + {Ecto.Query.dynamic(^left_expr / ^right_expr), acc} - defp default_dynamic_expr(query, value, bindings, embedded?, acc, type) - when not is_nil(value) and is_atom(value) and not is_boolean(value) do - do_dynamic_expr(query, to_string(value), bindings, embedded?, acc, type) - end + :* -> + {Ecto.Query.dynamic(^left_expr * ^right_expr), acc} - defp default_dynamic_expr(_query, nil, bindings, _, acc, type) do - if type && type != :any do - param_type = parameterized_type(bindings.sql_behaviour, type, [], :expr) + :<> -> + do_dynamic_expr( + query, + %Fragment{ + embedded?: pred_embedded?, + arguments: [ + raw: "(", + casted_expr: left_expr, + raw: " || ", + casted_expr: right_expr, + raw: ")" + ] + }, + bindings, + embedded?, + acc, + type + ) - if param_type do - {Ecto.Query.dynamic(type(fragment("NULL"), ^param_type)), acc} - else - {Ecto.Query.dynamic(fragment("NULL")), acc} - end - else - {Ecto.Query.dynamic(fragment("NULL")), acc} - end - end + :|| -> + cond do + boolean_type?(left_type) and boolean_type?(right_type) and + cant_return_nil?(left) -> + {Ecto.Query.dynamic(^left_expr or ^right_expr), acc} - defp default_dynamic_expr(query, value, bindings, false, acc, type) - when type == nil or type == :any do - if is_list(value) do - list_expr(query, value, bindings, false, acc, type) - else - handle_literal(query, value, bindings, true, acc, type) - end - end + boolean_type?(left_type) and boolean_type?(right_type) -> + {Ecto.Query.dynamic(coalesce(^left_expr or ^right_expr, false)), acc} - defp default_dynamic_expr(query, value, bindings, false, acc, type) do - if Ash.Expr.expr?(value) do - if is_list(value) do - list_expr(query, value, bindings, false, acc, type) - else - case bindings.sql_behaviour.expr(query, value, bindings, false, acc, type) do - {:ok, expr, acc} -> - {expr, acc} + cannot_be_boolean?(left_type) -> + {Ecto.Query.dynamic(coalesce(^left_expr, ^right_expr)), acc} - {:error, error} -> - raise "Error while building expression: #{error}" + true -> + if "ash-functions" in query.__ash_bindings__.sql_behaviour.repo( + query.__ash_bindings__.resource, + :mutate + ).installed_extensions() do + do_dynamic_expr( + query, + %Fragment{ + embedded?: pred_embedded?, + arguments: [ + raw: "ash_elixir_or(", + casted_expr: left_expr, + raw: ", ", + casted_expr: right_expr, + raw: ")" + ] + }, + bindings, + embedded?, + acc, + type + ) + else + if query.__ash_bindings__.sql_behaviour.require_ash_functions_for_or_and_and?() do + require_ash_functions!(query, "||") + end - :error -> - raise "Unsupported expression in #{bindings.sql_behaviour} query: #{inspect(value, structs: false)}" - end - end - else - if bindings[:no_cast?] do - {value, acc} - else - case handle_literal(query, value, bindings, true, acc, type) do - {^value, acc} -> - if type do - type = - parameterized_type( - bindings.sql_behaviour, - type, - [], - :expr - ) + do_dynamic_expr( + query, + %Ash.Query.Function.Fragment{ + embedded?: pred_embedded?, + arguments: [ + raw: "(CASE WHEN (", + casted_expr: left_expr, + raw: " = FALSE OR ", + casted_expr: left_expr, + raw: " IS NULL) THEN ", + casted_expr: right_expr, + raw: " ELSE ", + casted_expr: left_expr, + raw: "END)" + ] + }, + bindings, + embedded?, + acc, + type + ) + end + end - if type do - validate_type!(query, type, value) + :&& -> + cond do + boolean_type?(left_type) and boolean_type?(right_type) and + cant_return_nil?(left) -> + {Ecto.Query.dynamic(^left_expr and ^right_expr), acc} - {query.__ash_bindings__.sql_behaviour.type_expr(value, type), acc} - else - {value, acc} + boolean_type?(left_type) and boolean_type?(right_type) -> + {Ecto.Query.dynamic(coalesce(^left_expr and ^right_expr, false)), acc} + + true -> + if "ash-functions" in query.__ash_bindings__.sql_behaviour.repo( + query.__ash_bindings__.resource, + :mutate + ).installed_extensions() do + do_dynamic_expr( + query, + %Fragment{ + embedded?: pred_embedded?, + arguments: [ + raw: "ash_elixir_and(", + casted_expr: left_expr, + raw: ", ", + casted_expr: right_expr, + raw: ")" + ] + }, + bindings, + embedded?, + acc, + type + ) + else + if query.__ash_bindings__.sql_behaviour.require_ash_functions_for_or_and_and?() do + require_ash_functions!(query, "&&") + end + + do_dynamic_expr( + query, + %Fragment{ + embedded?: pred_embedded?, + arguments: [ + raw: "(CASE WHEN (", + casted_expr: left_expr, + raw: " = FALSE OR ", + casted_expr: left_expr, + raw: " IS NULL) THEN ", + casted_expr: left_expr, + raw: " ELSE ", + casted_expr: right_expr, + raw: "END)" + ] + }, + bindings, + embedded?, + acc, + type + ) + end end - else - {value, acc} - end - {value, acc} -> - {value, acc} + other -> + raise "Operator not implemented #{other}" + end end - end end end @@ -4044,7 +4083,7 @@ defmodule AshSql.Expr do acc ) when is_atom(field) do - unless to_string(field) =~ ~r/^[a-zA-Z_][a-zA-Z0-9_]*$/ do + if !(to_string(field) =~ ~r/^[a-zA-Z_][a-zA-Z0-9_]*$/) do raise Ash.Error.Query.InvalidExpression, expression: field, message: "#{inspect(field)} is not a valid composite type field name" @@ -4090,7 +4129,7 @@ defmodule AshSql.Expr do installed_extensions = query.__ash_bindings__.sql_behaviour.repo(query.__ash_bindings__.resource, :mutate).installed_extensions() - unless "ash-functions" in installed_extensions do + if "ash-functions" not in installed_extensions do raise """ Cannot use `#{operator}` without adding the extension `ash-functions` to your repo. @@ -4401,4 +4440,100 @@ defmodule AshSql.Expr do defp get_path_array_type?({type, _}) when is_tuple(type), do: get_path_array_type?(type) defp get_path_array_type?(_), do: false + + defp parent_rel_id_path(left, right) do + cond do + parent_rel_ref?(unwrap_parent(left)) and local_id_ref?(right) -> + {:ok, unwrap_parent(left).relationship_path} + + parent_rel_ref?(unwrap_parent(right)) and local_id_ref?(left) -> + {:ok, unwrap_parent(right).relationship_path} + + true -> + :error + end + end + + defp unwrap_parent(%Ash.Query.Parent{expr: expr}), do: expr + defp unwrap_parent(_), do: nil + + defp parent_rel_ref?(%Ref{relationship_path: path} = ref) + when is_list(path) and path != [] do + expr_ref_name(ref) == :id + end + + defp parent_rel_ref?(_), do: false + + defp local_id_ref?(%Ref{relationship_path: []} = ref) do + expr_ref_name(ref) == :id + end + + defp local_id_ref?(_), do: false + + defp expr_ref_name(%Ref{attribute: %{name: name}}), do: name + defp expr_ref_name(%Ref{attribute: name}) when is_atom(name), do: name + defp expr_ref_name(_), do: nil + + # Check whether the parent has a related row with this id. + defp parent_rel_id_exists(query, [first | _rest], bindings, acc) do + parent_resource = bindings.parent_bindings.resource + relationship = Ash.Resource.Info.relationship(parent_resource, first) + dest_attr = relationship.destination_attribute + source_attr = relationship.source_attribute + + parent_binding = + ref_binding( + %Ref{ + attribute: Ash.Resource.Info.attribute(parent_resource, source_attr), + relationship_path: [], + resource: parent_resource + }, + bindings.parent_bindings + ) || bindings.parent_bindings.root_binding + + local_binding = + ref_binding( + %Ref{ + attribute: Ash.Resource.Info.attribute(query.__ash_bindings__.resource, dest_attr), + relationship_path: [], + resource: query.__ash_bindings__.resource + }, + query.__ash_bindings__ + ) || query.__ash_bindings__.root_binding + + parent_query = %{ + query + | __ash_bindings__: Map.put(bindings.parent_bindings, :parent?, true) + } + + {:ok, subquery} = + if relationship.type == :many_to_many do + through_rel = + Ash.Resource.Info.relationship(parent_resource, relationship.join_relationship) + + {:ok, through} = AshSql.Join.related_subquery(through_rel, parent_query) + + {:ok, + Ecto.Query.from(t in Ecto.Query.subquery(through), + where: + field(t, ^relationship.source_attribute_on_join_resource) == + field(parent_as(^parent_binding), ^source_attr), + where: + field(t, ^relationship.destination_attribute_on_join_resource) == + field(parent_as(^local_binding), ^dest_attr) + )} + else + AshSql.Join.related_subquery(relationship, parent_query, + return_subquery?: true, + filter_subquery?: true, + on_subquery: fn subquery -> + Ecto.Query.from(d in subquery, + where: field(d, ^dest_attr) == field(parent_as(^local_binding), ^dest_attr) + ) + end + ) + end + + {Ecto.Query.dynamic(exists(subquery)), acc} + end end diff --git a/lib/join.ex b/lib/join.ex index d8a1893..757847c 100644 --- a/lib/join.ex +++ b/lib/join.ex @@ -257,7 +257,15 @@ defmodule AshSql.Join do {:ok, query} lateral_join_source_query -> - case join_all_relationships(lateral_join_source_query, parent_expr(filter)) do + # Don't join both parent lists when the filter is "this id or that id". + parent_filter = + if parent_rel_id_or?(filter) do + true + else + parent_expr(filter) + end + + case join_all_relationships(lateral_join_source_query, parent_filter) do {:ok, lateral_join_source_query} -> {:ok, put_in(query.__ash_bindings__.lateral_join_source_query, lateral_join_source_query) @@ -279,6 +287,41 @@ defmodule AshSql.Join do {:ok, query} end + # True when the filter is parent(rel.id) == id or-ed with another like it. + defp parent_rel_id_or?(filter) do + branches = AshSql.Expr.split_statements(filter, :or) + length(branches) >= 2 and Enum.all?(branches, &parent_rel_id_eq_local_id?/1) + end + + defp parent_rel_id_eq_local_id?(%Ash.Query.Operator.Eq{left: left, right: right}) do + parent_rel_id_eq_pair?(left, right) or parent_rel_id_eq_pair?(right, left) + end + + defp parent_rel_id_eq_local_id?(_), do: false + + defp parent_rel_id_eq_pair?(%Ash.Query.Parent{expr: parent_ref}, local_ref) do + parent_rel_ref?(parent_ref) and local_id_ref?(local_ref) + end + + defp parent_rel_id_eq_pair?(_, _), do: false + + defp parent_rel_ref?(%Ash.Query.Ref{relationship_path: path} = ref) + when is_list(path) and path != [] do + ref_name(ref) == :id + end + + defp parent_rel_ref?(_), do: false + + defp local_id_ref?(%Ash.Query.Ref{relationship_path: []} = ref) do + ref_name(ref) == :id + end + + defp local_id_ref?(_), do: false + + defp ref_name(%Ash.Query.Ref{attribute: %{name: name}}), do: name + defp ref_name(%Ash.Query.Ref{attribute: name}) when is_atom(name), do: name + defp ref_name(_), do: nil + defp to_joins(paths, filter, resource) do paths |> Enum.reject(&(&1 == [])) From 29db7a3431de2c1d1c3e6929871449f9498be6b2 Mon Sep 17 00:00:00 2001 From: TravelCurry02 Date: Wed, 30 Sep 2026 12:29:30 -0600 Subject: [PATCH 2/5] fix: rewrite cartesian parent filters to EXISTS instead of matching on = --- lib/expr.ex | 555 +++++++++++++++++++++++++------------------------- lib/filter.ex | 11 +- lib/join.ex | 139 +++++++++++-- 3 files changed, 403 insertions(+), 302 deletions(-) diff --git a/lib/expr.ex b/lib/expr.ex index 5cf6e3f..4dc9382 100644 --- a/lib/expr.ex +++ b/lib/expr.ex @@ -1823,32 +1823,9 @@ defmodule AshSql.Expr do {Ecto.Query.dynamic(-(^expr)), acc} end - # If this is parent(rel.id) == id, use EXISTS instead of a normal ==. - # Every other == must use the typed operator path (UUIDs, ltree, etc). defp default_dynamic_expr( query, - %Ash.Query.Operator.Eq{left: left, right: right} = op, - bindings, - embedded?, - acc, - type - ) do - if bindings[:parent_bindings] do - case parent_rel_id_path(left, right) do - {:ok, path} -> - parent_rel_id_exists(query, path, bindings, acc) - - _ -> - typed_binary_operator_expr(query, op, bindings, embedded?, acc, type) - end - else - typed_binary_operator_expr(query, op, bindings, embedded?, acc, type) - end - end - - defp default_dynamic_expr( - query, - %_mod{ + %_{ __predicate__?: _, left: _, right: _, @@ -2627,216 +2604,220 @@ defmodule AshSql.Expr do query, %Exists{at_path: at_path, path: [first | rest], expr: expr} = exists, bindings, - _embedded?, + embedded?, acc, _type ) do - full_at_path = List.wrap(bindings[:refs_at_path]) ++ at_path - resource = Ash.Resource.Info.related(bindings.resource, full_at_path) - - first_relationship = Ash.Resource.Info.relationship(resource, first) + if Map.get(exists, :from_parent) do + parent_exists_expr(query, exists, bindings, embedded?, acc) + else + full_at_path = List.wrap(bindings[:refs_at_path]) ++ at_path + resource = Ash.Resource.Info.related(bindings.resource, full_at_path) - if !first_relationship do - raise Ash.Error.Framework.AssumptionFailed, - message: """ - Unknown relationship #{inspect(bindings.resource)}.#{first} + first_relationship = Ash.Resource.Info.relationship(resource, first) - in exists expression: `#{inspect(exists)}` - """ - end + if !first_relationship do + raise Ash.Error.Framework.AssumptionFailed, + message: """ + Unknown relationship #{inspect(bindings.resource)}.#{first} - filter = - case Ash.Filter.move_to_relationship_path(expr, rest) do - %Ash.Filter{expression: expression} -> expression - expression -> expression + in exists expression: `#{inspect(exists)}` + """ end - filter = - exists - |> Map.get(:__join_filters__, %{}) - |> Map.fetch([first_relationship.name]) - |> case do - {:ok, join_filter} -> - Ash.Query.BooleanExpression.optimized_new( - :and, - filter, - Ash.Filter.move_to_relationship_path( - join_filter, - rest ++ [first_relationship.name] - ) - ) - - :error -> - filter - end + filter = + case Ash.Filter.move_to_relationship_path(expr, rest) do + %Ash.Filter{expression: expression} -> expression + expression -> expression + end - filter = - exists - |> Map.get(:__join_filters__, %{}) - |> Map.delete([first_relationship.name]) - |> Enum.reduce(filter, fn {path, path_filter}, filter -> - path = Enum.drop(path, 1) - parent_path = :lists.droplast(path) - - Ash.Query.BooleanExpression.optimized_new( - :and, - filter, - Ash.Filter.move_to_relationship_path(path_filter, path) - ) - |> Ash.Filter.map(fn - %Ash.Query.Parent{expr: expr} -> - {:halt, Ash.Filter.move_to_relationship_path(expr, parent_path)} + filter = + exists + |> Map.get(:__join_filters__, %{}) + |> Map.fetch([first_relationship.name]) + |> case do + {:ok, join_filter} -> + Ash.Query.BooleanExpression.optimized_new( + :and, + filter, + Ash.Filter.move_to_relationship_path( + join_filter, + rest ++ [first_relationship.name] + ) + ) - other -> - other - end) - end) + :error -> + filter + end - # Joins for `rest` are derived from the refs in the filter and are left - # joins, so a predicate with no refs (e.g. `exists(a.bs, true)`) drops the - # remaining path entirely, and a null-satisfiable predicate is satisfied - # by null-extended rows. Requiring a non-nil primary key at every hop - # (not just the last: `no_attributes?` hops join with `on: true`) - # excludes both while being a no-op for real rows. - filter = - rest - |> Enum.scan([], fn rel_name, prefix -> prefix ++ [rel_name] end) - |> Enum.reduce(filter, fn prefix, filter -> - with target when not is_nil(target) <- - Ash.Resource.Info.related(first_relationship.destination, prefix), - [pk | _] <- Ash.Resource.Info.primary_key(target) do - pk_ref = %Ref{ - attribute: Ash.Resource.Info.attribute(target, pk), - relationship_path: prefix, - resource: target - } + filter = + exists + |> Map.get(:__join_filters__, %{}) + |> Map.delete([first_relationship.name]) + |> Enum.reduce(filter, fn {path, path_filter}, filter -> + path = Enum.drop(path, 1) + parent_path = :lists.droplast(path) Ash.Query.BooleanExpression.optimized_new( :and, filter, - %Ash.Query.Operator.IsNil{left: pk_ref, right: false} + Ash.Filter.move_to_relationship_path(path_filter, path) ) - else - _ -> filter - end - end) + |> Ash.Filter.map(fn + %Ash.Query.Parent{expr: expr} -> + {:halt, Ash.Filter.move_to_relationship_path(expr, parent_path)} - query = - if first_relationship.type == :many_to_many do - put_in(query.__ash_bindings__[:lateral_join_bindings], [:join_source]) - |> AshSql.Bindings.explicitly_set_binding( - %{ - type: :left, - path: [first_relationship.join_relationship] - }, - :join_source - ) - else - query - end + other -> + other + end) + end) - {:ok, subquery} = - AshSql.Join.related_subquery(first_relationship, query, - filter: filter, - filter_subquery?: true, - sort?: Map.get(first_relationship, :from_many?) || not is_nil(first_relationship.sort), - start_bindings_at: 1, - select_star?: !Map.get(first_relationship, :manual), - in_group?: true, - refs_at_path: full_at_path, - parent_resources: [ - Ash.Resource.Info.related(resource, at_path) - | query.__ash_bindings__[:parent_resources] || [] - ], - return_subquery?: true, - on_subquery: fn subquery -> - subquery = - Ecto.Query.from(row in subquery, select: row) - |> Map.put(:__ash_bindings__, subquery.__ash_bindings__) + # Joins for `rest` are derived from the refs in the filter and are left + # joins, so a predicate with no refs (e.g. `exists(a.bs, true)`) drops the + # remaining path entirely, and a null-satisfiable predicate is satisfied + # by null-extended rows. Requiring a non-nil primary key at every hop + # (not just the last: `no_attributes?` hops join with `on: true`) + # excludes both while being a no-op for real rows. + filter = + rest + |> Enum.scan([], fn rel_name, prefix -> prefix ++ [rel_name] end) + |> Enum.reduce(filter, fn prefix, filter -> + with target when not is_nil(target) <- + Ash.Resource.Info.related(first_relationship.destination, prefix), + [pk | _] <- Ash.Resource.Info.primary_key(target) do + pk_ref = %Ref{ + attribute: Ash.Resource.Info.attribute(target, pk), + relationship_path: prefix, + resource: target + } - cond do - Map.get(first_relationship, :manual) -> - {module, opts} = first_relationship.manual - - source_binding = - ref_binding( - %Ref{ - attribute: - Ash.Resource.Info.attribute(resource, first_relationship.source_attribute), - relationship_path: at_path, - resource: resource - }, - bindings - ) + Ash.Query.BooleanExpression.optimized_new( + :and, + filter, + %Ash.Query.Operator.IsNil{left: pk_ref, right: false} + ) + else + _ -> filter + end + end) - {:ok, subquery} = - apply( - module, - query.__ash_bindings__.sql_behaviour.manual_relationship_subquery_function(), - [ - opts, - source_binding, - 1, - subquery - ] - ) + query = + if first_relationship.type == :many_to_many do + put_in(query.__ash_bindings__[:lateral_join_bindings], [:join_source]) + |> AshSql.Bindings.explicitly_set_binding( + %{ + type: :left, + path: [first_relationship.join_relationship] + }, + :join_source + ) + else + query + end - subquery - - Map.get(first_relationship, :no_attributes?) -> - subquery - - first_relationship.type == :many_to_many -> - source_ref = - ref_binding( - %Ref{ - attribute: - Ash.Resource.Info.attribute(resource, first_relationship.source_attribute), - relationship_path: at_path, - resource: resource - }, - bindings + {:ok, subquery} = + AshSql.Join.related_subquery(first_relationship, query, + filter: filter, + filter_subquery?: true, + sort?: Map.get(first_relationship, :from_many?) || not is_nil(first_relationship.sort), + start_bindings_at: 1, + select_star?: !Map.get(first_relationship, :manual), + in_group?: true, + refs_at_path: full_at_path, + parent_resources: [ + Ash.Resource.Info.related(resource, at_path) + | query.__ash_bindings__[:parent_resources] || [] + ], + return_subquery?: true, + on_subquery: fn subquery -> + subquery = + Ecto.Query.from(row in subquery, select: row) + |> Map.put(:__ash_bindings__, subquery.__ash_bindings__) + + cond do + Map.get(first_relationship, :manual) -> + {module, opts} = first_relationship.manual + + source_binding = + ref_binding( + %Ref{ + attribute: + Ash.Resource.Info.attribute(resource, first_relationship.source_attribute), + relationship_path: at_path, + resource: resource + }, + bindings + ) + + {:ok, subquery} = + apply( + module, + query.__ash_bindings__.sql_behaviour.manual_relationship_subquery_function(), + [ + opts, + source_binding, + 1, + subquery + ] + ) + + subquery + + Map.get(first_relationship, :no_attributes?) -> + subquery + + first_relationship.type == :many_to_many -> + source_ref = + ref_binding( + %Ref{ + attribute: + Ash.Resource.Info.attribute(resource, first_relationship.source_attribute), + relationship_path: at_path, + resource: resource + }, + bindings + ) + + through_relationship = + Ash.Resource.Info.relationship(resource, first_relationship.join_relationship) + + {:ok, through} = + AshSql.Join.related_subquery(through_relationship, query) + + Ecto.Query.from(destination in subquery, + join: through in ^through, + as: ^:join_source, + on: + field(through, ^first_relationship.destination_attribute_on_join_resource) == + field(destination, ^first_relationship.destination_attribute), + on: + field(parent_as(^source_ref), ^first_relationship.source_attribute) == + field(through, ^first_relationship.source_attribute_on_join_resource) ) - through_relationship = - Ash.Resource.Info.relationship(resource, first_relationship.join_relationship) - - {:ok, through} = - AshSql.Join.related_subquery(through_relationship, query) - - Ecto.Query.from(destination in subquery, - join: through in ^through, - as: ^:join_source, - on: - field(through, ^first_relationship.destination_attribute_on_join_resource) == - field(destination, ^first_relationship.destination_attribute), - on: - field(parent_as(^source_ref), ^first_relationship.source_attribute) == - field(through, ^first_relationship.source_attribute_on_join_resource) - ) - - true -> - source_ref = - ref_binding( - %Ref{ - attribute: - Ash.Resource.Info.attribute(resource, first_relationship.source_attribute), - relationship_path: at_path, - resource: resource - }, - bindings + true -> + source_ref = + ref_binding( + %Ref{ + attribute: + Ash.Resource.Info.attribute(resource, first_relationship.source_attribute), + relationship_path: at_path, + resource: resource + }, + bindings + ) + + Ecto.Query.from(destination in subquery, + where: + field(parent_as(^source_ref), ^first_relationship.source_attribute) == + field(destination, ^first_relationship.destination_attribute) ) - - Ecto.Query.from(destination in subquery, - where: - field(parent_as(^source_ref), ^first_relationship.source_attribute) == - field(destination, ^first_relationship.destination_attribute) - ) + end end - end - ) + ) - {Ecto.Query.dynamic(exists(subquery)), acc} + {Ecto.Query.dynamic(exists(subquery)), acc} + end end defp default_dynamic_expr( @@ -4441,44 +4422,37 @@ defmodule AshSql.Expr do defp get_path_array_type?(_), do: false - defp parent_rel_id_path(left, right) do - cond do - parent_rel_ref?(unwrap_parent(left)) and local_id_ref?(right) -> - {:ok, unwrap_parent(left).relationship_path} + # EXISTS against a parent to-many path. Inner parent() is the related row, not the parent resource. + defp parent_exists_expr( + query, + %Exists{path: [first | rest], expr: expr}, + bindings, + embedded?, + acc + ) do + parent_resource = bindings.parent_bindings.resource + relationship = Ash.Resource.Info.relationship(parent_resource, first) - parent_rel_ref?(unwrap_parent(right)) and local_id_ref?(left) -> - {:ok, unwrap_parent(right).relationship_path} + if !relationship do + raise Ash.Error.Framework.AssumptionFailed, + message: """ + Unknown relationship #{inspect(parent_resource)}.#{first} - true -> - :error + in parent exists expression + """ end - end - - defp unwrap_parent(%Ash.Query.Parent{expr: expr}), do: expr - defp unwrap_parent(_), do: nil - - defp parent_rel_ref?(%Ref{relationship_path: path} = ref) - when is_list(path) and path != [] do - expr_ref_name(ref) == :id - end - - defp parent_rel_ref?(_), do: false - defp local_id_ref?(%Ref{relationship_path: []} = ref) do - expr_ref_name(ref) == :id - end - - defp local_id_ref?(_), do: false + filter = + case Ash.Filter.move_to_relationship_path(expr, rest) do + %Ash.Filter{expression: expression} -> expression + expression -> expression + end - defp expr_ref_name(%Ref{attribute: %{name: name}}), do: name - defp expr_ref_name(%Ref{attribute: name}) when is_atom(name), do: name - defp expr_ref_name(_), do: nil + parent_query = %{ + query + | __ash_bindings__: Map.put(bindings.parent_bindings, :parent?, true) + } - # Check whether the parent has a related row with this id. - defp parent_rel_id_exists(query, [first | _rest], bindings, acc) do - parent_resource = bindings.parent_bindings.resource - relationship = Ash.Resource.Info.relationship(parent_resource, first) - dest_attr = relationship.destination_attribute source_attr = relationship.source_attribute parent_binding = @@ -4491,48 +4465,71 @@ defmodule AshSql.Expr do bindings.parent_bindings ) || bindings.parent_bindings.root_binding - local_binding = - ref_binding( - %Ref{ - attribute: Ash.Resource.Info.attribute(query.__ash_bindings__.resource, dest_attr), - relationship_path: [], - resource: query.__ash_bindings__.resource - }, - query.__ash_bindings__ - ) || query.__ash_bindings__.root_binding - - parent_query = %{ - query - | __ash_bindings__: Map.put(bindings.parent_bindings, :parent?, true) - } - {:ok, subquery} = - if relationship.type == :many_to_many do - through_rel = - Ash.Resource.Info.relationship(parent_resource, relationship.join_relationship) - - {:ok, through} = AshSql.Join.related_subquery(through_rel, parent_query) - - {:ok, - Ecto.Query.from(t in Ecto.Query.subquery(through), - where: - field(t, ^relationship.source_attribute_on_join_resource) == - field(parent_as(^parent_binding), ^source_attr), - where: - field(t, ^relationship.destination_attribute_on_join_resource) == - field(parent_as(^local_binding), ^dest_attr) - )} - else - AshSql.Join.related_subquery(relationship, parent_query, - return_subquery?: true, - filter_subquery?: true, - on_subquery: fn subquery -> - Ecto.Query.from(d in subquery, - where: field(d, ^dest_attr) == field(parent_as(^local_binding), ^dest_attr) + AshSql.Join.related_subquery(relationship, parent_query, + filter_subquery?: true, + start_bindings_at: 1, + select_star?: true, + in_group?: true, + return_subquery?: true, + on_subquery: fn subquery -> + subquery = + Ecto.Query.from(row in subquery, select: row) + |> Map.put(:__ash_bindings__, subquery.__ash_bindings__) + + inner_bindings = + set_location( + subquery.__ash_bindings__ + |> Map.put(:parent_bindings, Map.put(query.__ash_bindings__, :parent?, true)) + |> Map.put(:parent?, false), + :sub_expr ) + + {pred, _acc} = + if filter in [nil, true] do + {true, acc} + else + do_dynamic_expr( + %{subquery | __ash_bindings__: inner_bindings}, + filter, + inner_bindings, + embedded?, + acc + ) + end + + cond do + Map.get(relationship, :no_attributes?) -> + Ecto.Query.from(destination in subquery, where: ^pred) + + relationship.type == :many_to_many -> + through_relationship = + Ash.Resource.Info.relationship(parent_resource, relationship.join_relationship) + + {:ok, through} = + AshSql.Join.related_subquery(through_relationship, parent_query) + + Ecto.Query.from(destination in subquery, + join: through in ^through, + on: + field(through, ^relationship.destination_attribute_on_join_resource) == + field(destination, ^relationship.destination_attribute), + on: + field(parent_as(^parent_binding), ^source_attr) == + field(through, ^relationship.source_attribute_on_join_resource), + where: ^pred + ) + + true -> + Ecto.Query.from(destination in subquery, + where: + field(parent_as(^parent_binding), ^source_attr) == + field(destination, ^relationship.destination_attribute), + where: ^pred + ) end - ) - end + end + ) {Ecto.Query.dynamic(exists(subquery)), acc} end diff --git a/lib/filter.ex b/lib/filter.ex index 6817f3e..0e7fb4e 100644 --- a/lib/filter.ex +++ b/lib/filter.ex @@ -8,6 +8,7 @@ defmodule AshSql.Filter do require Ecto.Query def filter(query, filter, resource, opts \\ []) do + filter = rewrite_cartesian_parent_filter(query, filter) used_aggregates = Ash.Filter.used_aggregates(filter, []) query @@ -35,7 +36,8 @@ defmodule AshSql.Filter do end def add_filter_expression(query, filter) do - filter + query + |> rewrite_cartesian_parent_filter(filter) |> AshSql.Expr.split_statements(:and) |> Enum.reduce(query, fn filter, query -> {dynamic, acc} = @@ -57,4 +59,11 @@ defmodule AshSql.Filter do |> AshSql.Expr.merge_accumulator(acc) end) end + + defp rewrite_cartesian_parent_filter(query, filter) do + AshSql.Join.rewrite_filter_if_cartesian_parent( + filter, + AshSql.Join.cartesian_parent_resource(query) + ) + end end diff --git a/lib/join.ex b/lib/join.ex index 757847c..ad90df0 100644 --- a/lib/join.ex +++ b/lib/join.ex @@ -257,9 +257,15 @@ defmodule AshSql.Join do {:ok, query} lateral_join_source_query -> - # Don't join both parent lists when the filter is "this id or that id". + # Joining two parent to-many paths cartesian-products rows. Use EXISTS instead. + as_exists? = + cartesian_parent_join?( + filter, + lateral_join_source_query.__ash_bindings__.resource + ) + parent_filter = - if parent_rel_id_or?(filter) do + if as_exists? do true else parent_expr(filter) @@ -287,40 +293,128 @@ defmodule AshSql.Join do {:ok, query} end - # True when the filter is parent(rel.id) == id or-ed with another like it. - defp parent_rel_id_or?(filter) do - branches = AshSql.Expr.split_statements(filter, :or) - length(branches) >= 2 and Enum.all?(branches, &parent_rel_id_eq_local_id?/1) + # True when parent() names two or more to-many relationships (those would be joined). + defp cartesian_parent_join?(filter, parent_resource) do + filter + |> parent_expr() + |> Ash.Filter.relationship_paths() + |> Enum.reject(&(&1 == [])) + |> Enum.uniq() + |> Enum.filter(fn + [first | _] -> + case Ash.Resource.Info.relationship(parent_resource, first) do + %{cardinality: :many} -> true + _ -> false + end + + _ -> + false + end) + |> length() + |> Kernel.>=(2) end - defp parent_rel_id_eq_local_id?(%Ash.Query.Operator.Eq{left: left, right: right}) do - parent_rel_id_eq_pair?(left, right) or parent_rel_id_eq_pair?(right, left) + defp maybe_rewrite_cartesian_parent_query(%{valid?: false} = query, _parent_resource), + do: query + + defp maybe_rewrite_cartesian_parent_query(query, parent_resource) do + %{query | filter: rewrite_filter_if_cartesian_parent(query.filter, parent_resource)} end - defp parent_rel_id_eq_local_id?(_), do: false + @doc false + def rewrite_filter_if_cartesian_parent(filter, parent_resource) + when is_nil(filter) or is_nil(parent_resource) or filter in [true, false] do + filter + end + + def rewrite_filter_if_cartesian_parent(filter, parent_resource) do + if cartesian_parent_join?(filter, parent_resource) do + rewrite_parent_paths_as_exists(filter, parent_resource) + else + filter + end + end - defp parent_rel_id_eq_pair?(%Ash.Query.Parent{expr: parent_ref}, local_ref) do - parent_rel_ref?(parent_ref) and local_id_ref?(local_ref) + @doc false + def cartesian_parent_resource(query) do + case query.__ash_bindings__[:lateral_join_source_query] do + %{__ash_bindings__: %{resource: resource}} -> + resource + + _ -> + case query.__ash_bindings__[:parent_bindings] do + %{resource: resource} -> resource + _ -> nil + end + end end - defp parent_rel_id_eq_pair?(_, _), do: false + # Keep and/or/not. Turn each parent(to_many.*) leaf into exists from the parent. + defp rewrite_parent_paths_as_exists(filter, parent_resource) do + Ash.Filter.map(filter, fn + %Ash.Query.BooleanExpression{} = expr -> + expr + + %Ash.Query.Not{} = expr -> + expr + + %Ash.Query.Exists{} = expr -> + {:halt, expr} + + expr -> + case parent_to_many_first_hops(expr, parent_resource) do + [first] -> + {:halt, exists_from_parent_path(expr, first)} - defp parent_rel_ref?(%Ash.Query.Ref{relationship_path: path} = ref) - when is_list(path) and path != [] do - ref_name(ref) == :id + _ -> + expr + end + end) end - defp parent_rel_ref?(_), do: false + defp parent_to_many_first_hops(expr, parent_resource) do + expr + |> Ash.Filter.flat_map(fn + %Ash.Query.Parent{expr: %Ref{relationship_path: [first | _]}} -> + case Ash.Resource.Info.relationship(parent_resource, first) do + %{cardinality: :many} -> [first] + _ -> [] + end + + %Ash.Query.Parent{expr: inner} -> + parent_to_many_first_hops(inner, parent_resource) - defp local_id_ref?(%Ash.Query.Ref{relationship_path: []} = ref) do - ref_name(ref) == :id + _ -> + [] + end) + |> Enum.uniq() end - defp local_id_ref?(_), do: false + defp exists_from_parent_path(expr, first) do + rewritten = + Ash.Filter.map(expr, fn + %Ash.Query.Parent{expr: %Ref{relationship_path: [^first | rest]} = ref} -> + {:halt, %{ref | relationship_path: rest}} - defp ref_name(%Ash.Query.Ref{attribute: %{name: name}}), do: name - defp ref_name(%Ash.Query.Ref{attribute: name}) when is_atom(name), do: name - defp ref_name(_), do: nil + %Ref{relationship_path: []} = ref -> + {:halt, %Ash.Query.Parent{expr: ref}} + + other -> + other + end) + + inner = + case rewritten do + %Ash.Filter{expression: expression} -> expression + expression -> expression + end + + Map.put( + %Ash.Query.Exists{path: [first], expr: inner, at_path: [], related?: true}, + :from_parent, + true + ) + end defp to_joins(paths, filter, resource) do paths @@ -544,6 +638,7 @@ defmodule AshSql.Join do end end) |> set_has_parent_expr_context(relationship) + |> maybe_rewrite_cartesian_parent_query(relationship.source) |> case do %{valid?: true} = related_query -> parent_bindings = From a47d15c8ccd9c00b63fd484aaa27194be34ea5cb Mon Sep 17 00:00:00 2001 From: TravelCurry02 Date: Mon, 12 Oct 2026 11:17:00 -0600 Subject: [PATCH 3/5] fix: wrap parent to-many filters in a correlated EXISTS over the source --- lib/expr.ex | 512 +++++++++++++++++++++----------------------------- lib/filter.ex | 30 ++- lib/join.ex | 131 +++---------- 3 files changed, 258 insertions(+), 415 deletions(-) diff --git a/lib/expr.ex b/lib/expr.ex index 4dc9382..7beabcc 100644 --- a/lib/expr.ex +++ b/lib/expr.ex @@ -2543,7 +2543,7 @@ defmodule AshSql.Expr do defp default_dynamic_expr( query, - %Exists{related?: false, expr: expr, resource: resource}, + %Exists{related?: false, expr: expr, resource: resource} = exists, bindings, _embedded?, acc, @@ -2577,7 +2577,14 @@ defmodule AshSql.Expr do |> AshSql.Join.hydrate_refs(bindings.context[:private][:actor]) |> then(fn related_query -> if expr != true do - Ash.Query.do_filter(related_query, expr) + parent_stack = + if Map.get(exists, :correlate_source_pk) do + [query.__ash_bindings__.resource] + else + [] + end + + Ash.Query.do_filter(related_query, expr, parent_stack: parent_stack) else related_query end @@ -2586,7 +2593,16 @@ defmodule AshSql.Expr do %{valid?: true} = related_query -> case Ash.Query.data_layer_query(related_query) do {:ok, ecto_query} -> - {:ok, Ecto.Query.exclude(ecto_query, :select)} + ecto_query = Ecto.Query.exclude(ecto_query, :select) + + ecto_query = + if Map.get(exists, :correlate_source_pk) do + correlate_exists_to_source_pk(ecto_query, query, resource) + else + ecto_query + end + + {:ok, ecto_query} {:error, error} -> {:error, error} @@ -2604,220 +2620,216 @@ defmodule AshSql.Expr do query, %Exists{at_path: at_path, path: [first | rest], expr: expr} = exists, bindings, - embedded?, + _embedded?, acc, _type ) do - if Map.get(exists, :from_parent) do - parent_exists_expr(query, exists, bindings, embedded?, acc) - else - full_at_path = List.wrap(bindings[:refs_at_path]) ++ at_path - resource = Ash.Resource.Info.related(bindings.resource, full_at_path) - - first_relationship = Ash.Resource.Info.relationship(resource, first) - - if !first_relationship do - raise Ash.Error.Framework.AssumptionFailed, - message: """ - Unknown relationship #{inspect(bindings.resource)}.#{first} + full_at_path = List.wrap(bindings[:refs_at_path]) ++ at_path + resource = Ash.Resource.Info.related(bindings.resource, full_at_path) - in exists expression: `#{inspect(exists)}` - """ - end + first_relationship = Ash.Resource.Info.relationship(resource, first) - filter = - case Ash.Filter.move_to_relationship_path(expr, rest) do - %Ash.Filter{expression: expression} -> expression - expression -> expression - end - - filter = - exists - |> Map.get(:__join_filters__, %{}) - |> Map.fetch([first_relationship.name]) - |> case do - {:ok, join_filter} -> - Ash.Query.BooleanExpression.optimized_new( - :and, - filter, - Ash.Filter.move_to_relationship_path( - join_filter, - rest ++ [first_relationship.name] - ) - ) + if !first_relationship do + raise Ash.Error.Framework.AssumptionFailed, + message: """ + Unknown relationship #{inspect(bindings.resource)}.#{first} - :error -> - filter - end + in exists expression: `#{inspect(exists)}` + """ + end - filter = - exists - |> Map.get(:__join_filters__, %{}) - |> Map.delete([first_relationship.name]) - |> Enum.reduce(filter, fn {path, path_filter}, filter -> - path = Enum.drop(path, 1) - parent_path = :lists.droplast(path) + filter = + case Ash.Filter.move_to_relationship_path(expr, rest) do + %Ash.Filter{expression: expression} -> expression + expression -> expression + end + filter = + exists + |> Map.get(:__join_filters__, %{}) + |> Map.fetch([first_relationship.name]) + |> case do + {:ok, join_filter} -> Ash.Query.BooleanExpression.optimized_new( :and, filter, - Ash.Filter.move_to_relationship_path(path_filter, path) + Ash.Filter.move_to_relationship_path( + join_filter, + rest ++ [first_relationship.name] + ) ) - |> Ash.Filter.map(fn - %Ash.Query.Parent{expr: expr} -> - {:halt, Ash.Filter.move_to_relationship_path(expr, parent_path)} - other -> - other - end) - end) + :error -> + filter + end - # Joins for `rest` are derived from the refs in the filter and are left - # joins, so a predicate with no refs (e.g. `exists(a.bs, true)`) drops the - # remaining path entirely, and a null-satisfiable predicate is satisfied - # by null-extended rows. Requiring a non-nil primary key at every hop - # (not just the last: `no_attributes?` hops join with `on: true`) - # excludes both while being a no-op for real rows. - filter = - rest - |> Enum.scan([], fn rel_name, prefix -> prefix ++ [rel_name] end) - |> Enum.reduce(filter, fn prefix, filter -> - with target when not is_nil(target) <- - Ash.Resource.Info.related(first_relationship.destination, prefix), - [pk | _] <- Ash.Resource.Info.primary_key(target) do - pk_ref = %Ref{ - attribute: Ash.Resource.Info.attribute(target, pk), - relationship_path: prefix, - resource: target - } + filter = + exists + |> Map.get(:__join_filters__, %{}) + |> Map.delete([first_relationship.name]) + |> Enum.reduce(filter, fn {path, path_filter}, filter -> + path = Enum.drop(path, 1) + parent_path = :lists.droplast(path) + + Ash.Query.BooleanExpression.optimized_new( + :and, + filter, + Ash.Filter.move_to_relationship_path(path_filter, path) + ) + |> Ash.Filter.map(fn + %Ash.Query.Parent{expr: expr} -> + {:halt, Ash.Filter.move_to_relationship_path(expr, parent_path)} - Ash.Query.BooleanExpression.optimized_new( - :and, - filter, - %Ash.Query.Operator.IsNil{left: pk_ref, right: false} - ) - else - _ -> filter - end + other -> + other end) + end) - query = - if first_relationship.type == :many_to_many do - put_in(query.__ash_bindings__[:lateral_join_bindings], [:join_source]) - |> AshSql.Bindings.explicitly_set_binding( - %{ - type: :left, - path: [first_relationship.join_relationship] - }, - :join_source + # Joins for `rest` are derived from the refs in the filter and are left + # joins, so a predicate with no refs (e.g. `exists(a.bs, true)`) drops the + # remaining path entirely, and a null-satisfiable predicate is satisfied + # by null-extended rows. Requiring a non-nil primary key at every hop + # (not just the last: `no_attributes?` hops join with `on: true`) + # excludes both while being a no-op for real rows. + filter = + rest + |> Enum.scan([], fn rel_name, prefix -> prefix ++ [rel_name] end) + |> Enum.reduce(filter, fn prefix, filter -> + with target when not is_nil(target) <- + Ash.Resource.Info.related(first_relationship.destination, prefix), + [pk | _] <- Ash.Resource.Info.primary_key(target) do + pk_ref = %Ref{ + attribute: Ash.Resource.Info.attribute(target, pk), + relationship_path: prefix, + resource: target + } + + Ash.Query.BooleanExpression.optimized_new( + :and, + filter, + %Ash.Query.Operator.IsNil{left: pk_ref, right: false} ) else - query + _ -> filter end + end) + + query = + if first_relationship.type == :many_to_many do + put_in(query.__ash_bindings__[:lateral_join_bindings], [:join_source]) + |> AshSql.Bindings.explicitly_set_binding( + %{ + type: :left, + path: [first_relationship.join_relationship] + }, + :join_source + ) + else + query + end - {:ok, subquery} = - AshSql.Join.related_subquery(first_relationship, query, - filter: filter, - filter_subquery?: true, - sort?: Map.get(first_relationship, :from_many?) || not is_nil(first_relationship.sort), - start_bindings_at: 1, - select_star?: !Map.get(first_relationship, :manual), - in_group?: true, - refs_at_path: full_at_path, - parent_resources: [ - Ash.Resource.Info.related(resource, at_path) - | query.__ash_bindings__[:parent_resources] || [] - ], - return_subquery?: true, - on_subquery: fn subquery -> - subquery = - Ecto.Query.from(row in subquery, select: row) - |> Map.put(:__ash_bindings__, subquery.__ash_bindings__) - - cond do - Map.get(first_relationship, :manual) -> - {module, opts} = first_relationship.manual - - source_binding = - ref_binding( - %Ref{ - attribute: - Ash.Resource.Info.attribute(resource, first_relationship.source_attribute), - relationship_path: at_path, - resource: resource - }, - bindings - ) - - {:ok, subquery} = - apply( - module, - query.__ash_bindings__.sql_behaviour.manual_relationship_subquery_function(), - [ - opts, - source_binding, - 1, - subquery - ] - ) - - subquery - - Map.get(first_relationship, :no_attributes?) -> - subquery - - first_relationship.type == :many_to_many -> - source_ref = - ref_binding( - %Ref{ - attribute: - Ash.Resource.Info.attribute(resource, first_relationship.source_attribute), - relationship_path: at_path, - resource: resource - }, - bindings - ) - - through_relationship = - Ash.Resource.Info.relationship(resource, first_relationship.join_relationship) - - {:ok, through} = - AshSql.Join.related_subquery(through_relationship, query) - - Ecto.Query.from(destination in subquery, - join: through in ^through, - as: ^:join_source, - on: - field(through, ^first_relationship.destination_attribute_on_join_resource) == - field(destination, ^first_relationship.destination_attribute), - on: - field(parent_as(^source_ref), ^first_relationship.source_attribute) == - field(through, ^first_relationship.source_attribute_on_join_resource) + {:ok, subquery} = + AshSql.Join.related_subquery(first_relationship, query, + filter: filter, + filter_subquery?: true, + sort?: Map.get(first_relationship, :from_many?) || not is_nil(first_relationship.sort), + start_bindings_at: 1, + select_star?: !Map.get(first_relationship, :manual), + in_group?: true, + refs_at_path: full_at_path, + parent_resources: [ + Ash.Resource.Info.related(resource, at_path) + | query.__ash_bindings__[:parent_resources] || [] + ], + return_subquery?: true, + on_subquery: fn subquery -> + subquery = + Ecto.Query.from(row in subquery, select: row) + |> Map.put(:__ash_bindings__, subquery.__ash_bindings__) + + cond do + Map.get(first_relationship, :manual) -> + {module, opts} = first_relationship.manual + + source_binding = + ref_binding( + %Ref{ + attribute: + Ash.Resource.Info.attribute(resource, first_relationship.source_attribute), + relationship_path: at_path, + resource: resource + }, + bindings ) - true -> - source_ref = - ref_binding( - %Ref{ - attribute: - Ash.Resource.Info.attribute(resource, first_relationship.source_attribute), - relationship_path: at_path, - resource: resource - }, - bindings - ) - - Ecto.Query.from(destination in subquery, - where: - field(parent_as(^source_ref), ^first_relationship.source_attribute) == - field(destination, ^first_relationship.destination_attribute) + {:ok, subquery} = + apply( + module, + query.__ash_bindings__.sql_behaviour.manual_relationship_subquery_function(), + [ + opts, + source_binding, + 1, + subquery + ] ) - end + + subquery + + Map.get(first_relationship, :no_attributes?) -> + subquery + + first_relationship.type == :many_to_many -> + source_ref = + ref_binding( + %Ref{ + attribute: + Ash.Resource.Info.attribute(resource, first_relationship.source_attribute), + relationship_path: at_path, + resource: resource + }, + bindings + ) + + through_relationship = + Ash.Resource.Info.relationship(resource, first_relationship.join_relationship) + + {:ok, through} = + AshSql.Join.related_subquery(through_relationship, query) + + Ecto.Query.from(destination in subquery, + join: through in ^through, + as: ^:join_source, + on: + field(through, ^first_relationship.destination_attribute_on_join_resource) == + field(destination, ^first_relationship.destination_attribute), + on: + field(parent_as(^source_ref), ^first_relationship.source_attribute) == + field(through, ^first_relationship.source_attribute_on_join_resource) + ) + + true -> + source_ref = + ref_binding( + %Ref{ + attribute: + Ash.Resource.Info.attribute(resource, first_relationship.source_attribute), + relationship_path: at_path, + resource: resource + }, + bindings + ) + + Ecto.Query.from(destination in subquery, + where: + field(parent_as(^source_ref), ^first_relationship.source_attribute) == + field(destination, ^first_relationship.destination_attribute) + ) end - ) + end + ) - {Ecto.Query.dynamic(exists(subquery)), acc} - end + {Ecto.Query.dynamic(exists(subquery)), acc} end defp default_dynamic_expr( @@ -4422,115 +4434,17 @@ defmodule AshSql.Expr do defp get_path_array_type?(_), do: false - # EXISTS against a parent to-many path. Inner parent() is the related row, not the parent resource. - defp parent_exists_expr( - query, - %Exists{path: [first | rest], expr: expr}, - bindings, - embedded?, - acc - ) do - parent_resource = bindings.parent_bindings.resource - relationship = Ash.Resource.Info.relationship(parent_resource, first) - - if !relationship do - raise Ash.Error.Framework.AssumptionFailed, - message: """ - Unknown relationship #{inspect(parent_resource)}.#{first} - - in parent exists expression - """ - end - - filter = - case Ash.Filter.move_to_relationship_path(expr, rest) do - %Ash.Filter{expression: expression} -> expression - expression -> expression - end - - parent_query = %{ - query - | __ash_bindings__: Map.put(bindings.parent_bindings, :parent?, true) - } - - source_attr = relationship.source_attribute + defp correlate_exists_to_source_pk(subquery, outer_query, resource) do + source_binding = outer_query.__ash_bindings__.parent_bindings.root_binding - parent_binding = - ref_binding( - %Ref{ - attribute: Ash.Resource.Info.attribute(parent_resource, source_attr), - relationship_path: [], - resource: parent_resource - }, - bindings.parent_bindings - ) || bindings.parent_bindings.root_binding - - {:ok, subquery} = - AshSql.Join.related_subquery(relationship, parent_query, - filter_subquery?: true, - start_bindings_at: 1, - select_star?: true, - in_group?: true, - return_subquery?: true, - on_subquery: fn subquery -> - subquery = - Ecto.Query.from(row in subquery, select: row) - |> Map.put(:__ash_bindings__, subquery.__ash_bindings__) - - inner_bindings = - set_location( - subquery.__ash_bindings__ - |> Map.put(:parent_bindings, Map.put(query.__ash_bindings__, :parent?, true)) - |> Map.put(:parent?, false), - :sub_expr - ) - - {pred, _acc} = - if filter in [nil, true] do - {true, acc} - else - do_dynamic_expr( - %{subquery | __ash_bindings__: inner_bindings}, - filter, - inner_bindings, - embedded?, - acc - ) - end - - cond do - Map.get(relationship, :no_attributes?) -> - Ecto.Query.from(destination in subquery, where: ^pred) - - relationship.type == :many_to_many -> - through_relationship = - Ash.Resource.Info.relationship(parent_resource, relationship.join_relationship) - - {:ok, through} = - AshSql.Join.related_subquery(through_relationship, parent_query) - - Ecto.Query.from(destination in subquery, - join: through in ^through, - on: - field(through, ^relationship.destination_attribute_on_join_resource) == - field(destination, ^relationship.destination_attribute), - on: - field(parent_as(^parent_binding), ^source_attr) == - field(through, ^relationship.source_attribute_on_join_resource), - where: ^pred - ) - - true -> - Ecto.Query.from(destination in subquery, - where: - field(parent_as(^parent_binding), ^source_attr) == - field(destination, ^relationship.destination_attribute), - where: ^pred - ) - end - end + resource + |> Ash.Resource.Info.primary_key() + |> Enum.reduce(subquery, fn pk, subquery -> + Ecto.Query.where( + subquery, + [row], + field(parent_as(^source_binding), ^pk) == field(row, ^pk) ) - - {Ecto.Query.dynamic(exists(subquery)), acc} + end) end end diff --git a/lib/filter.ex b/lib/filter.ex index 0e7fb4e..a93fd6b 100644 --- a/lib/filter.ex +++ b/lib/filter.ex @@ -8,7 +8,6 @@ defmodule AshSql.Filter do require Ecto.Query def filter(query, filter, resource, opts \\ []) do - filter = rewrite_cartesian_parent_filter(query, filter) used_aggregates = Ash.Filter.used_aggregates(filter, []) query @@ -36,8 +35,8 @@ defmodule AshSql.Filter do end def add_filter_expression(query, filter) do - query - |> rewrite_cartesian_parent_filter(filter) + filter + |> maybe_wrap_source_exists(query) |> AshSql.Expr.split_statements(:and) |> Enum.reduce(query, fn filter, query -> {dynamic, acc} = @@ -60,10 +59,25 @@ defmodule AshSql.Filter do end) end - defp rewrite_cartesian_parent_filter(query, filter) do - AshSql.Join.rewrite_filter_if_cartesian_parent( - filter, - AshSql.Join.cartesian_parent_resource(query) - ) + defp maybe_wrap_source_exists(filter, query) do + if query.__ash_bindings__[:source_exists_parent_filter?] && + query.__ash_bindings__[:parent_bindings] do + rewritten = AshSql.Join.rewrite_parent_filter_for_source_exists(filter) + source = query.__ash_bindings__.parent_bindings.resource + + Map.put( + %Ash.Query.Exists{ + related?: false, + resource: source, + expr: rewritten, + path: [], + at_path: [] + }, + :correlate_source_pk, + true + ) + else + filter + end end end diff --git a/lib/join.ex b/lib/join.ex index ad90df0..74c9bbf 100644 --- a/lib/join.ex +++ b/lib/join.ex @@ -257,9 +257,10 @@ defmodule AshSql.Join do {:ok, query} lateral_join_source_query -> - # Joining two parent to-many paths cartesian-products rows. Use EXISTS instead. + # A to-many hop in parent() would be joined onto the lateral source and + # cartesian-product rows. Keep those joins inside a correlated EXISTS instead. as_exists? = - cartesian_parent_join?( + parent_expr_has_to_many?( filter, lateral_join_source_query.__ash_bindings__.resource ) @@ -276,11 +277,12 @@ defmodule AshSql.Join do {:ok, put_in(query.__ash_bindings__.lateral_join_source_query, lateral_join_source_query) |> Map.update!(:__ash_bindings__, fn bindings -> - Map.put( - bindings, + bindings + |> Map.put( :parent_bindings, Map.put(lateral_join_source_query.__ash_bindings__, :parent?, true) ) + |> Map.put(:source_exists_parent_filter?, as_exists?) end)} {:error, error} -> @@ -293,127 +295,41 @@ defmodule AshSql.Join do {:ok, query} end - # True when parent() names two or more to-many relationships (those would be joined). - defp cartesian_parent_join?(filter, parent_resource) do + # True when unwrapping parent() would join a to-many relationship at any hop. + defp parent_expr_has_to_many?(filter, parent_resource) do filter |> parent_expr() |> Ash.Filter.relationship_paths() |> Enum.reject(&(&1 == [])) - |> Enum.uniq() - |> Enum.filter(fn - [first | _] -> - case Ash.Resource.Info.relationship(parent_resource, first) do - %{cardinality: :many} -> true - _ -> false - end - - _ -> - false - end) - |> length() - |> Kernel.>=(2) - end - - defp maybe_rewrite_cartesian_parent_query(%{valid?: false} = query, _parent_resource), - do: query - - defp maybe_rewrite_cartesian_parent_query(query, parent_resource) do - %{query | filter: rewrite_filter_if_cartesian_parent(query.filter, parent_resource)} + |> Enum.any?(&to_many_in_path?(parent_resource, &1)) end - @doc false - def rewrite_filter_if_cartesian_parent(filter, parent_resource) - when is_nil(filter) or is_nil(parent_resource) or filter in [true, false] do - filter - end + defp to_many_in_path?(_resource, []), do: false - def rewrite_filter_if_cartesian_parent(filter, parent_resource) do - if cartesian_parent_join?(filter, parent_resource) do - rewrite_parent_paths_as_exists(filter, parent_resource) - else - filter + defp to_many_in_path?(resource, [name | rest]) do + case Ash.Resource.Info.relationship(resource, name) do + %{cardinality: :many} -> true + %{destination: dest} -> to_many_in_path?(dest, rest) + _ -> false end end + # parent(rel.field) is a join on the source EXISTS. Local fields stay parent() of the related row. @doc false - def cartesian_parent_resource(query) do - case query.__ash_bindings__[:lateral_join_source_query] do - %{__ash_bindings__: %{resource: resource}} -> - resource - - _ -> - case query.__ash_bindings__[:parent_bindings] do - %{resource: resource} -> resource - _ -> nil - end - end - end - - # Keep and/or/not. Turn each parent(to_many.*) leaf into exists from the parent. - defp rewrite_parent_paths_as_exists(filter, parent_resource) do + def rewrite_parent_filter_for_source_exists(filter) do Ash.Filter.map(filter, fn - %Ash.Query.BooleanExpression{} = expr -> - expr - - %Ash.Query.Not{} = expr -> - expr + %Ash.Query.Parent{expr: expr} -> + {:halt, expr} %Ash.Query.Exists{} = expr -> {:halt, expr} - expr -> - case parent_to_many_first_hops(expr, parent_resource) do - [first] -> - {:halt, exists_from_parent_path(expr, first)} + %Ref{relationship_path: []} = ref -> + {:halt, %Ash.Query.Parent{expr: ref}} - _ -> - expr - end - end) - end - - defp parent_to_many_first_hops(expr, parent_resource) do - expr - |> Ash.Filter.flat_map(fn - %Ash.Query.Parent{expr: %Ref{relationship_path: [first | _]}} -> - case Ash.Resource.Info.relationship(parent_resource, first) do - %{cardinality: :many} -> [first] - _ -> [] - end - - %Ash.Query.Parent{expr: inner} -> - parent_to_many_first_hops(inner, parent_resource) - - _ -> - [] + other -> + other end) - |> Enum.uniq() - end - - defp exists_from_parent_path(expr, first) do - rewritten = - Ash.Filter.map(expr, fn - %Ash.Query.Parent{expr: %Ref{relationship_path: [^first | rest]} = ref} -> - {:halt, %{ref | relationship_path: rest}} - - %Ref{relationship_path: []} = ref -> - {:halt, %Ash.Query.Parent{expr: ref}} - - other -> - other - end) - - inner = - case rewritten do - %Ash.Filter{expression: expression} -> expression - expression -> expression - end - - Map.put( - %Ash.Query.Exists{path: [first], expr: inner, at_path: [], related?: true}, - :from_parent, - true - ) end defp to_joins(paths, filter, resource) do @@ -638,7 +554,6 @@ defmodule AshSql.Join do end end) |> set_has_parent_expr_context(relationship) - |> maybe_rewrite_cartesian_parent_query(relationship.source) |> case do %{valid?: true} = related_query -> parent_bindings = From 4ef6c00d62b224788f262002ac03badd22a795af Mon Sep 17 00:00:00 2001 From: TravelCurry02 Date: Sat, 17 Oct 2026 19:30:40 -0600 Subject: [PATCH 4/5] fix: wrap only cartesian parent() filters in source EXISTS Co-authored-by: Cursor --- lib/aggregate/lateral.ex | 15 ++-- lib/expr.ex | 4 +- lib/filter.ex | 22 ++---- lib/join.ex | 153 +++++++++++++++++++++++++++++---------- 4 files changed, 126 insertions(+), 68 deletions(-) diff --git a/lib/aggregate/lateral.ex b/lib/aggregate/lateral.ex index 3017974..2e0572c 100644 --- a/lib/aggregate/lateral.ex +++ b/lib/aggregate/lateral.ex @@ -187,17 +187,12 @@ defmodule AshSql.Aggregate.Lateral do ) {:ok, query} = - AshSql.Join.join_all_relationships( + AshSql.Join.maybe_join_parent_expr( query, - parent_expr, - [], - nil, - [], - nil, - true, - nil, - nil, - true + first_relationship && first_relationship.filter, + parent_expr: parent_expr, + sort?: true, + no_inner_join?: true ) is_single? = match?([_], aggregates) diff --git a/lib/expr.ex b/lib/expr.ex index 5a880b1..ec32191 100644 --- a/lib/expr.ex +++ b/lib/expr.ex @@ -2890,7 +2890,7 @@ defmodule AshSql.Expr do |> then(fn related_query -> if expr != true do parent_stack = - if Map.get(exists, :correlate_source_pk) do + if exists.correlate_source_pk do [query.__ash_bindings__.resource] else [] @@ -2908,7 +2908,7 @@ defmodule AshSql.Expr do ecto_query = Ecto.Query.exclude(ecto_query, :select) ecto_query = - if Map.get(exists, :correlate_source_pk) do + if exists.correlate_source_pk do correlate_exists_to_source_pk(ecto_query, query, resource) else ecto_query diff --git a/lib/filter.ex b/lib/filter.ex index a93fd6b..13926f4 100644 --- a/lib/filter.ex +++ b/lib/filter.ex @@ -60,24 +60,12 @@ defmodule AshSql.Filter do end defp maybe_wrap_source_exists(filter, query) do - if query.__ash_bindings__[:source_exists_parent_filter?] && - query.__ash_bindings__[:parent_bindings] do - rewritten = AshSql.Join.rewrite_parent_filter_for_source_exists(filter) - source = query.__ash_bindings__.parent_bindings.resource + case query.__ash_bindings__[:parent_bindings] do + %{resource: resource} -> + AshSql.Join.wrap_cartesian_parent_filter(filter, resource) - Map.put( - %Ash.Query.Exists{ - related?: false, - resource: source, - expr: rewritten, - path: [], - at_path: [] - }, - :correlate_source_pk, - true - ) - else - filter + _ -> + filter end end end diff --git a/lib/join.ex b/lib/join.ex index 7077400..ff4bf4c 100644 --- a/lib/join.ex +++ b/lib/join.ex @@ -292,32 +292,18 @@ defmodule AshSql.Join do {:ok, query} lateral_join_source_query -> - # A to-many hop in parent() would be joined onto the lateral source and - # cartesian-product rows. Keep those joins inside a correlated EXISTS instead. - as_exists? = - parent_expr_has_to_many?( - filter, - lateral_join_source_query.__ash_bindings__.resource - ) - - parent_filter = - if as_exists? do - true - else - parent_expr(filter) - end - - case join_all_relationships(lateral_join_source_query, parent_filter) do + # Two or more parent to-many paths joined onto the source cartesian-product + # rows. Keep those joins inside a correlated EXISTS instead. + case maybe_join_parent_expr(lateral_join_source_query, filter) do {:ok, lateral_join_source_query} -> {:ok, put_in(query.__ash_bindings__.lateral_join_source_query, lateral_join_source_query) |> Map.update!(:__ash_bindings__, fn bindings -> - bindings - |> Map.put( + Map.put( + bindings, :parent_bindings, Map.put(lateral_join_source_query.__ash_bindings__, :parent?, true) ) - |> Map.put(:source_exists_parent_filter?, as_exists?) end)} {:error, error} -> @@ -330,13 +316,92 @@ defmodule AshSql.Join do {:ok, query} end - # True when unwrapping parent() would join a to-many relationship at any hop. - defp parent_expr_has_to_many?(filter, parent_resource) do + # Skip joining parent() paths onto `query` when that would cartesian-product to-many lists. + @doc false + def maybe_join_parent_expr(query, destination_filter, opts \\ []) do + if cartesian_parent_join?(destination_filter, query.__ash_bindings__.resource) do + {:ok, query} + else + join_all_relationships( + query, + Keyword.get(opts, :parent_expr, parent_expr(destination_filter)), + opts[:join_opts] || [], + opts[:relationship_paths], + opts[:path] || [], + opts[:source], + Keyword.get(opts, :sort?, true), + opts[:join_filters], + opts[:parent_query], + Keyword.get(opts, :no_inner_join?, false) + ) + end + end + + # Wrap only parent() subexpressions that would join two to-many source paths. + @doc false + def wrap_cartesian_parent_filter(filter, parent_resource) + when is_nil(filter) or is_nil(parent_resource) or filter in [true, false] do + filter + end + + def wrap_cartesian_parent_filter(%Ash.Filter{expression: expression} = filter, parent_resource) do + %{filter | expression: wrap_cartesian_parent_filter(expression, parent_resource)} + end + + def wrap_cartesian_parent_filter( + %Ash.Query.BooleanExpression{op: :and, left: left, right: right} = expr, + parent_resource + ) do + %{ + expr + | left: wrap_cartesian_parent_filter(left, parent_resource), + right: wrap_cartesian_parent_filter(right, parent_resource) + } + end + + def wrap_cartesian_parent_filter( + %Ash.Query.BooleanExpression{op: :or} = expr, + parent_resource + ) do + if cartesian_parent_join?(expr, parent_resource) do + parent_filter_as_source_exists(expr, parent_resource) + else + %{ + expr + | left: wrap_cartesian_parent_filter(expr.left, parent_resource), + right: wrap_cartesian_parent_filter(expr.right, parent_resource) + } + end + end + + def wrap_cartesian_parent_filter(%Ash.Query.Not{expression: expression} = expr, parent_resource) do + %{expr | expression: wrap_cartesian_parent_filter(expression, parent_resource)} + end + + def wrap_cartesian_parent_filter(%Ash.Query.Exists{} = expr, _parent_resource), do: expr + + def wrap_cartesian_parent_filter(expr, parent_resource) do + if cartesian_parent_join?(expr, parent_resource) do + parent_filter_as_source_exists(expr, parent_resource) + else + expr + end + end + + defp cartesian_parent_join?(filter, _parent_resource) + when is_nil(filter) or filter in [true, false] do + false + end + + defp cartesian_parent_join?(filter, parent_resource) do filter |> parent_expr() |> Ash.Filter.relationship_paths() |> Enum.reject(&(&1 == [])) - |> Enum.any?(&to_many_in_path?(parent_resource, &1)) + |> Enum.uniq() + |> Enum.filter(&to_many_in_path?(parent_resource, &1)) + |> length() + |> Kernel.>=(2) end defp to_many_in_path?(_resource, []), do: false @@ -349,9 +414,27 @@ defmodule AshSql.Join do end end + defp parent_filter_as_source_exists(expr, parent_resource) do + rewritten = rewrite_parent_filter_for_source_exists(expr) + + inner = + case rewritten do + %Ash.Filter{expression: expression} -> expression + expression -> expression + end + + %Ash.Query.Exists{ + related?: false, + resource: parent_resource, + expr: inner, + path: [], + at_path: [], + correlate_source_pk: true + } + end + # parent(rel.field) is a join on the source EXISTS. Local fields stay parent() of the related row. - @doc false - def rewrite_parent_filter_for_source_exists(filter) do + defp rewrite_parent_filter_for_source_exists(filter) do Ash.Filter.map(filter, fn %Ash.Query.Parent{expr: expr} -> {:halt, expr} @@ -1122,7 +1205,7 @@ defmodule AshSql.Join do sort?, apply_filter, used_parent_aggregates, - parent_expr + destination_filter ) else join_relationship = @@ -1147,7 +1230,7 @@ defmodule AshSql.Join do }) |> AshSql.Bindings.add_binding(binding_data) - with {:ok, query} <- join_all_relationships(query, parent_expr), + with {:ok, query} <- maybe_join_parent_expr(query, destination_filter), {:ok, relationship_through} <- related_subquery(join_relationship, query), {:ok, relationship_destination} <- related_subquery(relationship, query, @@ -1292,12 +1375,8 @@ defmodule AshSql.Join do require_lateral? = !Enum.empty?(used_parent_aggregates) with {:ok, query} <- - join_all_relationships( - query, - parent_expr, - [], - nil, - relationship_path_to_relationships(query.__ash_bindings__.resource, path) + maybe_join_parent_expr(query, destination_filter, + path: relationship_path_to_relationships(query.__ash_bindings__.resource, path) ), {:ok, query} <- AshSql.Aggregate.add_aggregates( @@ -1459,7 +1538,7 @@ defmodule AshSql.Join do sort?, apply_filter, used_parent_aggregates, - parent_expr + destination_filter ) do join_relationship = Ash.Resource.Info.relationship(relationship.source, relationship.join_relationship) @@ -1511,12 +1590,8 @@ defmodule AshSql.Join do ) with {:ok, query} <- - join_all_relationships( - query, - parent_expr, - [], - nil, - relationship_path_to_relationships(query.__ash_bindings__.resource, path) + maybe_join_parent_expr(query, destination_filter, + path: relationship_path_to_relationships(query.__ash_bindings__.resource, path) ), {:ok, query} <- AshSql.Aggregate.add_aggregates( From 27a40026a9c7e5cbd07ded76088475cb3dbe3979 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Sun, 18 Oct 2026 14:05:08 -0400 Subject: [PATCH 5/5] fix: build cartesian parent() EXISTS in ash_sql instead of Ash.Query.Exists Statements that reference parent() are grouped into one correlated EXISTS over the source, compiled by swapping bindings instead of rewriting the expression, so destination refs (including relationship paths) stay bound to the destination. Drops `correlate_source_pk`, so no ash core change is needed. Also hydrates relationship filters before the cartesian check, so the lateral aggregate and exists-through-relationship paths stop joining the parent paths onto the outer query, and resolves the source resource at nested paths. --- lib/aggregate/lateral.ex | 10 +-- lib/expr.ex | 44 ++------- lib/filter.ex | 21 ++--- lib/join.ex | 187 +++++++++++++++++++++++---------------- 4 files changed, 129 insertions(+), 133 deletions(-) diff --git a/lib/aggregate/lateral.ex b/lib/aggregate/lateral.ex index 2e0572c..426844e 100644 --- a/lib/aggregate/lateral.ex +++ b/lib/aggregate/lateral.ex @@ -164,12 +164,8 @@ defmodule AshSql.Aggregate.Lateral do parent_expr = if first_relationship do - first_relationship.filter - |> Ash.Filter.hydrate_refs(%{ - resource: first_relationship.destination, - parent_stack: [first_relationship.source] - }) - |> elem(1) + first_relationship + |> AshSql.Join.hydrated_relationship_filter() |> then(&[&1 | hydrated_agg_refs]) |> AshSql.Join.parent_expr() end @@ -189,7 +185,7 @@ defmodule AshSql.Aggregate.Lateral do {:ok, query} = AshSql.Join.maybe_join_parent_expr( query, - first_relationship && first_relationship.filter, + first_relationship && AshSql.Join.hydrated_relationship_filter(first_relationship), parent_expr: parent_expr, sort?: true, no_inner_join?: true diff --git a/lib/expr.ex b/lib/expr.ex index ec32191..3b5aad1 100644 --- a/lib/expr.ex +++ b/lib/expr.ex @@ -2435,7 +2435,7 @@ defmodule AshSql.Expr do new_field_name = query.__ash_bindings__.aggregate_names[field_name] - if !new_field_name do + unless new_field_name do raise "Unbound aggregate field: #{inspect(field_name)}" end @@ -2777,7 +2777,7 @@ defmodule AshSql.Expr do acc = %{acc | has_error?: true} - if !(Keyword.keyword?(input) || is_map(input)) do + unless Keyword.keyword?(input) || is_map(input) do raise "Input expression to `error` must be a map or keyword list" end @@ -2855,7 +2855,7 @@ defmodule AshSql.Expr do defp default_dynamic_expr( query, - %Exists{related?: false, expr: expr, resource: resource} = exists, + %Exists{related?: false, expr: expr, resource: resource}, bindings, _embedded?, acc, @@ -2889,14 +2889,7 @@ defmodule AshSql.Expr do |> AshSql.Join.hydrate_refs(bindings.context[:private][:actor]) |> then(fn related_query -> if expr != true do - parent_stack = - if exists.correlate_source_pk do - [query.__ash_bindings__.resource] - else - [] - end - - Ash.Query.do_filter(related_query, expr, parent_stack: parent_stack) + Ash.Query.do_filter(related_query, expr) else related_query end @@ -2905,16 +2898,7 @@ defmodule AshSql.Expr do %{valid?: true} = related_query -> case Ash.Query.data_layer_query(related_query) do {:ok, ecto_query} -> - ecto_query = Ecto.Query.exclude(ecto_query, :select) - - ecto_query = - if exists.correlate_source_pk do - correlate_exists_to_source_pk(ecto_query, query, resource) - else - ecto_query - end - - {:ok, ecto_query} + {:ok, Ecto.Query.exclude(ecto_query, :select)} {:error, error} -> {:error, error} @@ -4078,7 +4062,7 @@ defmodule AshSql.Expr do acc ) when is_atom(field) do - if !(to_string(field) =~ ~r/^[a-zA-Z_][a-zA-Z0-9_]*$/) do + unless to_string(field) =~ ~r/^[a-zA-Z_][a-zA-Z0-9_]*$/ do raise Ash.Error.Query.InvalidExpression, expression: field, message: "#{inspect(field)} is not a valid composite type field name" @@ -4124,7 +4108,7 @@ defmodule AshSql.Expr do installed_extensions = query.__ash_bindings__.sql_behaviour.repo(query.__ash_bindings__.resource, :mutate).installed_extensions() - if "ash-functions" not in installed_extensions do + unless "ash-functions" in installed_extensions do raise """ Cannot use `#{operator}` without adding the extension `ash-functions` to your repo. @@ -4433,18 +4417,4 @@ defmodule AshSql.Expr do defp get_path_array_type?({type, _}) when is_tuple(type), do: get_path_array_type?(type) defp get_path_array_type?(_), do: false - - defp correlate_exists_to_source_pk(subquery, outer_query, resource) do - source_binding = outer_query.__ash_bindings__.parent_bindings.root_binding - - resource - |> Ash.Resource.Info.primary_key() - |> Enum.reduce(subquery, fn pk, subquery -> - Ecto.Query.where( - subquery, - [row], - field(parent_as(^source_binding), ^pk) == field(row, ^pk) - ) - end) - end end diff --git a/lib/filter.ex b/lib/filter.ex index 13926f4..b632e0a 100644 --- a/lib/filter.ex +++ b/lib/filter.ex @@ -36,15 +36,10 @@ defmodule AshSql.Filter do def add_filter_expression(query, filter) do filter - |> maybe_wrap_source_exists(query) |> AshSql.Expr.split_statements(:and) + |> AshSql.Join.group_cartesian_parent_statements(query) |> Enum.reduce(query, fn filter, query -> - {dynamic, acc} = - AshSql.Expr.dynamic_expr( - query, - filter, - Map.put(query.__ash_bindings__, :location, :filter) - ) + {dynamic, acc} = filter_dynamic(query, filter) dynamic = if is_nil(dynamic) do @@ -59,13 +54,11 @@ defmodule AshSql.Filter do end) end - defp maybe_wrap_source_exists(filter, query) do - case query.__ash_bindings__[:parent_bindings] do - %{resource: resource} -> - AshSql.Join.wrap_cartesian_parent_filter(filter, resource) + defp filter_dynamic(query, {:parent_source_exists, filter}) do + AshSql.Join.parent_source_exists_expr(query, filter) + end - _ -> - filter - end + defp filter_dynamic(query, filter) do + AshSql.Expr.dynamic_expr(query, filter, Map.put(query.__ash_bindings__, :location, :filter)) end end diff --git a/lib/join.ex b/lib/join.ex index ff4bf4c..156e9cb 100644 --- a/lib/join.ex +++ b/lib/join.ex @@ -319,7 +319,13 @@ defmodule AshSql.Join do # Skip joining parent() paths onto `query` when that would cartesian-product to-many lists. @doc false def maybe_join_parent_expr(query, destination_filter, opts \\ []) do - if cartesian_parent_join?(destination_filter, query.__ash_bindings__.resource) do + source = + case opts[:path] do + [_ | _] = path -> List.last(path).destination + _ -> query.__ash_bindings__.resource + end + + if cartesian_parent_join?(destination_filter, source) do {:ok, query} else join_all_relationships( @@ -337,55 +343,120 @@ defmodule AshSql.Join do end end - # Wrap only parent() subexpressions that would join two to-many source paths. + # When `maybe_join_parent_expr/3` skipped the parent joins, every statement that + # references parent() is compiled together into one EXISTS over the source. @doc false - def wrap_cartesian_parent_filter(filter, parent_resource) - when is_nil(filter) or is_nil(parent_resource) or filter in [true, false] do - filter + def group_cartesian_parent_statements(statements, query) do + with %{} = parent_bindings <- query.__ash_bindings__[:parent_bindings], + {[_ | _] = parent_statements, rest} <- Enum.split_with(statements, &parent_ref?/1), + true <- + cartesian_parent_join?(and_all(parent_statements), parent_source(parent_bindings)) do + [{:parent_source_exists, and_all(parent_statements)} | rest] + else + _ -> statements + end end - def wrap_cartesian_parent_filter(%Ash.Filter{expression: expression} = filter, parent_resource) do - %{filter | expression: wrap_cartesian_parent_filter(expression, parent_resource)} - end + # EXISTS (SELECT 1 FROM source s JOIN WHERE s.pk = .pk AND filter) + # + # The filter is compiled from the destination with its bindings swapped: destination refs + # resolve to `parent_as` the outer query, and parent() refs resolve to `as` inside the subquery. + @doc false + def parent_source_exists_expr(query, filter) do + bindings = query.__ash_bindings__ + parent_bindings = bindings.parent_bindings + source = parent_source(parent_bindings) + parent_expr = parent_expr(filter) + + with {:ok, source_query} <- + source + |> Ash.Query.new() + |> Ash.Query.set_tenant(bindings.context[:private][:tenant]) + |> Ash.Query.set_context(%{data_layer: %{start_bindings_at: bindings.current + 1}}) + |> Ash.Query.data_layer_query(), + source_query = + put_in( + source_query.__ash_bindings__[:parent_bindings], + parent_bindings[:parent_bindings] + ), + {:ok, source_query} <- join_all_relationships(source_query, parent_expr), + {:ok, source_query} <- + AshSql.Aggregate.add_aggregates( + source_query, + Ash.Filter.used_aggregates(parent_expr, []), + source, + false, + source_query.__ash_bindings__.root_binding + ) do + inner_bindings = + bindings + |> Map.merge(%{ + parent?: true, + location: :filter, + current: source_query.__ash_bindings__.current, + parent_bindings: Map.put(source_query.__ash_bindings__, :parent_is_parent_as?, false) + }) - def wrap_cartesian_parent_filter( - %Ash.Query.BooleanExpression{op: :and, left: left, right: right} = expr, - parent_resource - ) do - %{ - expr - | left: wrap_cartesian_parent_filter(left, parent_resource), - right: wrap_cartesian_parent_filter(right, parent_resource) - } - end + {dynamic, acc} = + AshSql.Expr.dynamic_expr( + %{query | __ash_bindings__: inner_bindings}, + filter, + inner_bindings + ) - def wrap_cartesian_parent_filter( - %Ash.Query.BooleanExpression{op: :or} = expr, - parent_resource - ) do - if cartesian_parent_join?(expr, parent_resource) do - parent_filter_as_source_exists(expr, parent_resource) + root_binding = source_query.__ash_bindings__.root_binding + + source_query = + source + |> Ash.Resource.Info.primary_key() + |> Enum.reduce(source_query, fn pk, source_query -> + source_row_binding = + AshSql.Expr.ref_binding( + %Ref{ + attribute: Ash.Resource.Info.attribute(source, pk), + relationship_path: [], + resource: source + }, + parent_bindings + ) + + from(row in source_query, + where: field(as(^root_binding), ^pk) == field(parent_as(^source_row_binding), ^pk) + ) + end) + |> Ecto.Query.where(^dynamic) + |> Ecto.Query.exclude(:select) + |> Ecto.Query.exclude(:distinct) + + {Ecto.Query.dynamic(exists(source_query)), acc} else - %{ - expr - | left: wrap_cartesian_parent_filter(expr.left, parent_resource), - right: wrap_cartesian_parent_filter(expr.right, parent_resource) - } + {:error, error} -> raise Ash.Error.to_ash_error(error) end end - def wrap_cartesian_parent_filter(%Ash.Query.Not{expression: expression} = expr, parent_resource) do - %{expr | expression: wrap_cartesian_parent_filter(expression, parent_resource)} + @doc false + def hydrated_relationship_filter(%{filter: nil}), do: nil + + def hydrated_relationship_filter(relationship) do + case Ash.Filter.hydrate_refs(relationship.filter, %{ + resource: relationship.destination, + parent_stack: [relationship.source] + }) do + {:ok, filter} -> filter + _ -> relationship.filter + end end - def wrap_cartesian_parent_filter(%Ash.Query.Exists{} = expr, _parent_resource), do: expr + defp parent_source(parent_bindings) do + Ash.Resource.Info.related(parent_bindings.resource, parent_bindings[:refs_at_path] || []) + end - def wrap_cartesian_parent_filter(expr, parent_resource) do - if cartesian_parent_join?(expr, parent_resource) do - parent_filter_as_source_exists(expr, parent_resource) - else - expr - end + defp parent_ref?(statement) do + !!Ash.Filter.find(statement, &match?(%Ash.Query.Parent{}, &1)) + end + + defp and_all(statements) do + Enum.reduce(statements, &Ash.Query.BooleanExpression.optimized_new(:and, &2, &1)) end defp cartesian_parent_join?(filter, _parent_resource) @@ -414,42 +485,6 @@ defmodule AshSql.Join do end end - defp parent_filter_as_source_exists(expr, parent_resource) do - rewritten = rewrite_parent_filter_for_source_exists(expr) - - inner = - case rewritten do - %Ash.Filter{expression: expression} -> expression - expression -> expression - end - - %Ash.Query.Exists{ - related?: false, - resource: parent_resource, - expr: inner, - path: [], - at_path: [], - correlate_source_pk: true - } - end - - # parent(rel.field) is a join on the source EXISTS. Local fields stay parent() of the related row. - defp rewrite_parent_filter_for_source_exists(filter) do - Ash.Filter.map(filter, fn - %Ash.Query.Parent{expr: expr} -> - {:halt, expr} - - %Ash.Query.Exists{} = expr -> - {:halt, expr} - - %Ref{relationship_path: []} = ref -> - {:halt, %Ash.Query.Parent{expr: ref}} - - other -> - other - end) - end - defp to_joins(paths, filter, resource) do paths |> Enum.reject(&(&1 == [])) @@ -479,7 +514,9 @@ defmodule AshSql.Join do # Get the first relationship relationship = Ash.Resource.Info.relationship(resource, first_rel_name) - if relationship && relationship.filter do + # Cartesian parent paths are joined inside the EXISTS instead, see `parent_source_exists_expr/2` + if relationship && relationship.filter && + !cartesian_parent_join?(hydrated_relationship_filter(relationship), resource) do # Extract parent expressions from the relationship filter extract_parent_refs_from_filter(relationship.filter) else