Skip to content

Commit a3c15a2

Browse files
zachdanielzachdaniel
authored
Support for subqueries in order_bys and group_bys (#607)
1 parent 14281b0 commit a3c15a2

File tree

10 files changed

+270
-46
lines changed

10 files changed

+270
-46
lines changed

‎integration_test/myxql/test_helper.exs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,9 @@ excludes = [
113113
# MySQL doesn't support indexed parameters
114114
:placeholders,
115115
# MySQL doesn't support specifying columns for ON DELETE SET NULL
116-
:on_delete_nilify_column_list
116+
:on_delete_nilify_column_list,
117+
# MySQL doesnt' support anything except a single column in DISTINCT
118+
:multicolumn_distinct
117119
]
118120

119121
if Version.match?(version, ">= 8.0.0") do

‎integration_test/sql/subquery.exs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,4 +112,42 @@ defmodule Ecto.Integration.SubQueryTest do
112112
select: fragment("? + ?", p.visits, ^1)
113113
assert [12, 12] = TestRepo.all(query)
114114
end
115+
116+
@tag :subquery_in_order_by
117+
test "subqueries in order by" do
118+
TestRepo.insert!(%Post{visits: 10, title: "hello"})
119+
TestRepo.insert!(%Post{visits: 11, title: "hello"})
120+
121+
query = from p in Post, as: :p, order_by: [asc: exists(from p in Post, where: p.visits > parent_as(:p).visits)]
122+
123+
assert [%{visits: 11}, %{visits: 10}] = TestRepo.all(query)
124+
end
125+
126+
@tag :multicolumn_distinct
127+
@tag :subquery_in_distinct
128+
test "subqueries in distinct" do
129+
TestRepo.insert!(%Post{visits: 10, title: "hello1"})
130+
TestRepo.insert!(%Post{visits: 10, title: "hello2"})
131+
TestRepo.insert!(%Post{visits: 11, title: "hello"})
132+
133+
query = from p in Post, as: :p, distinct: exists(from p in Post, where: p.visits > parent_as(:p).visits), order_by: [asc: :title]
134+
135+
assert [%{title: "hello"}, %{title: "hello1"}] = TestRepo.all(query)
136+
end
137+
138+
@tag :subquery_in_group_by
139+
test "subqueries in group by" do
140+
TestRepo.insert!(%Post{visits: 10, title: "hello1"})
141+
TestRepo.insert!(%Post{visits: 10, title: "hello2"})
142+
TestRepo.insert!(%Post{visits: 11, title: "hello"})
143+
144+
query = from p in Post, as: :p, select: sum(p.visits), group_by: exists(from p in Post, where: p.visits > parent_as(:p).visits), order_by: [sum(p.visits)]
145+
146+
query
147+
|> TestRepo.all()
148+
|> Enum.map(&Decimal.new/1)
149+
|> Enum.zip([Decimal.new(11), Decimal.new(20)])
150+
|> Enum.all?(fn {a, b} -> Decimal.eq?(a, b) end)
151+
|> assert()
152+
end
115153
end

‎integration_test/tds/test_helper.exs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,13 @@ ExUnit.start(
5757
# MSSQL can't reference aliased columns in ORDER BY expressions
5858
:selected_as_with_order_by_expression,
5959
# MSSQL doesn't support specifying columns for ON DELETE SET NULL
60-
:on_delete_nilify_column_list
60+
:on_delete_nilify_column_list,
61+
# MSSQL doesnt' support anything except a single column in DISTINCT
62+
:multicolumn_distinct,
63+
# MSSQL doesnt' support subqueries in group by or in distinct
64+
:subquery_in_group_by,
65+
:subquery_in_distinct,
66+
:subquery_in_order_by
6167
]
6268
)
6369

‎lib/ecto/adapters/myxql/connection.ex

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ if Code.ensure_loaded?(MyXQL) do
7575
## Query
7676

7777
@parent_as __MODULE__
78-
alias Ecto.Query.{BooleanExpr, JoinExpr, QueryExpr, WithExpr}
78+
alias Ecto.Query.{BooleanExpr, ByExpr, JoinExpr, QueryExpr, WithExpr}
7979

8080
@impl true
8181
def all(query, as_prefix \\ []) do
@@ -319,10 +319,10 @@ if Code.ensure_loaded?(MyXQL) do
319319
end
320320

321321
defp distinct(nil, _sources, _query), do: []
322-
defp distinct(%QueryExpr{expr: true}, _sources, _query), do: "DISTINCT "
323-
defp distinct(%QueryExpr{expr: false}, _sources, _query), do: []
322+
defp distinct(%ByExpr{expr: true}, _sources, _query), do: "DISTINCT "
323+
defp distinct(%ByExpr{expr: false}, _sources, _query), do: []
324324

