Repository: elixir-ecto/ecto
Pull request #2373 (2018-01-11 20:06:28)

add an exclusion for all the other join types
This should ideally add support for excluding `:inner_join`, `:cross_join`, `:left_join`, `:right_join`, :full_join`, `, :inner_lateral_join`, and finally `:left_lateral_join`.

josevalim: Beautiful. Can we have some tests and updated docs as well? Thanks!
BenMorganIO: @josevalim just finished that before you asked. I've moved that case statement to a pattern matched private function. I was trying to figure out how to keep it all DRY with the tests but I gave in to hard coding it all. Not that big of a problem until SQL decides a new join type; which probably won't happen for a really long time.

Patch:
diff --git a/lib/ecto/query.ex b/lib/ecto/query.ex
index 5e6d4f0d..a91b1e06 100644
--- a/lib/ecto/query.ex
+++ b/lib/ecto/query.ex
@@ -99,14 +99,14 @@ defmodule Ecto.Query do
 
   `nil` comparison in filters, such as where and having, is forbidden
   and it will raise an error:
-  
+
       # Raises if age is nil
       from u in User, where: u.age == ^age
-  
+
   This is done as a security measure to avoid attacks that attempt
   to traverse entries with nil columns. To check that value is `nil`,
   use `is_nil/1` instead:
-  
+
       from u in User, where: is_nil(u.age)
 
   ## Composition
@@ -469,6 +469,9 @@ 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,
+          :inner_lateral_join, :left_lateral_join]
+
   @doc """
   Resets a previously set field on a query.
 
@@ -483,6 +486,10 @@ defmodule Ecto.Query do
   def exclude(query, field), do: do_exclude(Ecto.Queryable.to_query(query), field)
 
   defp do_exclude(%Ecto.Query{} = query, :join), do: %{query | joins: []}
+  defp do_exclude(%Ecto.Query{} = query, join_keyword) when join_keyword in @joins do
+    qual = join_qual(join_keyword)
+    %{query | joins: Enum.reject(query.joins, &(&1.qual == qual))}
+  end
   defp do_exclude(%Ecto.Query{} = query, :where), do: %{query | wheres: []}
   defp do_exclude(%Ecto.Query{} = query, :order_by), do: %{query | order_bys: []}
   defp do_exclude(%Ecto.Query{} = query, :group_by), do: %{query | group_bys: []}
@@ -561,8 +568,6 @@ defmodule Ecto.Query do
   @binds    [:where, :or_where, :select, :distinct, :order_by, :group_by,
              :having, :or_having, :limit, :offset, :preload, :update, :select_merge]
   @no_binds [:lock]
-  @joins    [:join, :inner_join, :cross_join, :left_join, :right_join, :full_join,
-             :inner_lateral_join, :left_lateral_join]
 
   defp from([{type, expr}|t], env, count_bind, quoted, binds) when type in @binds do
     # If all bindings are integer indexes keep AST Macro expandable to %Query{},
@@ -592,18 +597,7 @@ defmodule Ecto.Query do
   end
 
   defp from([{join, expr}|t], env, count_bind, quoted, binds) when join in @joins do
-    qual =
-      case join do
-        :join -> :inner
-        :full_join -> :full
-        :left_join -> :left
-        :right_join -> :right
-        :inner_join -> :inner
-        :cross_join -> :cross
-        :left_lateral_join -> :left_lateral
-        :inner_lateral_join -> :inner_lateral
-      end
-
+    qual = join_qual(join)
     {t, on} = collect_on(t, nil)
     {quoted, binds, count_bind} = Join.build(quoted, qual, binds, expr, on, count_bind, env)
     from(t, env, count_bind, quoted, binds)
@@ -621,6 +615,15 @@ defmodule Ecto.Query do
     quoted
   end
 
+  defp join_qual(:join), do: :inner
+  defp join_qual(:full_join), do: :full
+  defp join_qual(:left_join), do: :left
+  defp join_qual(:right_join), do: :right
+  defp join_qual(:inner_join), do: :inner
+  defp join_qual(:cross_join), do: :cross
+  defp join_qual(:left_lateral_join), do: :left_lateral
+  defp join_qual(:inner_lateral_join), do: :inner_lateral
+
   defp collect_on([{:on, expr}|t], nil),
     do: collect_on(t, expr)
   defp collect_on([{:on, expr}|t], acc),


Test patch:
diff --git a/test/ecto/query_test.exs b/test/ecto/query_test.exs
index 92442a40..48c2cc5a 100644
--- a/test/ecto/query_test.exs
+++ b/test/ecto/query_test.exs
@@ -360,6 +360,47 @@ defmodule Ecto.QueryTest do
       assert excluded_query.preloads == base.preloads
       assert excluded_query.assocs == base.assocs
     end
+
+    test "removes join qualifiers" do
+      base = %Ecto.Query{}
+
+      inner_query         = from p in "posts", inner_join: b in "blogs"
+      cross_query         = from p in "posts", cross_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"
+      inner_lateral_query = from p in "posts", inner_lateral_join: b in "blogs"
+      left_lateral_query  = from p in "posts", left_lateral_join: b in "blogs"
+
+      refute inner_query.joins == base.joins
+      refute cross_query.joins == base.joins
+      refute left_query.joins == base.joins
+      refute right_query.joins == base.joins
+      refute full_query.joins == base.joins
+      refute inner_lateral_query.joins == base.joins
+      refute left_lateral_query.joins == base.joins
+
+      excluded_inner_query = exclude(inner_query, :inner_join)
+      assert excluded_inner_query.joins == base.joins
+
+      excluded_cross_query = exclude(cross_query, :cross_join)
+      assert excluded_cross_query.joins == base.joins
+
+      excluded_left_query = exclude(left_query, :left_join)
+      assert excluded_left_query.joins == base.joins
+
+      excluded_right_query = exclude(right_query, :right_join)
+      assert excluded_right_query.joins == base.joins
+
+      excluded_full_query = exclude(full_query, :full_join)
+      assert excluded_full_query.joins == base.joins
+
+      excluded_inner_lateral_query = exclude(inner_lateral_query, :inner_lateral_join)
+      assert excluded_inner_lateral_query.joins == base.joins
+
+      excluded_left_lateral_query = exclude(left_lateral_query, :left_lateral_join)
+      assert excluded_left_lateral_query.joins == base.joins
+    end
   end
 
   describe "fragment/1" do