Skip to content

Commit

Permalink
Fix: wrap in a subquery when aggregate query has combinations
Browse files Browse the repository at this point in the history
  • Loading branch information
smartepsh committed Aug 6, 2021
1 parent becc9f4 commit 4d6da2b
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 4 deletions.
2 changes: 1 addition & 1 deletion lib/ecto/repo.ex
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,7 @@ defmodule Ecto.Repo do
@doc """
Calculate the given `aggregate`.
If the query has a limit, offset or distinct set, it will be
If the query has a limit, offset, distinct or combination set, it will be
automatically wrapped in a subquery in order to return the
proper result.
Expand Down
4 changes: 2 additions & 2 deletions lib/ecto/repo/queryable.ex
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ defmodule Ecto.Repo.Queryable do
defp query_for_aggregate(queryable, aggregate) do
query =
case prepare_for_aggregate(queryable) do
%{distinct: nil, limit: nil, offset: nil} = query ->
%{distinct: nil, limit: nil, offset: nil, combinations: []} = query ->
%{query | order_bys: []}

query ->
Expand All @@ -491,7 +491,7 @@ defmodule Ecto.Repo.Queryable do

query =
case prepare_for_aggregate(queryable) do
%{distinct: nil, limit: nil, offset: nil} = query ->
%{distinct: nil, limit: nil, offset: nil, combinations: []} = query ->
%{query | order_bys: []}

query ->
Expand Down
13 changes: 12 additions & 1 deletion test/ecto/repo_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ defmodule Ecto.RepoTest do
assert inspect(query) == "#Ecto.Query<from m0 in Ecto.RepoTest.MySchemaWithAssoc, select: count(m0.id)>"
end

test "removes order by from query without distinct/limit/offset" do
test "removes order by from query without distinct/limit/offset/combinations" do
from(MySchema, order_by: :id) |> TestRepo.aggregate(:count, :id)
assert_received {:all, query}
assert inspect(query) == "#Ecto.Query<from m0 in Ecto.RepoTest.MySchema, select: count(m0.id)>"
Expand All @@ -376,6 +376,17 @@ defmodule Ecto.RepoTest do
" select: %{id: m0.id}), select: count(m0.id)>"
end

test "uses subqueries with combinations" do
from(MySchema, union: ^from(MySchema)) |> TestRepo.aggregate(:count, :id)
assert_received {:all, query}

assert inspect(query) ==
"#Ecto.Query<from m0 in subquery(from m0 in Ecto.RepoTest.MySchema,\n" <>
" union: (from m0 in Ecto.RepoTest.MySchema,\n" <>
" select: m0),\n" <>
" select: %{id: m0.id}), select: count(m0.id)>"
end

test "raises when aggregating with group_by" do
assert_raise Ecto.QueryError, ~r"cannot aggregate on query with group_by", fn ->
from(MySchema, group_by: [:id]) |> TestRepo.aggregate(:count, :id)
Expand Down

0 comments on commit 4d6da2b

Please sign in to comment.