From f151839a4fb99fdeff2e069c51ea83004fbbf67a Mon Sep 17 00:00:00 2001 From: William Wilkinson Date: Tue, 22 Nov 2022 21:22:21 -0600 Subject: [PATCH 1/3] Add support for cross lateral joins. --- integration_test/cases/joins.exs | 10 ++++++++++ lib/ecto/query.ex | 8 +++++--- lib/ecto/query/builder/join.ex | 4 ++-- lib/ecto/query/inspect.ex | 1 + lib/ecto/query/planner.ex | 4 ++-- test/ecto/query/planner_test.exs | 2 +- test/ecto/query_test.exs | 13 +++++++++++++ 7 files changed, 34 insertions(+), 8 deletions(-) diff --git a/integration_test/cases/joins.exs b/integration_test/cases/joins.exs index 543cf810a3..0a5015b14b 100644 --- a/integration_test/cases/joins.exs +++ b/integration_test/cases/joins.exs @@ -120,6 +120,16 @@ defmodule Ecto.Integration.JoinsTest do assert [{^p1, ^c1}, {^p2, ^c1}] = TestRepo.all(query) end + @tag :cross_lateral + test "cross lateral joins with missing entries" do + p1 = TestRepo.insert!(%Post{title: "1"}) + p2 = TestRepo.insert!(%Post{title: "2"}) + c1 = TestRepo.insert!(%Permalink{url: "1", post_id: p2.id}) + + query = from(p in Post, cross_lateral_join: c in Permalink, order_by: p.id, select: {p, c}) + assert [{^p1, ^c1}, {^p2, ^c1}] = TestRepo.all(query) + end + @tag :left_join test "left joins with missing entries" do p1 = TestRepo.insert!(%Post{title: "1"}) diff --git a/lib/ecto/query.ex b/lib/ecto/query.ex index 4a86360d90..51c17a0873 100644 --- a/lib/ecto/query.ex +++ b/lib/ecto/query.ex @@ -709,7 +709,7 @@ defmodule Ecto.Query do defp wrap_in_subquery(%Ecto.Query{} = query), do: %Ecto.SubQuery{query: query} defp wrap_in_subquery(queryable), do: %Ecto.SubQuery{query: Ecto.Queryable.to_query(queryable)} - @joins [:join, :inner_join, :cross_join, :left_join, :right_join, :full_join, + @joins [:join, :inner_join, :cross_join, :cross_lateral_join, :left_join, :right_join, :full_join, :inner_lateral_join, :left_lateral_join] @doc """ @@ -751,6 +751,7 @@ defmodule Ecto.Query do Ecto.Query.exclude(query, :inner_join) Ecto.Query.exclude(query, :cross_join) + Ecto.Query.exclude(query, :cross_lateral_join) Ecto.Query.exclude(query, :left_join) Ecto.Query.exclude(query, :right_join) Ecto.Query.exclude(query, :full_join) @@ -939,6 +940,7 @@ defmodule Ecto.Query do defp join_qual(:right_join), do: :right defp join_qual(:inner_join), do: :inner defp join_qual(:cross_join), do: :cross + defp join_qual(:cross_lateral_join), do: :cross_lateral defp join_qual(:left_lateral_join), do: :left_lateral defp join_qual(:inner_lateral_join), do: :inner_lateral @@ -975,10 +977,10 @@ defmodule Ecto.Query do Receives a source that is to be joined to the query and a condition for the join. The join condition can be any expression that evaluates to a boolean value. The qualifier must be one of `:inner`, `:left`, - `:right`, `:cross`, `:full`, `:inner_lateral` or `:left_lateral`. + `:right`, `:cross`, `:cross_lateral`, `:full`, `:inner_lateral` or `:left_lateral`. For a keyword query the `:join` keyword can be changed to `:inner_join`, - `:left_join`, `:right_join`, `:cross_join`, `:full_join`, `:inner_lateral_join` + `:left_join`, `:right_join`, `:cross_join`, `:cross_lateral_join`, `:full_join`, `:inner_lateral_join` or `:left_lateral_join`. `:join` is equivalent to `:inner_join`. Currently it is possible to join on: diff --git a/lib/ecto/query/builder/join.ex b/lib/ecto/query/builder/join.ex index acba57d9bb..3bbc7b784b 100644 --- a/lib/ecto/query/builder/join.ex +++ b/lib/ecto/query/builder/join.ex @@ -136,7 +136,7 @@ defmodule Ecto.Query.Builder.Join do unless is_binary(prefix) or is_nil(prefix) do Builder.error! "`prefix` must be a compile time string, got: `#{Macro.to_string(prefix)}`" end - + as = case as do {:^, _, [as]} -> as as when is_atom(as) -> as @@ -299,7 +299,7 @@ defmodule Ecto.Query.Builder.Join do end end - @qualifiers [:inner, :inner_lateral, :left, :left_lateral, :right, :full, :cross] + @qualifiers [:inner, :inner_lateral, :left, :left_lateral, :right, :full, :cross, :cross_lateral] @doc """ Called at runtime to check dynamic qualifier. diff --git a/lib/ecto/query/inspect.ex b/lib/ecto/query/inspect.ex index c0d2f778bd..98524bbf77 100644 --- a/lib/ecto/query/inspect.ex +++ b/lib/ecto/query/inspect.ex @@ -353,6 +353,7 @@ defimpl Inspect, for: Ecto.Query do defp join_qual(:right), do: :right_join defp join_qual(:full), do: :full_join defp join_qual(:cross), do: :cross_join + defp join_qual(:cross_lateral), do: :cross_join_lateral defp collect_sources(%{from: nil, joins: joins}) do ["query" | join_sources(joins)] diff --git a/lib/ecto/query/planner.ex b/lib/ecto/query/planner.ex index 17f8b23293..8f752281df 100644 --- a/lib/ecto/query/planner.ex +++ b/lib/ecto/query/planner.ex @@ -891,11 +891,11 @@ defmodule Ecto.Query.Planner do end case find_source_expr(query, child_ix) do - %JoinExpr{qual: qual} when qual in [:inner, :left, :inner_lateral, :left_lateral] -> + %JoinExpr{qual: qual} when qual in [:inner, :left, :inner_lateral, :left_lateral, :cross_lateral] -> :ok %JoinExpr{qual: qual} -> error! query, "association `#{inspect parent_schema}.#{assoc}` " <> - "in preload requires an inner, left or lateral join, got #{qual} join" + "in preload requires an inner, left, cross or lateral join, got #{qual} join" _ -> :ok end diff --git a/test/ecto/query/planner_test.exs b/test/ecto/query/planner_test.exs index 1e82e9ed9a..f9e9bf527a 100644 --- a/test/ecto/query/planner_test.exs +++ b/test/ecto/query/planner_test.exs @@ -1602,7 +1602,7 @@ defmodule Ecto.Query.PlannerTest do normalize(query) end - message = ~r"requires an inner, left or lateral join, got right join" + message = ~r"requires an inner, left, cross or lateral join, got right join" assert_raise Ecto.QueryError, message, fn -> query = from(p in Post, right_join: c in assoc(p, :comments), preload: [comments: c]) normalize(query) diff --git a/test/ecto/query_test.exs b/test/ecto/query_test.exs index 76287f3644..b6521229db 100644 --- a/test/ecto/query_test.exs +++ b/test/ecto/query_test.exs @@ -708,6 +708,7 @@ defmodule Ecto.QueryTest do inner_query = from p in "posts", inner_join: b in "blogs" cross_query = from p in "posts", cross_join: b in "blogs" + cross_lateral_query = from p in "posts", cross_lateral_join: b in "blogs" left_query = from p in "posts", left_join: b in "blogs" right_query = from p in "posts", right_join: b in "blogs" full_query = from p in "posts", full_join: b in "blogs" @@ -716,6 +717,7 @@ defmodule Ecto.QueryTest do refute inner_query.joins == base.joins refute cross_query.joins == base.joins + refute cross_lateral_query.joins == base.joins refute left_query.joins == base.joins refute right_query.joins == base.joins refute full_query.joins == base.joins @@ -728,6 +730,9 @@ defmodule Ecto.QueryTest do excluded_cross_query = exclude(cross_query, :cross_join) assert excluded_cross_query.joins == base.joins + excluded_cross_lateral_query = exclude(cross_lateral_query, :cross_lateral_join) + assert excluded_cross_lateral_query.joins == base.joins + excluded_left_query = exclude(left_query, :left_join) assert excluded_left_query.joins == base.joins @@ -751,6 +756,8 @@ defmodule Ecto.QueryTest do as: :blogs_i, cross_join: bc in "blogs", as: :blogs_c, + cross_lateral_join: bcl in "blogs", + as: :blogs_bcl, left_join: bl in "blogs", as: :blogs_l, right_join: br in "blogs", @@ -777,6 +784,12 @@ defmodule Ecto.QueryTest do refute Map.has_key?(excluded_cross_join_query.aliases, :blogs_c) assert Map.has_key?(excluded_cross_join_query.aliases, :base) + excluded_cross_lateral_join_query = exclude(query, :cross_lateral_join) + assert length(excluded_cross_lateral_join_query.joins) == original_joins_number - 1 + assert map_size(excluded_cross_lateral_join_query.aliases) == original_aliases_number - 1 + refute Map.has_key?(excluded_cross_lateral_join_query.aliases, :blogs_bcl) + assert Map.has_key?(excluded_cross_lateral_join_query.aliases, :base) + excluded_left_join_query = exclude(query, :left_join) assert length(excluded_left_join_query.joins) == original_joins_number - 1 assert map_size(excluded_left_join_query.aliases) == original_aliases_number - 1 From 648872890edf5cf06ec4b0edd990bfe654ea034b Mon Sep 17 00:00:00 2001 From: William Wilkinson Date: Tue, 22 Nov 2022 23:44:10 -0600 Subject: [PATCH 2/3] Fix cross lateral integration test in `cases/joins.exs`. --- integration_test/cases/joins.exs | 10 ---------- lib/ecto/query/planner.ex | 6 +++--- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/integration_test/cases/joins.exs b/integration_test/cases/joins.exs index 0a5015b14b..543cf810a3 100644 --- a/integration_test/cases/joins.exs +++ b/integration_test/cases/joins.exs @@ -120,16 +120,6 @@ defmodule Ecto.Integration.JoinsTest do assert [{^p1, ^c1}, {^p2, ^c1}] = TestRepo.all(query) end - @tag :cross_lateral - test "cross lateral joins with missing entries" do - p1 = TestRepo.insert!(%Post{title: "1"}) - p2 = TestRepo.insert!(%Post{title: "2"}) - c1 = TestRepo.insert!(%Permalink{url: "1", post_id: p2.id}) - - query = from(p in Post, cross_lateral_join: c in Permalink, order_by: p.id, select: {p, c}) - assert [{^p1, ^c1}, {^p2, ^c1}] = TestRepo.all(query) - end - @tag :left_join test "left joins with missing entries" do p1 = TestRepo.insert!(%Post{title: "1"}) diff --git a/lib/ecto/query/planner.ex b/lib/ecto/query/planner.ex index 8f752281df..1e15ab9018 100644 --- a/lib/ecto/query/planner.ex +++ b/lib/ecto/query/planner.ex @@ -329,7 +329,7 @@ defmodule Ecto.Query.Planner do defp normalize_subquery_types([{source_alias, type_value} | types], [field | fields], select_aliases, acc) do if Map.has_key?(select_aliases, source_alias) do raise ArgumentError, """ - the alias, #{inspect(source_alias)}, provided to `selected_as/2` conflicts + the alias, #{inspect(source_alias)}, provided to `selected_as/2` conflicts with the subquery's automatic aliasing. For example, the following query is not allowed because the alias `:y` @@ -891,11 +891,11 @@ defmodule Ecto.Query.Planner do end case find_source_expr(query, child_ix) do - %JoinExpr{qual: qual} when qual in [:inner, :left, :inner_lateral, :left_lateral, :cross_lateral] -> + %JoinExpr{qual: qual} when qual in [:inner, :left, :inner_lateral, :left_lateral] -> :ok %JoinExpr{qual: qual} -> error! query, "association `#{inspect parent_schema}.#{assoc}` " <> - "in preload requires an inner, left, cross or lateral join, got #{qual} join" + "in preload requires an inner, left or lateral join, got #{qual} join" _ -> :ok end From ae6f792752a4353fc33fe5ce6942c1b36021952f Mon Sep 17 00:00:00 2001 From: William Wilkinson Date: Wed, 23 Nov 2022 09:02:59 -0600 Subject: [PATCH 3/3] Fix broken test. --- test/ecto/query/planner_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ecto/query/planner_test.exs b/test/ecto/query/planner_test.exs index f9e9bf527a..1e82e9ed9a 100644 --- a/test/ecto/query/planner_test.exs +++ b/test/ecto/query/planner_test.exs @@ -1602,7 +1602,7 @@ defmodule Ecto.Query.PlannerTest do normalize(query) end - message = ~r"requires an inner, left, cross or lateral join, got right join" + message = ~r"requires an inner, left or lateral join, got right join" assert_raise Ecto.QueryError, message, fn -> query = from(p in Post, right_join: c in assoc(p, :comments), preload: [comments: c]) normalize(query)