-
Notifications
You must be signed in to change notification settings - Fork 53
Implement values #174
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
base: main
Are you sure you want to change the base?
Implement values #174
Changes from 7 commits
ff6b2e1
bbd1dbb
cafc720
40733b8
2b01d45
e851684
2054a79
3183b70
b86ecbb
99cec9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can shove this into the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inside Also, I didn't see any other similar tests in that directory, tests that actually operate on a Repo. Only in |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
defmodule Ecto.Integration.ValuesTest do | ||
use Ecto.Integration.Case, async: true | ||
|
||
import Ecto.Query, only: [from: 2, with_cte: 3] | ||
|
||
alias Ecto.Integration.Comment | ||
alias Ecto.Integration.Post | ||
alias Ecto.Integration.TestRepo | ||
|
||
test "values works with datetime" do | ||
TestRepo.insert!(%Post{inserted_at: ~N[2000-01-01 00:01:00]}) | ||
TestRepo.insert!(%Post{inserted_at: ~N[2000-01-01 00:02:00]}) | ||
TestRepo.insert!(%Post{inserted_at: ~N[2000-01-01 00:03:00]}) | ||
|
||
params = [ | ||
%{id: 1, date: ~N[2000-01-01 00:00:00]}, | ||
%{id: 2, date: ~N[2000-01-01 00:01:00]}, | ||
%{id: 3, date: ~N[2000-01-01 00:02:00]}, | ||
%{id: 4, date: ~N[2000-01-01 00:03:00]} | ||
] | ||
|
||
types = %{id: :integer, date: :naive_datetime} | ||
|
||
results = | ||
from(params in values(params, types), | ||
left_join: p in Post, | ||
on: p.inserted_at <= params.date, | ||
group_by: params.id, | ||
select: %{id: params.id, count: count(p.id)}, | ||
order_by: count(p.id) | ||
) | ||
|> TestRepo.all() | ||
|
||
assert results == [ | ||
%{count: 0, id: 1}, | ||
%{count: 1, id: 2}, | ||
%{count: 2, id: 3}, | ||
%{count: 3, id: 4} | ||
] | ||
end | ||
|
||
test "join to values works" do | ||
TestRepo.insert!(%Post{id: 1}) | ||
TestRepo.insert!(%Comment{post_id: 1, text: "short"}) | ||
TestRepo.insert!(%Comment{post_id: 1, text: "much longer text"}) | ||
|
||
params = [%{id: 1, post_id: 1, n: 0}, %{id: 2, post_id: 1, n: 10}] | ||
types = %{id: :integer, post_id: :integer, n: :integer} | ||
|
||
results = | ||
from(p in Post, | ||
right_join: params in values(params, types), | ||
on: params.post_id == p.id, | ||
left_join: c in Comment, | ||
on: c.post_id == p.id and fragment("LENGTH(?)", c.text) > params.n, | ||
group_by: params.id, | ||
select: {params.id, count(c.id)} | ||
) | ||
|> TestRepo.all() | ||
|
||
assert [{1, 2}, {2, 1}] = results | ||
end | ||
|
||
test "values can be used together with CTE" do | ||
TestRepo.insert!(%Post{id: 1, visits: 42}) | ||
TestRepo.insert!(%Comment{post_id: 1, text: "short"}) | ||
TestRepo.insert!(%Comment{post_id: 1, text: "much longer text"}) | ||
|
||
params = [%{id: 1, post_id: 1, n: 0}, %{id: 2, post_id: 1, n: 10}] | ||
types = %{id: :integer, post_id: :integer, n: :integer} | ||
|
||
cte_query = from(p in Post, select: %{id: p.id, visits: coalesce(p.visits, 0)}) | ||
|
||
q = Post |> with_cte("xxx", as: ^cte_query) | ||
|
||
results = | ||
from(p in q, | ||
right_join: params in values(params, types), | ||
on: params.post_id == p.id, | ||
left_join: c in Comment, | ||
on: c.post_id == p.id and fragment("LENGTH(?)", c.text) > params.n, | ||
left_join: cte in "xxx", | ||
on: cte.id == p.id, | ||
group_by: params.id, | ||
select: {params.id, count(c.id), cte.visits} | ||
) | ||
|> TestRepo.all() | ||
|
||
assert [{1, 2, 42}, {2, 1, 42}] = results | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like
:location
was not added yet in ExUnit v1.15.Any idea how we could get this to work with elixir 1.15?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be acceptable to skip all
:values_list
if running on Elixir v1.15?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No ideas. But when elixir 1.19 is released, I am dropping support for 1.15. I could drop support for it now since I typically only like to support 3 versions back including current. 1.18, 1.17, 1.16 and otp 28, 27, 26
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, in that case I think it makes sense to not run those tests for 1.15. Then you can remove that once you decide to drop support. I've done that in the latest commit