From e66c9c88b94e47f562ec23f2186a8e028bd3abd8 Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Tue, 30 Sep 2025 09:04:48 -0400 Subject: [PATCH 1/3] allow take with tuple fragment sources --- integration_test/cases/repo.exs | 13 +++++++++++++ lib/ecto/query/planner.ex | 13 +++++++++++-- test/ecto/query/planner_test.exs | 29 +++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/integration_test/cases/repo.exs b/integration_test/cases/repo.exs index 4d3c7338e6..47cd13568e 100644 --- a/integration_test/cases/repo.exs +++ b/integration_test/cases/repo.exs @@ -782,6 +782,19 @@ defmodule Ecto.Integration.RepoTest do assert_raise Ecto.NoResultsError, fn -> query |> last |> TestRepo.one! end end + test "fragment source" do + query = from f in {fragment("select 1 as num"), Barebone} + assert %Barebone{num: 1} = TestRepo.one(query) + end + + test "fragment source with take" do + query = from f in {fragment("select 1 as visits"), Post}, select: struct(f, [:visits]) + assert %Post{visits: 1} = TestRepo.one(query) + + query = from f in {fragment("select 1 as visits"), Post}, select: map(f, [:visits]) + assert %{visits: 1} = TestRepo.one(query) + end + test "exists?" do TestRepo.insert!(%Post{title: "1", visits: 2}) TestRepo.insert!(%Post{title: "2", visits: 1}) diff --git a/lib/ecto/query/planner.ex b/lib/ecto/query/planner.ex index c002daa6f6..6cfe8b25a5 100644 --- a/lib/ecto/query/planner.ex +++ b/lib/ecto/query/planner.ex @@ -2220,11 +2220,15 @@ defmodule Ecto.Query.Planner do error!(query, "struct/2 in select expects a source with a schema") {{:ok, {kind, fields}}, {source, schema, prefix}} when is_binary(source) -> - dumper = if schema, do: schema.__schema__(:dump), else: %{} + {types, fields} = select_dump_for_schema(schema, List.wrap(fields), ix, drop) schema = if kind == :map, do: nil, else: schema - {types, fields} = select_dump(List.wrap(fields), dumper, ix, drop) {{:source, {source, schema}, prefix || query.prefix, types}, fields} + {{:ok, {kind, fields}}, {{:fragment, _, _} = source, schema, prefix}} -> + {types, fields} = select_dump_for_schema(schema, List.wrap(fields), ix, drop) + schema = if kind == :map, do: nil, else: schema + {{:source, {source, schema}, prefix, types}, fields} + {{:ok, {_, fields}}, _} -> {{:map, Enum.map(fields, &{&1, {:value, :any}})}, Enum.map(fields, &select_field(&1, ix, :always))} @@ -2258,6 +2262,11 @@ defmodule Ecto.Query.Planner do end end + defp select_dump_for_schema(schema, fields, ix, drop) do + dumper = if schema, do: schema.__schema__(:dump), else: %{} + select_dump(List.wrap(fields), dumper, ix, drop) + end + defp select_dump(fields, dumper, ix, drop) do fields |> Enum.reverse() diff --git a/test/ecto/query/planner_test.exs b/test/ecto/query/planner_test.exs index ba6c773ad1..0d71f489e8 100644 --- a/test/ecto/query/planner_test.exs +++ b/test/ecto/query/planner_test.exs @@ -958,6 +958,23 @@ defmodule Ecto.Query.PlannerTest do assert [:all, {:from, {{:fragment, _, _}, Barebone, _, _}, []}] = cache_key end + test "plan: tuple source with fragment and take" do + {query, cast_params, dump_params, cache_key} = + plan(from f in {fragment("? as text", ^"hi"), Post}, select: struct(f, [:text])) + + assert query.select.take == %{0 => {:struct, [:text]}} + assert {{{:fragment, [], _}, Post, nil}} = query.sources + assert cast_params == ["hi"] + assert dump_params == ["hi"] + + assert [ + :all, + {:take, %{0 => {:struct, [:text]}}}, + {:from, {{:fragment, _, _}, Post, _, _}, []}, + {:select, {:&, [], [0]}} + ] = cache_key + end + describe "plan: CTEs" do test "with uncacheable queries are uncacheable" do {_, _, _, cache} = @@ -2601,6 +2618,18 @@ defmodule Ecto.Query.PlannerTest do assert query.select.fields == [{{:., [writable: :always], [{:&, [], [0]}, :num]}, [], []}] end + test "normalize: tuple source with fragment and take" do + {query, _, _, select} = + normalize_with_params( + from f in {fragment("? as text", ^"hi"), Post}, select: struct(f, [:text]) + ) + + %{from: {_, {:source, {{:fragment, _, _}, Post}, nil, types}}} = select + assert types == [text: :string] + assert {{:fragment, _, _}, Post} = query.from.source + assert query.select.fields == [{{:., [writable: :always], [{:&, [], [0]}, :text]}, [], []}] + end + describe "normalize: subqueries in boolean expressions" do test "replaces {:subquery, index} with an Ecto.SubQuery struct" do subquery = from(p in Post, select: p.visits) From 786c9edcce46f432f3ec70033fdf2a46f6b69cdd Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Tue, 30 Sep 2025 09:06:49 -0400 Subject: [PATCH 2/3] make test name clearer --- integration_test/cases/repo.exs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration_test/cases/repo.exs b/integration_test/cases/repo.exs index 47cd13568e..1ae309ecd9 100644 --- a/integration_test/cases/repo.exs +++ b/integration_test/cases/repo.exs @@ -782,12 +782,12 @@ defmodule Ecto.Integration.RepoTest do assert_raise Ecto.NoResultsError, fn -> query |> last |> TestRepo.one! end end - test "fragment source" do + test "fragment source mapped to schema" do query = from f in {fragment("select 1 as num"), Barebone} assert %Barebone{num: 1} = TestRepo.one(query) end - test "fragment source with take" do + test "fragment source mapped to schema with take" do query = from f in {fragment("select 1 as visits"), Post}, select: struct(f, [:visits]) assert %Post{visits: 1} = TestRepo.one(query) From fe6297c200ee9e1b24fb996e3cbe81d099e6c368 Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Tue, 30 Sep 2025 09:08:20 -0400 Subject: [PATCH 3/3] improve test --- integration_test/cases/repo.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration_test/cases/repo.exs b/integration_test/cases/repo.exs index 1ae309ecd9..50722c5492 100644 --- a/integration_test/cases/repo.exs +++ b/integration_test/cases/repo.exs @@ -792,7 +792,7 @@ defmodule Ecto.Integration.RepoTest do assert %Post{visits: 1} = TestRepo.one(query) query = from f in {fragment("select 1 as visits"), Post}, select: map(f, [:visits]) - assert %{visits: 1} = TestRepo.one(query) + assert TestRepo.one(query) == %{visits: 1} end test "exists?" do