Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for subqueries in order_bys and group_bys #607

Merged
merged 9 commits into from
May 28, 2024
33 changes: 24 additions & 9 deletions lib/ecto/adapters/myxql/connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ if Code.ensure_loaded?(MyXQL) do
## Query

@parent_as __MODULE__
alias Ecto.Query.{BooleanExpr, JoinExpr, QueryExpr, WithExpr}
alias Ecto.Query.{BooleanExpr, ByExpr, JoinExpr, QueryExpr, WithExpr}

@impl true
def all(query, as_prefix \\ []) do
Expand Down Expand Up @@ -319,10 +319,10 @@ if Code.ensure_loaded?(MyXQL) do
end

defp distinct(nil, _sources, _query), do: []
defp distinct(%QueryExpr{expr: true}, _sources, _query), do: "DISTINCT "
defp distinct(%QueryExpr{expr: false}, _sources, _query), do: []
defp distinct(%ByExpr{expr: true}, _sources, _query), do: "DISTINCT "
defp distinct(%ByExpr{expr: false}, _sources, _query), do: []

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

Expand Down Expand Up @@ -511,8 +511,8 @@ if Code.ensure_loaded?(MyXQL) do
defp group_by(%{group_bys: group_bys} = query, sources) do
[
" GROUP BY "
| Enum.map_intersperse(group_bys, ", ", fn %QueryExpr{expr: expr} ->
Enum.map_intersperse(expr, ", ", &expr(&1, sources, query))
| Enum.map_intersperse(group_bys, ", ", fn %ByExpr{expr: expr} ->
Enum.map_intersperse(expr, ", ", &expr({:top_level, &1}, sources, query))
end)
]
end
Expand Down Expand Up @@ -549,14 +549,14 @@ if Code.ensure_loaded?(MyXQL) do
defp order_by(%{order_bys: order_bys} = query, sources) do
[
" ORDER BY "
| Enum.map_intersperse(order_bys, ", ", fn %QueryExpr{expr: expr} ->
| Enum.map_intersperse(order_bys, ", ", fn %ByExpr{expr: expr} ->
Enum.map_intersperse(expr, ", ", &order_by_expr(&1, sources, query))
end)
]
end

defp order_by_expr({dir, expr}, sources, query) do
str = expr(expr, sources, query)
str = expr({:top_level, expr}, sources, query)
zachdaniel marked this conversation as resolved.
Show resolved Hide resolved

case dir do
:asc -> str
Expand Down Expand Up @@ -687,6 +687,17 @@ if Code.ensure_loaded?(MyXQL) do
error!(query, "MySQL adapter does not support aggregate filters")
end

defp expr({:top_level, %Ecto.SubQuery{query: query}}, sources, parent_query) do
combinations =
Enum.map(query.combinations, fn {type, combination_query} ->
{type, put_in(combination_query.aliases[@parent_as], {parent_query, sources})}
end)

query = put_in(query.combinations, combinations)
query = put_in(query.aliases[@parent_as], {parent_query, sources})
[all(query, subquery_as_prefix(sources))]
end
zachdaniel marked this conversation as resolved.
Show resolved Hide resolved

defp expr(%Ecto.SubQuery{query: query}, sources, parent_query) do
combinations =
Enum.map(query.combinations, fn {type, combination_query} ->
Expand Down Expand Up @@ -790,10 +801,14 @@ if Code.ensure_loaded?(MyXQL) do
[op_to_binary(left, sources, query), op | op_to_binary(right, sources, query)]

{:fun, fun} ->
[fun, ?(, modifier, Enum.map_intersperse(args, ", ", &expr(&1, sources, query)), ?)]
[fun, ?(, modifier, Enum.map_intersperse(args, ", ", &expr({:top_level, &1}, sources, query)), ?)]
end
end

defp expr({:top_level, other}, sources, parent_query) do
expr(other, sources, parent_query)
end

defp expr(list, _sources, query) when is_list(list) do
error!(query, "Array type is not supported by MySQL")
end
Expand Down
12 changes: 6 additions & 6 deletions lib/ecto/adapters/postgres/connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ if Code.ensure_loaded?(Postgrex) do
end

@parent_as __MODULE__
alias Ecto.Query.{BooleanExpr, JoinExpr, QueryExpr, WithExpr}
alias Ecto.Query.{BooleanExpr, ByExpr, JoinExpr, QueryExpr, WithExpr}

@impl true
def all(query, as_prefix \\ []) do
Expand Down Expand Up @@ -551,11 +551,11 @@ if Code.ensure_loaded?(Postgrex) do
end

defp distinct(nil, _, _), do: {[], []}
defp distinct(%QueryExpr{expr: []}, _, _), do: {[], []}
defp distinct(%QueryExpr{expr: true}, _, _), do: {" DISTINCT", []}
defp distinct(%QueryExpr{expr: false}, _, _), do: {[], []}
defp distinct(%ByExpr{expr: []}, _, _), do: {[], []}
defp distinct(%ByExpr{expr: true}, _, _), do: {" DISTINCT", []}
defp distinct(%ByExpr{expr: false}, _, _), do: {[], []}

defp distinct(%QueryExpr{expr: exprs}, sources, query) do
defp distinct(%ByExpr{expr: exprs}, sources, query) do
{[
" DISTINCT ON (",
Enum.map_intersperse(exprs, ", ", fn {_, expr} -> expr(expr, sources, query) end),
Expand Down Expand Up @@ -772,7 +772,7 @@ if Code.ensure_loaded?(Postgrex) do
[
" GROUP BY "
| Enum.map_intersperse(group_bys, ", ", fn
%QueryExpr{expr: expr} ->
%ByExpr{expr: expr} ->
Enum.map_intersperse(expr, ", ", &expr(&1, sources, query))
end)
]
Expand Down
12 changes: 6 additions & 6 deletions lib/ecto/adapters/tds/connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ if Code.ensure_loaded?(Tds) do

@parent_as __MODULE__
alias Ecto.Query
alias Ecto.Query.{BooleanExpr, JoinExpr, QueryExpr, WithExpr}
alias Ecto.Query.{BooleanExpr, ByExpr, JoinExpr, QueryExpr, WithExpr}

@impl true
def all(query, as_prefix \\ []) do
Expand Down Expand Up @@ -390,10 +390,10 @@ if Code.ensure_loaded?(Tds) do
end

defp distinct(nil, _sources, _query), do: []
defp distinct(%QueryExpr{expr: true}, _sources, _query), do: "DISTINCT "
defp distinct(%QueryExpr{expr: false}, _sources, _query), do: []
defp distinct(%ByExpr{expr: true}, _sources, _query), do: "DISTINCT "
defp distinct(%ByExpr{expr: false}, _sources, _query), do: []

defp distinct(%QueryExpr{expr: exprs}, _sources, query) when is_list(exprs) do
defp distinct(%ByExpr{expr: exprs}, _sources, query) when is_list(exprs) do
error!(
query,
"DISTINCT with multiple columns is not supported by MsSQL. " <>
Expand Down Expand Up @@ -584,7 +584,7 @@ if Code.ensure_loaded?(Tds) do
defp group_by(%{group_bys: group_bys} = query, sources) do
[
" GROUP BY "
| Enum.map_intersperse(group_bys, ", ", fn %QueryExpr{expr: expr} ->
| Enum.map_intersperse(group_bys, ", ", fn %ByExpr{expr: expr} ->
Enum.map_intersperse(expr, ", ", &expr(&1, sources, query))
end)
]
Expand All @@ -595,7 +595,7 @@ if Code.ensure_loaded?(Tds) do
defp order_by(%{order_bys: order_bys} = query, sources) do
[
" ORDER BY "
| Enum.map_intersperse(order_bys, ", ", fn %QueryExpr{expr: expr} ->
| Enum.map_intersperse(order_bys, ", ", fn %ByExpr{expr: expr} ->
Enum.map_intersperse(expr, ", ", &order_by_expr(&1, sources, query))
end)
]
Expand Down
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ defmodule EctoSQL.MixProject do
if path = System.get_env("ECTO_PATH") do
{:ecto, path: path}
else
{:ecto, github: "elixir-ecto/ecto"}
{:ecto, github: "zachdaniel/ecto", branch: "subqueries-in-order-by"}
zachdaniel marked this conversation as resolved.
Show resolved Hide resolved
end
end

Expand Down
4 changes: 2 additions & 2 deletions mix.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
"decimal": {:hex, :decimal, "2.1.1", "5611dca5d4b2c3dd497dec8f68751f1f1a54755e8ed2a966c2633cf885973ad6", [:mix], [], "hexpm", "53cfe5f497ed0e7771ae1a475575603d77425099ba5faef9394932b35020ffcc"},
"deep_merge": {:hex, :deep_merge, "1.0.0", "b4aa1a0d1acac393bdf38b2291af38cb1d4a52806cf7a4906f718e1feb5ee961", [:mix], [], "hexpm", "ce708e5f094b9cd4e8f2be4f00d2f4250c4095be93f8cd6d018c753894885430"},
"earmark_parser": {:hex, :earmark_parser, "1.4.39", "424642f8335b05bb9eb611aa1564c148a8ee35c9c8a8bba6e129d51a3e3c6769", [:mix], [], "hexpm", "06553a88d1f1846da9ef066b87b57c6f605552cfbe40d20bd8d59cc6bde41944"},
"ecto": {:git, "https://github.com/elixir-ecto/ecto.git", "bed81b9a69f3425147fb57df1b3dd5fb4c95792c", []},
"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"},
"ecto": {:git, "https://github.com/zachdaniel/ecto.git", "57fdecbf000720061be15c2ce24e2c6fc3ef0c3a", [branch: "subqueries-in-order-by"]},
"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"},
"jason": {:hex, :jason, "1.4.1", "af1504e35f629ddcdd6addb3513c3853991f694921b1b9368b0bd32beb9f1b63", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "fbb01ecdfd565b56261302f7e1fcc27c4fb8f32d56eab74db621fc154604a7a1"},
"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"},
"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"},
Expand Down
68 changes: 68 additions & 0 deletions test/ecto/adapters/myxql_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,22 @@ defmodule Ecto.Adapters.MyXQLTest do

all(query)
end

assert_raise Ecto.QueryError,
~r"DISTINCT with multiple columns is not supported by MySQL", fn ->
query =
from(row in Schema, as: :r, select: row.x)
|> distinct(
exists(
from other_schema in "schema",
where: other_schema.x == parent_as(:r).x,
select: [other_schema.x]
)
)
|> plan()

all(query)
end
end

test "coalesce" do
Expand Down Expand Up @@ -446,6 +462,23 @@ defmodule Ecto.Adapters.MyXQLTest do
Schema |> order_by([r], [{^dir, r.x}]) |> select([r], r.x) |> plan() |> all()
end
end

query =
from(row in Schema, as: :r)
|> order_by(
asc:
exists(
from other_schema in "schema",
where: other_schema.x == parent_as(:r).x,
select: [other_schema.x]
)
)
|> select([r], r.x)
|> plan()

assert all(query) ==
~s{SELECT s0.`x` FROM `schema` AS s0 ORDER BY exists(SELECT ss0.`x` AS `result` FROM `schema` AS ss0 WHERE (ss0.`x` = s0.`x`))}

end

test "union and union all" do
Expand Down Expand Up @@ -835,6 +868,21 @@ defmodule Ecto.Adapters.MyXQLTest do

query = Schema |> group_by([r], []) |> select([r], r.x) |> plan()
assert all(query) == ~s{SELECT s0.`x` FROM `schema` AS s0}

query =
from(row in Schema, as: :r, select: row.x)
|> group_by(
[r],
exists(
from other_schema in "schema",
where: other_schema.x == parent_as(:r).x,
select: [other_schema.x]
)
)
|> plan()

assert all(query) ==
~s{SELECT s0.`x` FROM `schema` AS s0 GROUP BY exists(SELECT ss0.`x` AS `result` FROM `schema` AS ss0 WHERE (ss0.`x` = s0.`x`))}
end

test "interpolated values" do
Expand Down Expand Up @@ -1024,6 +1072,26 @@ defmodule Ecto.Adapters.MyXQLTest do
~s{SELECT s0.`x` FROM `schema` AS s0 WINDOW `w` AS (PARTITION BY s0.`x`)}
end

test "window with subquery" do
query =
from(row in Schema, as: :r)
|> select([r], r.x)
|> windows([r],
w: [
order_by:
exists(
from other_schema in "schema",
where: other_schema.x == parent_as(:r).x,
select: [other_schema.x]
)
]
)
|> plan

assert all(query) ==
~s{SELECT s0.`x` FROM `schema` AS s0 WINDOW `w` AS (ORDER BY exists(SELECT ss0.`x` AS `result` FROM `schema` AS ss0 WHERE (ss0.`x` = s0.`x`)))}
end

test "two windows" do
query =
Schema
Expand Down
65 changes: 65 additions & 0 deletions test/ecto/adapters/postgres_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,20 @@

query = Schema |> distinct(false) |> select([r], {r.x, r.y}) |> plan()
assert all(query) == ~s{SELECT s0."x", s0."y" FROM "schema" AS s0}

query =
from(row in Schema, as: :r, select: row.x)
|> distinct(
exists(
from other_schema in "schema",
where: other_schema.x == parent_as(:r).x,
select: [other_schema.x]
)
)
|> plan()

assert all(query) ==
~s{SELECT DISTINCT ON (exists((SELECT ss0."x" AS "result" FROM "schema" AS ss0 WHERE (ss0."x" = s0."x")))) s0."x" FROM "schema" AS s0}
end

test "distinct with order by" do
Expand Down Expand Up @@ -618,6 +632,22 @@
assert all(query) ==
~s{SELECT s0."x" FROM "schema" AS s0 ORDER BY s0."x" ASC NULLS FIRST, s0."y" DESC NULLS FIRST}

query =
from(row in Schema, as: :r)
|> order_by(
asc:
exists(
from other_schema in "schema",
where: other_schema.x == parent_as(:r).x,
select: [other_schema.x]
)
)
|> select([r], r.x)
|> plan()

assert all(query) ==
~s{SELECT s0."x" FROM "schema" AS s0 ORDER BY exists((SELECT ss0."x" AS "result" FROM "schema" AS ss0 WHERE (ss0."x" = s0."x")))}

query =
Schema
|> order_by([r], asc_nulls_last: r.x, desc_nulls_last: r.y)
Expand Down Expand Up @@ -829,7 +859,7 @@
query = "schema" |> where(foo: "abc") |> select([], true) |> plan()
assert all(query) == ~s{SELECT TRUE FROM "schema" AS s0 WHERE (s0."foo" = 'abc')}

query = "schema" |> where(foo: <<0, ?a, ?b, ?c>>) |> select([], true) |> plan()

Check warning on line 862 in test/ecto/adapters/postgres_test.exs

View workflow job for this annotation

GitHub Actions / unittest (1.15.6, 24.3.4.13)

this check/guard will always yield the same result

assert all(query) ==
~s{SELECT TRUE FROM "schema" AS s0 WHERE (s0."foo" = '\\x00616263'::bytea)}
Expand Down Expand Up @@ -1057,6 +1087,21 @@

query = Schema |> group_by([r], []) |> select([r], r.x) |> plan()
assert all(query) == ~s{SELECT s0."x" FROM "schema" AS s0}

query =
from(row in Schema, as: :r, select: row.x)
|> group_by(
[r],
exists(
from other_schema in "schema",
where: other_schema.x == parent_as(:r).x,
select: [other_schema.x]
)
)
|> plan()

assert all(query) ==
~s{SELECT s0."x" FROM "schema" AS s0 GROUP BY exists((SELECT ss0."x" AS "result" FROM "schema" AS ss0 WHERE (ss0."x" = s0."x")))}
end

test "arrays and sigils" do
Expand Down Expand Up @@ -1364,6 +1409,26 @@
~s{SELECT s0."x" FROM "schema" AS s0 WINDOW "w" AS (PARTITION BY s0."x")}
end

test "window with subquery" do
query =
from(row in Schema, as: :r)
|> select([r], r.x)
|> windows([r],
w: [
order_by:
exists(
from other_schema in "schema",
where: other_schema.x == parent_as(:r).x,
select: [other_schema.x]
)
]
)
|> plan

assert all(query) ==
~s{SELECT s0."x" FROM "schema" AS s0 WINDOW "w" AS (ORDER BY exists((SELECT ss0."x" AS "result" FROM "schema" AS ss0 WHERE (ss0."x" = s0."x"))))}
end

test "two windows" do
query =
Schema
Expand Down
Loading
Loading