325-
defp distinct(%QueryExpr{expr: exprs}, _sources, query) when is_list(exprs) do
325+
defp distinct(%ByExpr{expr: exprs}, _sources, query) when is_list(exprs) do
326326
error!(query, "DISTINCT with multiple columns is not supported by MySQL")
327327
end
328328

@@ -511,8 +511,8 @@ if Code.ensure_loaded?(MyXQL) do
511511
defp group_by(%{group_bys: group_bys} = query, sources) do
512512
[
513513
" GROUP BY "
514-
| Enum.map_intersperse(group_bys, ", ", fn %QueryExpr{expr: expr} ->
515-
Enum.map_intersperse(expr, ", ", &expr(&1, sources, query))
514+
| Enum.map_intersperse(group_bys, ", ", fn %ByExpr{expr: expr} ->
515+
Enum.map_intersperse(expr, ", ", &top_level_expr(&1, sources, query))
516516
end)
517517
]
518518
end
@@ -549,14 +549,14 @@ if Code.ensure_loaded?(MyXQL) do
549549
defp order_by(%{order_bys: order_bys} = query, sources) do
550550
[
551551
" ORDER BY "
552-
| Enum.map_intersperse(order_bys, ", ", fn %QueryExpr{expr: expr} ->
552+
| Enum.map_intersperse(order_bys, ", ", fn %ByExpr{expr: expr} ->
553553
Enum.map_intersperse(expr, ", ", &order_by_expr(&1, sources, query))
554554
end)
555555
]
556556
end
557557

558558
defp order_by_expr({dir, expr}, sources, query) do
559-
str = expr(expr, sources, query)
559+
str = top_level_expr(expr, sources, query)
560560

561561
case dir do
562562
:asc -> str
@@ -627,6 +627,21 @@ if Code.ensure_loaded?(MyXQL) do
627627
[?(, expr(expr, sources, query), ?)]
628628
end
629629

630+
defp top_level_expr(%Ecto.SubQuery{query: query}, sources, parent_query) do
631+
combinations =
632+
Enum.map(query.combinations, fn {type, combination_query} ->
633+
{type, put_in(combination_query.aliases[@parent_as], {parent_query, sources})}
634+
end)
635+
636+
query = put_in(query.combinations, combinations)
637+
query = put_in(query.aliases[@parent_as], {parent_query, sources})
638+
[all(query, subquery_as_prefix(sources))]
639+
end
640+
641+
defp top_level_expr(other, sources, parent_query) do
642+
expr(other, sources, parent_query)
643+
end
644+
630645
defp expr({:^, [], [_ix]}, _sources, _query) do
631646
~c"?"
632647
end
@@ -687,15 +702,8 @@ if Code.ensure_loaded?(MyXQL) do
687702
error!(query, "MySQL adapter does not support aggregate filters")
688703
end
689704

690-
defp expr(%Ecto.SubQuery{query: query}, sources, parent_query) do
691-
combinations =
692-
Enum.map(query.combinations, fn {type, combination_query} ->
693-
{type, put_in(combination_query.aliases[@parent_as], {parent_query, sources})}
694-
end)
695-
696-
query = put_in(query.combinations, combinations)
697-
query = put_in(query.aliases[@parent_as], {parent_query, sources})
698-
[?(, all(query, subquery_as_prefix(sources)), ?)]
705+
defp expr(%Ecto.SubQuery{} = subquery, sources, parent_query) do
706+
[?(, top_level_expr(subquery, sources, parent_query), ?)]
699707
end
700708

701709
defp expr({:fragment, _, [kw]}, _sources, query) when is_list(kw) or tuple_size(kw) == 3 do
@@ -790,7 +798,13 @@ if Code.ensure_loaded?(MyXQL) do
790798
[op_to_binary(left, sources, query), op | op_to_binary(right, sources, query)]
791799

792800
{:fun, fun} ->
793-
[fun, ?(, modifier, Enum.map_intersperse(args, ", ", &expr(&1, sources, query)), ?)]
801+
[
802+
fun,
803+
?(,
804+
modifier,
805+
Enum.map_intersperse(args, ", ", &top_level_expr(&1, sources, query)),
806+
?)
807+
]
794808
end
795809
end
796810

‎lib/ecto/adapters/postgres/connection.ex

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ if Code.ensure_loaded?(Postgrex) do
155155
end
156156

157157
@parent_as __MODULE__
158-
alias Ecto.Query.{BooleanExpr, JoinExpr, QueryExpr, WithExpr}
158+
alias Ecto.Query.{BooleanExpr, ByExpr, JoinExpr, QueryExpr, WithExpr}
159159

160160
@impl true
161161
def all(query, as_prefix \\ []) do
@@ -551,11 +551,11 @@ if Code.ensure_loaded?(Postgrex) do
551551
end
552552

553553
defp distinct(nil, _, _), do: {[], []}
554-
defp distinct(%QueryExpr{expr: []}, _, _), do: {[], []}
555-
defp distinct(%QueryExpr{expr: true}, _, _), do: {" DISTINCT", []}
556-
defp distinct(%QueryExpr{expr: false}, _, _), do: {[], []}
554+
defp distinct(%ByExpr{expr: []}, _, _), do: {[], []}
555+
defp distinct(%ByExpr{expr: true}, _, _), do: {" DISTINCT", []}
556+
defp distinct(%ByExpr{expr: false}, _, _), do: {[], []}
557557

558-
defp distinct(%QueryExpr{expr: exprs}, sources, query) do
558+
defp distinct(%ByExpr{expr: exprs}, sources, query) do
559559
{[
560560
" DISTINCT ON (",
561561
Enum.map_intersperse(exprs, ", ", fn {_, expr} -> expr(expr, sources, query) end),
@@ -772,7 +772,7 @@ if Code.ensure_loaded?(Postgrex) do
772772
[
773773
" GROUP BY "
774774
| Enum.map_intersperse(group_bys, ", ", fn
775-
%QueryExpr{expr: expr} ->
775+
%ByExpr{expr: expr} ->
776776
Enum.map_intersperse(expr, ", ", &expr(&1, sources, query))
777777
end)
778778
]

‎lib/ecto/adapters/tds/connection.ex

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ if Code.ensure_loaded?(Tds) do
149149

150150
@parent_as __MODULE__
151151
alias Ecto.Query
152-
alias Ecto.Query.{BooleanExpr, JoinExpr, QueryExpr, WithExpr}
152+
alias Ecto.Query.{BooleanExpr, ByExpr, JoinExpr, QueryExpr, WithExpr}
153153

154154
@impl true
155155
def all(query, as_prefix \\ []) do
@@ -390,10 +390,10 @@ if Code.ensure_loaded?(Tds) do
390390
end
391391

392392
defp distinct(nil, _sources, _query), do: []
393-
defp distinct(%QueryExpr{expr: true}, _sources, _query), do: "DISTINCT "
394-
defp distinct(%QueryExpr{expr: false}, _sources, _query), do: []
393+
defp distinct(%ByExpr{expr: true}, _sources, _query), do: "DISTINCT "
394+
defp distinct(%ByExpr{expr: false}, _sources, _query), do: []
395395

396-
defp distinct(%QueryExpr{expr: exprs}, _sources, query) when is_list(exprs) do
396+
defp distinct(%ByExpr{expr: exprs}, _sources, query) when is_list(exprs) do
397397
error!(
398398
query,
399399
"DISTINCT with multiple columns is not supported by MsSQL. " <>
@@ -584,8 +584,8 @@ if Code.ensure_loaded?(Tds) do
584584
defp group_by(%{group_bys: group_bys} = query, sources) do
585585
[
586586
" GROUP BY "
587-
| Enum.map_intersperse(group_bys, ", ", fn %QueryExpr{expr: expr} ->
588-
Enum.map_intersperse(expr, ", ", &expr(&1, sources, query))
587+
| Enum.map_intersperse(group_bys, ", ", fn %ByExpr{expr: expr} ->
588+
Enum.map_intersperse(expr, ", ", &top_level_expr(&1, sources, query))
589589
end)
590590
]
591591
end
@@ -595,14 +595,14 @@ if Code.ensure_loaded?(Tds) do
595595
defp order_by(%{order_bys: order_bys} = query, sources) do
596596
[
597597
" ORDER BY "
598-
| Enum.map_intersperse(order_bys, ", ", fn %QueryExpr{expr: expr} ->
598+
| Enum.map_intersperse(order_bys, ", ", fn %ByExpr{expr: expr} ->
599599
Enum.map_intersperse(expr, ", ", &order_by_expr(&1, sources, query))
600600
end)
601601
]
602602
end
603603

604604
defp order_by_expr({dir, expr}, sources, query) do
605-
str = expr(expr, sources, query)
605+
str = top_level_expr(expr, sources, query)
606606

607607
case dir do
608608
:asc -> str
@@ -708,6 +708,21 @@ if Code.ensure_loaded?(Tds) do
708708
[?(, expr(expr, sources, query), ?)]
709709
end
710710

711+
defp top_level_expr(%Ecto.SubQuery{query: query}, sources, parent_query) do
712+
combinations =
713+
Enum.map(query.combinations, fn {type, combination_query} ->
714+
{type, put_in(combination_query.aliases[@parent_as], {parent_query, sources})}
715+
end)
716+
717+
query = put_in(query.combinations, combinations)
718+
query = put_in(query.aliases[@parent_as], {parent_query, sources})
719+
[all(query, subquery_as_prefix(sources))]
720+
end
721+
722+
defp top_level_expr(other, sources, parent_query) do
723+
expr(other, sources, parent_query)
724+
end
725+
711726
# :^ - represents parameter ix is index number
712727
defp expr({:^, [], [idx]}, _sources, _query) do
713728
"@#{idx + 1}"
@@ -787,15 +802,8 @@ if Code.ensure_loaded?(Tds) do
787802
error!(query, "Tds adapter does not support aggregate filters")
788803
end
789804

790-
defp expr(%Ecto.SubQuery{query: query}, sources, parent_query) do
791-
combinations =
792-
Enum.map(query.combinations, fn {type, combination_query} ->
793-
{type, put_in(combination_query.aliases[@parent_as], {parent_query, sources})}
794-
end)
795-
796-
query = put_in(query.combinations, combinations)
797-
query = put_in(query.aliases[@parent_as], {parent_query, sources})
798-
[?(, all(query, subquery_as_prefix(sources)), ?)]
805+
defp expr(%Ecto.SubQuery{} = subquery, sources, parent_query) do
806+
[?(, top_level_expr(subquery, sources, parent_query), ?)]
799807
end
800808

801809
defp expr({:fragment, _, [kw]}, _sources, query) when is_list(kw) or tuple_size(kw) == 3 do
@@ -873,7 +881,13 @@ if Code.ensure_loaded?(Tds) do
873881
[op_to_binary(left, sources, query), op | op_to_binary(right, sources, query)]
874882

875883
{:fun, fun} ->
876-
[fun, ?(, modifier, Enum.map_intersperse(args, ", ", &expr(&1, sources, query)), ?)]
884+
[
885+
fun,
886+
?(,
887+
modifier,
888+
Enum.map_intersperse(args, ", ", &top_level_expr(&1, sources, query)),
889+
?)
890+
]
877891
end
878892
end
879893

‎mix.lock

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
"decimal": {:hex, :decimal, "2.1.1", "5611dca5d4b2c3dd497dec8f68751f1f1a54755e8ed2a966c2633cf885973ad6", [:mix], [], "hexpm", "53cfe5f497ed0e7771ae1a475575603d77425099ba5faef9394932b35020ffcc"},
55
"deep_merge": {:hex, :deep_merge, "1.0.0", "b4aa1a0d1acac393bdf38b2291af38cb1d4a52806cf7a4906f718e1feb5ee961", [:mix], [], "hexpm", "ce708e5f094b9cd4e8f2be4f00d2f4250c4095be93f8cd6d018c753894885430"},
66
"earmark_parser": {:hex, :earmark_parser, "1.4.39", "424642f8335b05bb9eb611aa1564c148a8ee35c9c8a8bba6e129d51a3e3c6769", [:mix], [], "hexpm", "06553a88d1f1846da9ef066b87b57c6f605552cfbe40d20bd8d59cc6bde41944"},
7-
"ecto": {:git, "https://github.com/elixir-ecto/ecto.git", "bed81b9a69f3425147fb57df1b3dd5fb4c95792c", []},
8-
"ex_doc": {:hex, :ex_doc, "0.32.2", "f60bbeb6ccbe75d005763e2a328e6f05e0624232f2393bc693611c2d3ae9fa0e", [:mix], [{:earmark_parser, "~> 1.4.39", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "a4480305cdfe7fdfcbb77d1092c76161626d9a7aa4fb698aee745996e34602df"},
7+
"ecto": {:git, "https://github.com/elixir-ecto/ecto.git", "a898915d2f16dbf1257b8c83a11a1ae07193de42", []},
8+
"ex_doc": {:hex, :ex_doc, "0.33.0", "690562b153153c7e4d455dc21dab86e445f66ceba718defe64b0ef6f0bd83ba0", [:mix], [{:earmark_parser, "~> 1.4.39", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "3f69adc28274cb51be37d09b03e4565232862a4b10288a3894587b0131412124"},
99
"jason": {:hex, :jason, "1.4.1", "af1504e35f629ddcdd6addb3513c3853991f694921b1b9368b0bd32beb9f1b63", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "fbb01ecdfd565b56261302f7e1fcc27c4fb8f32d56eab74db621fc154604a7a1"},
1010
"makeup": {:hex, :makeup, "1.1.2", "9ba8837913bdf757787e71c1581c21f9d2455f4dd04cfca785c70bbfff1a76a3", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cce1566b81fbcbd21eca8ffe808f33b221f9eee2cbc7a1706fc3da9ff18e6cac"},
1111
"makeup_elixir": {:hex, :makeup_elixir, "0.16.2", "627e84b8e8bf22e60a2579dad15067c755531fea049ae26ef1020cad58fe9578", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "41193978704763f6bbe6cc2758b84909e62984c7752b3784bd3c218bb341706b"},

0 commit comments

Comments
 (0)