From cc857f4ec275209a365d16fef20d7a0649deef57 Mon Sep 17 00:00:00 2001 From: Aliaksandr Sasnouski Date: Wed, 3 Jun 2026 15:04:26 +0200 Subject: [PATCH 1/3] Add Ecto.Query.label/2 query expression for tagging queries with an SQL comment --- lib/ecto/query.ex | 41 +++++++++++- lib/ecto/query/builder/label.ex | 82 +++++++++++++++++++++++ lib/ecto/query/inspect.ex | 2 + lib/ecto/query/planner.ex | 6 +- lib/ecto/repo.ex | 21 ++++++ test/ecto/query/builder/label_test.exs | 93 ++++++++++++++++++++++++++ 6 files changed, 241 insertions(+), 4 deletions(-) create mode 100644 lib/ecto/query/builder/label.ex create mode 100644 test/ecto/query/builder/label_test.exs diff --git a/lib/ecto/query.ex b/lib/ecto/query.ex index 110331b23f..849ca16c1c 100644 --- a/lib/ecto/query.ex +++ b/lib/ecto/query.ex @@ -414,7 +414,8 @@ defmodule Ecto.Query do distinct: nil, lock: nil, windows: [], - with_ctes: nil + with_ctes: nil, + label: nil defmodule FromExpr do @moduledoc false @@ -955,6 +956,7 @@ defmodule Ecto.Query do Ecto.Query.exclude(query, :limit) Ecto.Query.exclude(query, :offset) Ecto.Query.exclude(query, :lock) + Ecto.Query.exclude(query, :label) Ecto.Query.exclude(query, :preload) Ecto.Query.exclude(query, :update) Ecto.Query.exclude(query, :windows) @@ -1024,6 +1026,7 @@ defmodule Ecto.Query do defp do_exclude(%Ecto.Query{} = query, :limit), do: %{query | limit: nil} defp do_exclude(%Ecto.Query{} = query, :offset), do: %{query | offset: nil} defp do_exclude(%Ecto.Query{} = query, :lock), do: %{query | lock: nil} + defp do_exclude(%Ecto.Query{} = query, :label), do: %{query | label: nil} defp do_exclude(%Ecto.Query{} = query, :preload), do: %{query | preloads: [], assocs: []} defp do_exclude(%Ecto.Query{} = query, :update), do: %{query | updates: []} defp do_exclude(%Ecto.Query{} = query, :windows), do: %{query | windows: []} @@ -1150,7 +1153,7 @@ defmodule Ecto.Query do end @from_join_opts [:as, :prefix, :hints] - @no_binds [:union, :union_all, :except, :except_all, :intersect, :intersect_all] + @no_binds [:union, :union_all, :except, :except_all, :intersect, :intersect_all, :label] @binds [:lock, :where, :or_where, :select, :distinct, :order_by, :group_by, :windows] ++ [:having, :or_having, :limit, :offset, :preload, :update, :select_merge, :with_ctes] @@ -2537,6 +2540,40 @@ defmodule Ecto.Query do Builder.Lock.build(query, binding, expr, __CALLER__) end + @doc ~S""" + A label query expression. + + Adds the given text to the generated statement as a leading SQL comment, + immediately before the statement keyword: + + /* get_username_q */ SELECT ... + + This is useful to tag and identify queries in database logs and monitoring + tools. The label is rendered *leading* (rather than trailing) so it survives + truncation of long statements in logs. + + Because the label becomes part of the generated SQL, it is also part of the + query cache key. Avoid highly dynamic values (such as per-request ids) as they + would defeat Ecto's prepared-statement caching; prefer a stable identifier per + call site. + + ## Keywords example + + from(p in Post, label: "get_post_titles_q", select: p.title) + + ## Expressions example + + Post |> label("get_post_titles_q") |> select([p], p.title) + + ## Interpolation + + report = "get_posts_q" + from(p in Post, label: ^report) + """ + defmacro label(query, expr) do + Builder.Label.build(query, expr, __CALLER__) + end + @doc ~S""" An update query expression. diff --git a/lib/ecto/query/builder/label.ex b/lib/ecto/query/builder/label.ex new file mode 100644 index 0000000000..38089311d5 --- /dev/null +++ b/lib/ecto/query/builder/label.ex @@ -0,0 +1,82 @@ +import Kernel, except: [apply: 2] + +defmodule Ecto.Query.Builder.Label do + @moduledoc false + + alias Ecto.Query.Builder + + @forbidden ["*/", "/*", <<0>>] + + @doc """ + Escapes the label text. + + iex> escape(quote(do: "my-query")) + "my-query" + + """ + @spec escape(Macro.t()) :: Macro.t() + def escape(label) when is_binary(label) do + if String.contains?(label, @forbidden) do + Builder.error!(forbidden_message(label)) + end + + label + end + + def escape({:^, _, [expr]}) do + quote do + Ecto.Query.Builder.Label.runtime!(unquote(expr)) + end + end + + def escape(other) do + Builder.error!( + "`#{Macro.to_string(other)}` is not a valid label. " <> + "For security reasons, a label must be a literal string or an interpolated string" + ) + end + + @doc """ + Validates a label given at runtime via interpolation. + """ + @spec runtime!(term) :: String.t() + def runtime!(label) when is_binary(label) do + if String.contains?(label, @forbidden) do + raise ArgumentError, forbidden_message(label) + end + + label + end + + def runtime!(other) do + raise ArgumentError, "a label must be a string, got: `#{inspect(other)}`" + end + + defp forbidden_message(label) do + "a label cannot contain `/*`, `*/`, or null bytes, got: `#{inspect(label)}`. " + end + + @doc """ + Builds a quoted expression. + + The quoted expression should evaluate to a query at runtime. + If possible, it does all calculations at compile time to avoid + runtime work. + """ + @spec build(Macro.t(), Macro.t(), Macro.Env.t()) :: Macro.t() + def build(query, expr, env) do + Builder.apply_query(query, __MODULE__, [escape(expr)], env) + end + + @doc """ + The callback applied by `build/3` to build the query. + """ + @spec apply(Ecto.Queryable.t(), term) :: Ecto.Query.t() + def apply(%Ecto.Query{} = query, value) do + %{query | label: value} + end + + def apply(query, value) do + apply(Ecto.Queryable.to_query(query), value) + end +end diff --git a/lib/ecto/query/inspect.ex b/lib/ecto/query/inspect.ex index d5427ff0f0..ab2c8e5a1e 100644 --- a/lib/ecto/query/inspect.ex +++ b/lib/ecto/query/inspect.ex @@ -124,6 +124,7 @@ defimpl Inspect, for: Ecto.Query do updates = kw_exprs(:update, query.updates, names) lock = kw_inspect(:lock, query.lock) + label = kw_inspect(:label, query.label) offset = kw_expr(:offset, query.offset, names) select = kw_expr(:select, query.select, names) distinct = kw_expr(:distinct, query.distinct, names) @@ -140,6 +141,7 @@ defimpl Inspect, for: Ecto.Query do limit, offset, lock, + label, distinct, updates, select, diff --git a/lib/ecto/query/planner.ex b/lib/ecto/query/planner.ex index 55a437fbcb..4ce14f2157 100644 --- a/lib/ecto/query/planner.ex +++ b/lib/ecto/query/planner.ex @@ -13,7 +13,7 @@ defmodule Ecto.Query.Planner do LimitExpr } - if map_size(%Ecto.Query{}) != 21 do + if map_size(%Ecto.Query{}) != 22 do raise "Ecto.Query match out of date in builder" end @@ -1073,7 +1073,8 @@ defmodule Ecto.Query.Planner do end defp finalize_cache(query, operation, cache) do - %{assocs: assocs, prefix: prefix, lock: lock, select: select, aliases: aliases} = query + %{assocs: assocs, prefix: prefix, lock: lock, label: label, select: select, aliases: aliases} = + query aliases = Map.delete(aliases, @parent_as) cache = @@ -1090,6 +1091,7 @@ defmodule Ecto.Query.Planner do |> prepend_if(assocs != [], assocs: assocs) |> prepend_if(prefix != nil, prefix: prefix) |> prepend_if(lock != nil, lock: lock) + |> prepend_if(label != nil, label: label) |> prepend_if(aliases != %{}, aliases: aliases) [operation | cache] diff --git a/lib/ecto/repo.ex b/lib/ecto/repo.ex index c99d5fabca..6a7e94bafc 100644 --- a/lib/ecto/repo.ex +++ b/lib/ecto/repo.ex @@ -1581,6 +1581,13 @@ defmodule Ecto.Repo do in the schema. For more information see the ["Query Prefix"](`m:Ecto.Query#module-query-prefix`) section of the `Ecto.Query` documentation. + * `:label` - A string to tag the generated `UPDATE` with a leading SQL + comment, such as `/* bump_visits */ UPDATE ...`, to identify the statement + in database logs and monitoring tools. The string is embedded verbatim into + the SQL and therefore cannot contain `/*`, `*/`, or null bytes. Prefer a + stable identifier per call site, as the label becomes part of the query + cache key. See `Ecto.Query.label/2` to set it on the query itself instead. + See the ["Shared options"](#module-shared-options) section at the module documentation for remaining options. @@ -1630,6 +1637,13 @@ defmodule Ecto.Repo do in the schema. For more information see the ["Query Prefix"](`m:Ecto.Query#module-query-prefix`) section of the `Ecto.Query` documentation. + * `:label` - A string to tag the generated `DELETE` with a leading SQL + comment, such as `/* purge_stale */ DELETE ...`, to identify the statement + in database logs and monitoring tools. The string is embedded verbatim into + the SQL and therefore cannot contain `/*`, `*/`, or null bytes. Prefer a + stable identifier per call site, as the label becomes part of the query + cache key. See `Ecto.Query.label/2` to set it on the query itself instead. + See the ["Shared options"](#module-shared-options) section at the module documentation for remaining options. @@ -1732,6 +1746,13 @@ defmodule Ecto.Repo do * `:placeholders` - A map with placeholders. This feature is not supported by all databases. See the ["Placeholders" section](#c:insert_all/3-placeholders) for more information. + * `:label` - A string to tag the generated `INSERT` with a leading SQL + comment, such as `/* import_users */ INSERT ...`, to identify the statement + in database logs and monitoring tools. The string is embedded verbatim into + the SQL and therefore cannot contain `/*`, `*/`, or null bytes. Prefer a + stable identifier per call site, as the label becomes part of the query + cache key. + See the ["Shared options"](#module-shared-options) section at the module documentation for remaining options. diff --git a/test/ecto/query/builder/label_test.exs b/test/ecto/query/builder/label_test.exs new file mode 100644 index 0000000000..c93fbf77c2 --- /dev/null +++ b/test/ecto/query/builder/label_test.exs @@ -0,0 +1,93 @@ +Code.require_file "../../../support/eval_helpers.exs", __DIR__ + +defmodule Ecto.Query.Builder.LabelTest do + use ExUnit.Case, async: true + + import Ecto.Query.Builder.Label + doctest Ecto.Query.Builder.Label + + import Ecto.Query + import Support.EvalHelpers + + test "label with literal string" do + query = %Ecto.Query{} |> label("my-report") + assert query.label == "my-report" + end + + test "label via keyword syntax" do + query = from p in "posts", label: "list-posts" + assert query.label == "list-posts" + end + + test "label with interpolated string" do + report = "monthly-report" + query = %Ecto.Query{} |> label(^report) + assert query.label == "monthly-report" + end + + test "overrides on duplicated label" do + query = %Ecto.Query{} |> label("FOO") |> label("BAR") + assert query.label == "BAR" + end + + test "raises on non-string at compile time" do + assert_raise Ecto.Query.CompileError, ~r"`1` is not a valid label", fn -> + quote_and_eval(%Ecto.Query{} |> label(1)) + end + end + + test "raises on literal containing */" do + assert_raise Ecto.Query.CompileError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> + quote_and_eval(%Ecto.Query{} |> label("evil */ DROP TABLE")) + end + end + + test "raises on literal containing /*" do + assert_raise Ecto.Query.CompileError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> + quote_and_eval(%Ecto.Query{} |> label("evil /* nested")) + end + end + + test "raises on literal containing a null byte" do + assert_raise Ecto.Query.CompileError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> + quote_and_eval(%Ecto.Query{} |> label("nul\0byte")) + end + end + + test "raises on interpolated value containing a null byte" do + evil = "nul\0byte" + + assert_raise ArgumentError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> + %Ecto.Query{} |> label(^evil) + end + end + + test "raises on interpolated value containing */" do + evil = "evil */ DROP TABLE" + + assert_raise ArgumentError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> + %Ecto.Query{} |> label(^evil) + end + end + + test "raises on interpolated value containing /*" do + evil = "evil /* nested" + + assert_raise ArgumentError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> + %Ecto.Query{} |> label(^evil) + end + end + + test "raises on interpolated non-string" do + not_a_string = 123 + + assert_raise ArgumentError, ~r"must be a string", fn -> + %Ecto.Query{} |> label(^not_a_string) + end + end + + test "exclude resets the label" do + query = %Ecto.Query{} |> label("FOO") + assert Ecto.Query.exclude(query, :label).label == nil + end +end From 76da993fc74ba4d29d9c0f19e1f36c7e86ced03f Mon Sep 17 00:00:00 2001 From: Aliaksandr Sasnouski Date: Thu, 4 Jun 2026 11:10:00 +0200 Subject: [PATCH 2/3] Make :label a repo option instead of a query expression. --- lib/ecto/query.ex | 37 +--------- lib/ecto/query/builder/label.ex | 82 ----------------------- lib/ecto/query/inspect.ex | 2 - lib/ecto/query/planner.ex | 30 +++++++++ lib/ecto/repo.ex | 27 ++------ lib/ecto/repo/queryable.ex | 6 +- test/ecto/query/builder/label_test.exs | 93 -------------------------- test/ecto/query/planner_test.exs | 46 +++++++++++++ 8 files changed, 86 insertions(+), 237 deletions(-) delete mode 100644 lib/ecto/query/builder/label.ex delete mode 100644 test/ecto/query/builder/label_test.exs diff --git a/lib/ecto/query.ex b/lib/ecto/query.ex index 849ca16c1c..933080f2a5 100644 --- a/lib/ecto/query.ex +++ b/lib/ecto/query.ex @@ -956,7 +956,6 @@ defmodule Ecto.Query do Ecto.Query.exclude(query, :limit) Ecto.Query.exclude(query, :offset) Ecto.Query.exclude(query, :lock) - Ecto.Query.exclude(query, :label) Ecto.Query.exclude(query, :preload) Ecto.Query.exclude(query, :update) Ecto.Query.exclude(query, :windows) @@ -1026,7 +1025,6 @@ defmodule Ecto.Query do defp do_exclude(%Ecto.Query{} = query, :limit), do: %{query | limit: nil} defp do_exclude(%Ecto.Query{} = query, :offset), do: %{query | offset: nil} defp do_exclude(%Ecto.Query{} = query, :lock), do: %{query | lock: nil} - defp do_exclude(%Ecto.Query{} = query, :label), do: %{query | label: nil} defp do_exclude(%Ecto.Query{} = query, :preload), do: %{query | preloads: [], assocs: []} defp do_exclude(%Ecto.Query{} = query, :update), do: %{query | updates: []} defp do_exclude(%Ecto.Query{} = query, :windows), do: %{query | windows: []} @@ -1153,7 +1151,7 @@ defmodule Ecto.Query do end @from_join_opts [:as, :prefix, :hints] - @no_binds [:union, :union_all, :except, :except_all, :intersect, :intersect_all, :label] + @no_binds [:union, :union_all, :except, :except_all, :intersect, :intersect_all] @binds [:lock, :where, :or_where, :select, :distinct, :order_by, :group_by, :windows] ++ [:having, :or_having, :limit, :offset, :preload, :update, :select_merge, :with_ctes] @@ -2540,39 +2538,6 @@ defmodule Ecto.Query do Builder.Lock.build(query, binding, expr, __CALLER__) end - @doc ~S""" - A label query expression. - - Adds the given text to the generated statement as a leading SQL comment, - immediately before the statement keyword: - - /* get_username_q */ SELECT ... - - This is useful to tag and identify queries in database logs and monitoring - tools. The label is rendered *leading* (rather than trailing) so it survives - truncation of long statements in logs. - - Because the label becomes part of the generated SQL, it is also part of the - query cache key. Avoid highly dynamic values (such as per-request ids) as they - would defeat Ecto's prepared-statement caching; prefer a stable identifier per - call site. - - ## Keywords example - - from(p in Post, label: "get_post_titles_q", select: p.title) - - ## Expressions example - - Post |> label("get_post_titles_q") |> select([p], p.title) - - ## Interpolation - - report = "get_posts_q" - from(p in Post, label: ^report) - """ - defmacro label(query, expr) do - Builder.Label.build(query, expr, __CALLER__) - end @doc ~S""" An update query expression. diff --git a/lib/ecto/query/builder/label.ex b/lib/ecto/query/builder/label.ex deleted file mode 100644 index 38089311d5..0000000000 --- a/lib/ecto/query/builder/label.ex +++ /dev/null @@ -1,82 +0,0 @@ -import Kernel, except: [apply: 2] - -defmodule Ecto.Query.Builder.Label do - @moduledoc false - - alias Ecto.Query.Builder - - @forbidden ["*/", "/*", <<0>>] - - @doc """ - Escapes the label text. - - iex> escape(quote(do: "my-query")) - "my-query" - - """ - @spec escape(Macro.t()) :: Macro.t() - def escape(label) when is_binary(label) do - if String.contains?(label, @forbidden) do - Builder.error!(forbidden_message(label)) - end - - label - end - - def escape({:^, _, [expr]}) do - quote do - Ecto.Query.Builder.Label.runtime!(unquote(expr)) - end - end - - def escape(other) do - Builder.error!( - "`#{Macro.to_string(other)}` is not a valid label. " <> - "For security reasons, a label must be a literal string or an interpolated string" - ) - end - - @doc """ - Validates a label given at runtime via interpolation. - """ - @spec runtime!(term) :: String.t() - def runtime!(label) when is_binary(label) do - if String.contains?(label, @forbidden) do - raise ArgumentError, forbidden_message(label) - end - - label - end - - def runtime!(other) do - raise ArgumentError, "a label must be a string, got: `#{inspect(other)}`" - end - - defp forbidden_message(label) do - "a label cannot contain `/*`, `*/`, or null bytes, got: `#{inspect(label)}`. " - end - - @doc """ - Builds a quoted expression. - - The quoted expression should evaluate to a query at runtime. - If possible, it does all calculations at compile time to avoid - runtime work. - """ - @spec build(Macro.t(), Macro.t(), Macro.Env.t()) :: Macro.t() - def build(query, expr, env) do - Builder.apply_query(query, __MODULE__, [escape(expr)], env) - end - - @doc """ - The callback applied by `build/3` to build the query. - """ - @spec apply(Ecto.Queryable.t(), term) :: Ecto.Query.t() - def apply(%Ecto.Query{} = query, value) do - %{query | label: value} - end - - def apply(query, value) do - apply(Ecto.Queryable.to_query(query), value) - end -end diff --git a/lib/ecto/query/inspect.ex b/lib/ecto/query/inspect.ex index ab2c8e5a1e..d5427ff0f0 100644 --- a/lib/ecto/query/inspect.ex +++ b/lib/ecto/query/inspect.ex @@ -124,7 +124,6 @@ defimpl Inspect, for: Ecto.Query do updates = kw_exprs(:update, query.updates, names) lock = kw_inspect(:lock, query.lock) - label = kw_inspect(:label, query.label) offset = kw_expr(:offset, query.offset, names) select = kw_expr(:select, query.select, names) distinct = kw_expr(:distinct, query.distinct, names) @@ -141,7 +140,6 @@ defimpl Inspect, for: Ecto.Query do limit, offset, lock, - label, distinct, updates, select, diff --git a/lib/ecto/query/planner.ex b/lib/ecto/query/planner.ex index 4ce14f2157..45b0a9dcdc 100644 --- a/lib/ecto/query/planner.ex +++ b/lib/ecto/query/planner.ex @@ -2383,6 +2383,36 @@ defmodule Ecto.Query.Planner do def attach_prefix(query, _), do: query + @doc """ + Puts the label given via `opts` into the given query, if available. + + The label is rendered by the adapter as a leading `/* ... */` SQL comment + and becomes part of the query cache key. It is embedded verbatim, so it may + not contain `/*`, `*/`, or null bytes. + """ + def attach_label(%{label: nil} = query, opts) when is_list(opts) do + case Keyword.fetch(opts, :label) do + {:ok, label} -> %{query | label: validate_label!(label)} + :error -> query + end + end + + def attach_label(query, _), do: query + + defp validate_label!(label) when is_binary(label) do + if String.contains?(label, ["/*", "*/", <<0>>]) do + raise ArgumentError, + "a label cannot contain `/*`, `*/`, or null bytes, got: #{inspect(label)}. " <> + "Allowing them would let the label break out of the surrounding `/* */` comment" + end + + label + end + + defp validate_label!(other) do + raise ArgumentError, "a label must be a string, got: #{inspect(other)}" + end + ## Helpers @all_exprs [ diff --git a/lib/ecto/repo.ex b/lib/ecto/repo.ex index 6a7e94bafc..fa6563f59a 100644 --- a/lib/ecto/repo.ex +++ b/lib/ecto/repo.ex @@ -141,6 +141,12 @@ defmodule Ecto.Repo do in the cache and no cache update function will not be passed to the adapter. Note that this doesn't necessarily disable the database cache, it only affects Ecto's internal cache of normalized queries and adapter prepared statements. Defaults to `true`. + * `:label` - A string embedded into the generated statement as a leading SQL comment, + such as `/* import_users */ INSERT ...`, to identify the query in database logs and + monitoring tools. It is rendered leading (rather than trailing) so it survives truncation + of long statements in logs. The string is embedded verbatim and therefore cannot contain + `/*`, `*/`, or null bytes. The label becomes part of the query cache key, so prefer a + stable identifier per call site over highly dynamic values such as per-request ids. ## Adapter-Specific Errors @@ -1581,13 +1587,6 @@ defmodule Ecto.Repo do in the schema. For more information see the ["Query Prefix"](`m:Ecto.Query#module-query-prefix`) section of the `Ecto.Query` documentation. - * `:label` - A string to tag the generated `UPDATE` with a leading SQL - comment, such as `/* bump_visits */ UPDATE ...`, to identify the statement - in database logs and monitoring tools. The string is embedded verbatim into - the SQL and therefore cannot contain `/*`, `*/`, or null bytes. Prefer a - stable identifier per call site, as the label becomes part of the query - cache key. See `Ecto.Query.label/2` to set it on the query itself instead. - See the ["Shared options"](#module-shared-options) section at the module documentation for remaining options. @@ -1637,13 +1636,6 @@ defmodule Ecto.Repo do in the schema. For more information see the ["Query Prefix"](`m:Ecto.Query#module-query-prefix`) section of the `Ecto.Query` documentation. - * `:label` - A string to tag the generated `DELETE` with a leading SQL - comment, such as `/* purge_stale */ DELETE ...`, to identify the statement - in database logs and monitoring tools. The string is embedded verbatim into - the SQL and therefore cannot contain `/*`, `*/`, or null bytes. Prefer a - stable identifier per call site, as the label becomes part of the query - cache key. See `Ecto.Query.label/2` to set it on the query itself instead. - See the ["Shared options"](#module-shared-options) section at the module documentation for remaining options. @@ -1746,13 +1738,6 @@ defmodule Ecto.Repo do * `:placeholders` - A map with placeholders. This feature is not supported by all databases. See the ["Placeholders" section](#c:insert_all/3-placeholders) for more information. - * `:label` - A string to tag the generated `INSERT` with a leading SQL - comment, such as `/* import_users */ INSERT ...`, to identify the statement - in database logs and monitoring tools. The string is embedded verbatim into - the SQL and therefore cannot contain `/*`, `*/`, or null bytes. Prefer a - stable identifier per call site, as the label becomes part of the query - cache key. - See the ["Shared options"](#module-shared-options) section at the module documentation for remaining options. diff --git a/lib/ecto/repo/queryable.ex b/lib/ecto/repo/queryable.ex index dc462a6a60..56c70a2642 100644 --- a/lib/ecto/repo/queryable.ex +++ b/lib/ecto/repo/queryable.ex @@ -6,7 +6,7 @@ defmodule Ecto.Repo.Queryable do alias Ecto.Query.Planner alias Ecto.Query.SelectExpr - import Ecto.Query.Planner, only: [attach_prefix: 2] + import Ecto.Query.Planner, only: [attach_prefix: 2, attach_label: 2] require Ecto.Query @@ -37,7 +37,7 @@ defmodule Ecto.Repo.Queryable do |> Ecto.Query.Planner.ensure_select(true) {query, opts} = repo.prepare_query(:stream, query, opts) - query = attach_prefix(query, opts) + query = query |> attach_prefix(opts) |> attach_label(opts) query_cache? = Keyword.get(opts, :query_cache, true) @@ -222,7 +222,7 @@ defmodule Ecto.Repo.Queryable do %{adapter: adapter, cache: cache, repo: repo} = adapter_meta {query, opts} = repo.prepare_query(operation, query, opts) - query = attach_prefix(query, opts) + query = query |> attach_prefix(opts) |> attach_label(opts) query_cache? = Keyword.get(opts, :query_cache, true) diff --git a/test/ecto/query/builder/label_test.exs b/test/ecto/query/builder/label_test.exs deleted file mode 100644 index c93fbf77c2..0000000000 --- a/test/ecto/query/builder/label_test.exs +++ /dev/null @@ -1,93 +0,0 @@ -Code.require_file "../../../support/eval_helpers.exs", __DIR__ - -defmodule Ecto.Query.Builder.LabelTest do - use ExUnit.Case, async: true - - import Ecto.Query.Builder.Label - doctest Ecto.Query.Builder.Label - - import Ecto.Query - import Support.EvalHelpers - - test "label with literal string" do - query = %Ecto.Query{} |> label("my-report") - assert query.label == "my-report" - end - - test "label via keyword syntax" do - query = from p in "posts", label: "list-posts" - assert query.label == "list-posts" - end - - test "label with interpolated string" do - report = "monthly-report" - query = %Ecto.Query{} |> label(^report) - assert query.label == "monthly-report" - end - - test "overrides on duplicated label" do - query = %Ecto.Query{} |> label("FOO") |> label("BAR") - assert query.label == "BAR" - end - - test "raises on non-string at compile time" do - assert_raise Ecto.Query.CompileError, ~r"`1` is not a valid label", fn -> - quote_and_eval(%Ecto.Query{} |> label(1)) - end - end - - test "raises on literal containing */" do - assert_raise Ecto.Query.CompileError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> - quote_and_eval(%Ecto.Query{} |> label("evil */ DROP TABLE")) - end - end - - test "raises on literal containing /*" do - assert_raise Ecto.Query.CompileError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> - quote_and_eval(%Ecto.Query{} |> label("evil /* nested")) - end - end - - test "raises on literal containing a null byte" do - assert_raise Ecto.Query.CompileError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> - quote_and_eval(%Ecto.Query{} |> label("nul\0byte")) - end - end - - test "raises on interpolated value containing a null byte" do - evil = "nul\0byte" - - assert_raise ArgumentError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> - %Ecto.Query{} |> label(^evil) - end - end - - test "raises on interpolated value containing */" do - evil = "evil */ DROP TABLE" - - assert_raise ArgumentError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> - %Ecto.Query{} |> label(^evil) - end - end - - test "raises on interpolated value containing /*" do - evil = "evil /* nested" - - assert_raise ArgumentError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> - %Ecto.Query{} |> label(^evil) - end - end - - test "raises on interpolated non-string" do - not_a_string = 123 - - assert_raise ArgumentError, ~r"must be a string", fn -> - %Ecto.Query{} |> label(^not_a_string) - end - end - - test "exclude resets the label" do - query = %Ecto.Query{} |> label("FOO") - assert Ecto.Query.exclude(query, :label).label == nil - end -end diff --git a/test/ecto/query/planner_test.exs b/test/ecto/query/planner_test.exs index 93be9b9eab..eef8a2fa0a 100644 --- a/test/ecto/query/planner_test.exs +++ b/test/ecto/query/planner_test.exs @@ -2907,4 +2907,50 @@ defmodule Ecto.Query.PlannerTest do assert :ets.info(cache, :size) == 0 end end + + describe "attach_label/2" do + test "sets the label from opts" do + query = Planner.attach_label(from(p in "posts"), label: "get_posts_q") + assert query.label == "get_posts_q" + end + + test "leaves the query unchanged when no label is given" do + query = from(p in "posts") + assert Planner.attach_label(query, []).label == nil + end + + test "is part of the query cache key" do + cache = Planner.new_query_cache(__MODULE__) + query = from(p in Post, where: p.title == ^"hello") + + cache_query = fn label -> + query + |> Planner.attach_label(label: label) + |> Planner.query(:all, cache, Ecto.CachingTestAdapter, 0, true) + end + + # Different labels produce distinct cache entries... + cache_query.("a") + cache_query.("b") + assert :ets.info(cache, :size) == 2 + + # ...while the same label reuses one. + cache_query.("a") + assert :ets.info(cache, :size) == 2 + end + + test "raises on a non-string label" do + assert_raise ArgumentError, ~r/must be a string/, fn -> + Planner.attach_label(from(p in "posts"), label: 123) + end + end + + test "raises on a label that could break out of the comment" do + for bad <- ["a */ b", "a /* b", "a\0b"] do + assert_raise ArgumentError, ~r/cannot contain/, fn -> + Planner.attach_label(from(p in "posts"), label: bad) + end + end + end + end end From dc5c18c19103b4427f3383edcd05d73b37dd7c20 Mon Sep 17 00:00:00 2001 From: Aliaksandr Sasnouski Date: Tue, 16 Jun 2026 15:37:23 +0200 Subject: [PATCH 3/3] Support pre_comment and post_comment options in ecto. --- lib/ecto/query.ex | 57 ++++++++++++++++++++- lib/ecto/query/builder/comment.ex | 63 ++++++++++++++++++++++++ lib/ecto/query/inspect.ex | 6 +++ lib/ecto/query/planner.ex | 37 +++++--------- lib/ecto/repo.ex | 17 ++++--- lib/ecto/repo/queryable.ex | 6 +-- test/ecto/query/builder/comment_test.exs | 62 +++++++++++++++++++++++ test/ecto/query/planner_test.exs | 57 ++++++++++++++------- 8 files changed, 251 insertions(+), 54 deletions(-) create mode 100644 lib/ecto/query/builder/comment.ex create mode 100644 test/ecto/query/builder/comment_test.exs diff --git a/lib/ecto/query.ex b/lib/ecto/query.ex index 933080f2a5..a71227f7bd 100644 --- a/lib/ecto/query.ex +++ b/lib/ecto/query.ex @@ -415,7 +415,7 @@ defmodule Ecto.Query do lock: nil, windows: [], with_ctes: nil, - label: nil + comments: [] defmodule FromExpr do @moduledoc false @@ -956,6 +956,7 @@ defmodule Ecto.Query do Ecto.Query.exclude(query, :limit) Ecto.Query.exclude(query, :offset) Ecto.Query.exclude(query, :lock) + Ecto.Query.exclude(query, :comments) Ecto.Query.exclude(query, :preload) Ecto.Query.exclude(query, :update) Ecto.Query.exclude(query, :windows) @@ -1025,6 +1026,7 @@ defmodule Ecto.Query do defp do_exclude(%Ecto.Query{} = query, :limit), do: %{query | limit: nil} defp do_exclude(%Ecto.Query{} = query, :offset), do: %{query | offset: nil} defp do_exclude(%Ecto.Query{} = query, :lock), do: %{query | lock: nil} + defp do_exclude(%Ecto.Query{} = query, :comments), do: %{query | comments: []} defp do_exclude(%Ecto.Query{} = query, :preload), do: %{query | preloads: [], assocs: []} defp do_exclude(%Ecto.Query{} = query, :update), do: %{query | updates: []} defp do_exclude(%Ecto.Query{} = query, :windows), do: %{query | windows: []} @@ -1151,7 +1153,8 @@ defmodule Ecto.Query do end @from_join_opts [:as, :prefix, :hints] - @no_binds [:union, :union_all, :except, :except_all, :intersect, :intersect_all] + @no_binds [:union, :union_all, :except, :except_all, :intersect, :intersect_all] ++ + [:pre_comment, :post_comment] @binds [:lock, :where, :or_where, :select, :distinct, :order_by, :group_by, :windows] ++ [:having, :or_having, :limit, :offset, :preload, :update, :select_merge, :with_ctes] @@ -2538,6 +2541,56 @@ defmodule Ecto.Query do Builder.Lock.build(query, binding, expr, __CALLER__) end + @doc ~S""" + Adds a comment *before* the generated statement. + + The text is rendered as a leading SQL comment, immediately before the + statement keyword: + + /* list_users */ SELECT ... + + This is useful to tag and identify queries in database logs and monitoring + tools. A leading comment (rather than a trailing one, see `post_comment/2`) + survives truncation of long statements in logs. + + The comment must be a compile-time literal string, so the set of comments for + a given query stays bounded and the query remains safely cacheable. It is + embedded verbatim and therefore cannot contain `/*`, `*/`, or null bytes. For + dynamic comments, use the `:comments` option on the repository operation + instead. Multiple comments may be added and are rendered in order. + + ## Keywords example + + from(p in Post, pre_comment: "list_posts", select: p.title) + + ## Expressions example + + Post |> pre_comment("list_posts") |> select([p], p.title) + """ + defmacro pre_comment(query, expr) do + Builder.Comment.build(:pre, query, expr, __CALLER__) + end + + @doc ~S""" + Adds a comment *after* the generated statement. + + Works like `pre_comment/2` but renders the text as a trailing SQL comment, + after the statement (the SQLCommenter convention): + + SELECT ... /* list_users */ + + ## Keywords example + + from(p in Post, post_comment: "list_posts", select: p.title) + + ## Expressions example + + Post |> post_comment("list_posts") |> select([p], p.title) + """ + defmacro post_comment(query, expr) do + Builder.Comment.build(:post, query, expr, __CALLER__) + end + @doc ~S""" An update query expression. diff --git a/lib/ecto/query/builder/comment.ex b/lib/ecto/query/builder/comment.ex new file mode 100644 index 0000000000..3d52362b15 --- /dev/null +++ b/lib/ecto/query/builder/comment.ex @@ -0,0 +1,63 @@ +import Kernel, except: [apply: 3] + +defmodule Ecto.Query.Builder.Comment do + @moduledoc false + + alias Ecto.Query.Builder + + # A comment is rendered verbatim inside a `/* ... */` SQL comment, so the only + # way for its text to escape into executable SQL is to manipulate the comment + # delimiters. Reject `*/` (closes the block early) and `/*` (opens a nested + # block that, where comments nest, swallows the closing `*/`), plus null bytes. + @forbidden ["*/", "/*", <<0>>] + + @doc """ + Escapes the comment text. + + iex> escape("my-query") + "my-query" + + """ + @spec escape(Macro.t()) :: Macro.t() + def escape(comment) when is_binary(comment) do + if String.contains?(comment, @forbidden) do + Builder.error!( + "a comment cannot contain `/*`, `*/`, or null bytes, got: `#{comment}`. " + ) + end + + comment + end + + def escape({:^, _, [_]}) do + Builder.error!( + "interpolation is not allowed in a query comment. " <> + "Comments must be compile-time literal strings so they stay a bounded set " <> + "and remain safe to cache. For dynamic comments use the `:comments` repo option" + ) + end + + def escape(other) do + Builder.error!("`#{Macro.to_string(other)}` is not a valid comment, it must be a literal string") + end + + @doc """ + Builds a quoted expression that appends a `{position, comment}` to the query. + """ + @spec build(:pre | :post, Macro.t(), Macro.t(), Macro.Env.t()) :: Macro.t() + def build(position, query, expr, env) do + Builder.apply_query(query, __MODULE__, [position, escape(expr)], env) + end + + @doc """ + The callback applied by `build/4` to build the query. + """ + @spec apply(Ecto.Queryable.t(), :pre | :post, term) :: Ecto.Query.t() + def apply(%Ecto.Query{} = query, position, value) do + %{query | comments: query.comments ++ [{position, value}]} + end + + def apply(query, position, value) do + apply(Ecto.Queryable.to_query(query), position, value) + end +end diff --git a/lib/ecto/query/inspect.ex b/lib/ecto/query/inspect.ex index d5427ff0f0..3c291123e0 100644 --- a/lib/ecto/query/inspect.ex +++ b/lib/ecto/query/inspect.ex @@ -124,6 +124,7 @@ defimpl Inspect, for: Ecto.Query do updates = kw_exprs(:update, query.updates, names) lock = kw_inspect(:lock, query.lock) + comments = comments(query.comments) offset = kw_expr(:offset, query.offset, names) select = kw_expr(:select, query.select, names) distinct = kw_expr(:distinct, query.distinct, names) @@ -140,6 +141,7 @@ defimpl Inspect, for: Ecto.Query do limit, offset, lock, + comments, distinct, updates, select, @@ -248,6 +250,10 @@ defimpl Inspect, for: Ecto.Query do defp kw_inspect(_key, nil), do: [] defp kw_inspect(key, val), do: [{key, inspect(val)}] + defp comments(comments) do + for {position, comment} <- comments, do: {:"#{position}_comment", inspect(comment)} + end + defp kw_as_and_prefix(%{as: as, prefix: prefix}) do kw_inspect(:as, as) ++ kw_inspect(:prefix, prefix) end diff --git a/lib/ecto/query/planner.ex b/lib/ecto/query/planner.ex index 45b0a9dcdc..e82a58fb24 100644 --- a/lib/ecto/query/planner.ex +++ b/lib/ecto/query/planner.ex @@ -1073,7 +1073,7 @@ defmodule Ecto.Query.Planner do end defp finalize_cache(query, operation, cache) do - %{assocs: assocs, prefix: prefix, lock: lock, label: label, select: select, aliases: aliases} = + %{assocs: assocs, prefix: prefix, lock: lock, comments: comments, select: select, aliases: aliases} = query aliases = Map.delete(aliases, @parent_as) @@ -1091,7 +1091,7 @@ defmodule Ecto.Query.Planner do |> prepend_if(assocs != [], assocs: assocs) |> prepend_if(prefix != nil, prefix: prefix) |> prepend_if(lock != nil, lock: lock) - |> prepend_if(label != nil, label: label) + |> prepend_if(comments != [], comments: comments) |> prepend_if(aliases != %{}, aliases: aliases) [operation | cache] @@ -2384,34 +2384,23 @@ defmodule Ecto.Query.Planner do def attach_prefix(query, _), do: query @doc """ - Puts the label given via `opts` into the given query, if available. + Appends the comments given via the `:comments` option into the query. - The label is rendered by the adapter as a leading `/* ... */` SQL comment - and becomes part of the query cache key. It is embedded verbatim, so it may - not contain `/*`, `*/`, or null bytes. + The option is a keyword list of `[pre: string, post: string]` entries, which + share the representation of the query's `comments` field. They become part of + the query cache key and are rendered by the adapter as `/* ... */` comments. + Validating and escaping them is left to the adapter, since the comment syntax + (and therefore what is unsafe) depends on the adapter. """ - def attach_label(%{label: nil} = query, opts) when is_list(opts) do - case Keyword.fetch(opts, :label) do - {:ok, label} -> %{query | label: validate_label!(label)} + def attach_comments(query, opts) when is_list(opts) do + case Keyword.fetch(opts, :comments) do + {:ok, comments} when is_list(comments) -> %{query | comments: query.comments ++ comments} + {:ok, other} -> raise ArgumentError, "the :comments option must be a keyword list of [pre: string, post: string], got: #{inspect(other)}" :error -> query end end - def attach_label(query, _), do: query - - defp validate_label!(label) when is_binary(label) do - if String.contains?(label, ["/*", "*/", <<0>>]) do - raise ArgumentError, - "a label cannot contain `/*`, `*/`, or null bytes, got: #{inspect(label)}. " <> - "Allowing them would let the label break out of the surrounding `/* */` comment" - end - - label - end - - defp validate_label!(other) do - raise ArgumentError, "a label must be a string, got: #{inspect(other)}" - end + def attach_comments(query, _), do: query ## Helpers diff --git a/lib/ecto/repo.ex b/lib/ecto/repo.ex index fa6563f59a..11bb22c581 100644 --- a/lib/ecto/repo.ex +++ b/lib/ecto/repo.ex @@ -141,12 +141,17 @@ defmodule Ecto.Repo do in the cache and no cache update function will not be passed to the adapter. Note that this doesn't necessarily disable the database cache, it only affects Ecto's internal cache of normalized queries and adapter prepared statements. Defaults to `true`. - * `:label` - A string embedded into the generated statement as a leading SQL comment, - such as `/* import_users */ INSERT ...`, to identify the query in database logs and - monitoring tools. It is rendered leading (rather than trailing) so it survives truncation - of long statements in logs. The string is embedded verbatim and therefore cannot contain - `/*`, `*/`, or null bytes. The label becomes part of the query cache key, so prefer a - stable identifier per call site over highly dynamic values such as per-request ids. + * `:comments` - A keyword list of `[pre: string, post: string]` entries embedded into the + generated statement as `/* ... */` SQL comments to identify the query in database logs and + monitoring tools. `:pre` comments are rendered before the statement (e.g. + `/* import_users */ INSERT ...`) so they survive truncation of long statements in logs; + `:post` comments are rendered after it. Strings are embedded verbatim and therefore cannot + contain `/*`, `*/`, or null bytes. Comments become part of the query cache key, so prefer + stable identifiers per call site over highly dynamic values such as per-request ids. If you + do pass dynamic comments to a query operation (`all`/`update_all`/`delete_all`), combine them + with `query_cache: false` so the unbounded set of comments does not grow Ecto's query cache + and the adapter's prepared statements. For query expressions you can also use + `Ecto.Query.pre_comment/2` and `Ecto.Query.post_comment/2`, which must be static. ## Adapter-Specific Errors diff --git a/lib/ecto/repo/queryable.ex b/lib/ecto/repo/queryable.ex index 56c70a2642..251cd93926 100644 --- a/lib/ecto/repo/queryable.ex +++ b/lib/ecto/repo/queryable.ex @@ -6,7 +6,7 @@ defmodule Ecto.Repo.Queryable do alias Ecto.Query.Planner alias Ecto.Query.SelectExpr - import Ecto.Query.Planner, only: [attach_prefix: 2, attach_label: 2] + import Ecto.Query.Planner, only: [attach_prefix: 2, attach_comments: 2] require Ecto.Query @@ -37,7 +37,7 @@ defmodule Ecto.Repo.Queryable do |> Ecto.Query.Planner.ensure_select(true) {query, opts} = repo.prepare_query(:stream, query, opts) - query = query |> attach_prefix(opts) |> attach_label(opts) + query = query |> attach_prefix(opts) |> attach_comments(opts) query_cache? = Keyword.get(opts, :query_cache, true) @@ -222,7 +222,7 @@ defmodule Ecto.Repo.Queryable do %{adapter: adapter, cache: cache, repo: repo} = adapter_meta {query, opts} = repo.prepare_query(operation, query, opts) - query = query |> attach_prefix(opts) |> attach_label(opts) + query = query |> attach_prefix(opts) |> attach_comments(opts) query_cache? = Keyword.get(opts, :query_cache, true) diff --git a/test/ecto/query/builder/comment_test.exs b/test/ecto/query/builder/comment_test.exs new file mode 100644 index 0000000000..5a7befedb0 --- /dev/null +++ b/test/ecto/query/builder/comment_test.exs @@ -0,0 +1,62 @@ +Code.require_file "../../../support/eval_helpers.exs", __DIR__ + +defmodule Ecto.Query.Builder.CommentTest do + use ExUnit.Case, async: true + + import Ecto.Query.Builder.Comment + doctest Ecto.Query.Builder.Comment + + import Ecto.Query + import Support.EvalHelpers + + test "pre_comment with literal string" do + query = %Ecto.Query{} |> pre_comment("list_posts") + assert query.comments == [pre: "list_posts"] + end + + test "post_comment with literal string" do + query = %Ecto.Query{} |> post_comment("list_posts") + assert query.comments == [post: "list_posts"] + end + + test "pre_comment via keyword syntax" do + query = from p in "posts", pre_comment: "list_posts" + assert query.comments == [pre: "list_posts"] + end + + test "comments accumulate in order" do + query = %Ecto.Query{} |> pre_comment("a") |> post_comment("b") |> pre_comment("c") + assert query.comments == [{:pre, "a"}, {:post, "b"}, {:pre, "c"}] + end + + test "raises on interpolation (comments must be static)" do + _report = "monthly" + + assert_raise Ecto.Query.CompileError, ~r"interpolation is not allowed", fn -> + quote_and_eval(%Ecto.Query{} |> pre_comment(^_report)) + end + end + + test "raises on a non-literal" do + assert_raise Ecto.Query.CompileError, ~r"is not a valid comment", fn -> + quote_and_eval(%Ecto.Query{} |> pre_comment(1)) + end + end + + test "raises on a comment containing */" do + assert_raise Ecto.Query.CompileError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> + quote_and_eval(%Ecto.Query{} |> post_comment("evil */ DROP")) + end + end + + test "raises on a comment containing /*" do + assert_raise Ecto.Query.CompileError, ~r"cannot contain `/\*`, `\*/`, or null bytes", fn -> + quote_and_eval(%Ecto.Query{} |> pre_comment("evil /* nested")) + end + end + + test "exclude resets the comments" do + query = %Ecto.Query{} |> pre_comment("a") |> post_comment("b") + assert Ecto.Query.exclude(query, :comments).comments == [] + end +end diff --git a/test/ecto/query/planner_test.exs b/test/ecto/query/planner_test.exs index eef8a2fa0a..84a215b9f1 100644 --- a/test/ecto/query/planner_test.exs +++ b/test/ecto/query/planner_test.exs @@ -2908,49 +2908,68 @@ defmodule Ecto.Query.PlannerTest do end end - describe "attach_label/2" do - test "sets the label from opts" do - query = Planner.attach_label(from(p in "posts"), label: "get_posts_q") - assert query.label == "get_posts_q" + describe "attach_comments/2" do + test "appends the comments from opts" do + query = Planner.attach_comments(from(p in "posts"), comments: [pre: "get_posts_q"]) + assert query.comments == [pre: "get_posts_q"] end - test "leaves the query unchanged when no label is given" do + test "appends to comments already on the query" do + query = + from(p in "posts") + |> Ecto.Query.pre_comment("from_macro") + |> Planner.attach_comments(comments: [post: "from_opts"]) + + assert query.comments == [{:pre, "from_macro"}, {:post, "from_opts"}] + end + + test "leaves the query unchanged when no comments are given" do query = from(p in "posts") - assert Planner.attach_label(query, []).label == nil + assert Planner.attach_comments(query, []).comments == [] + end + + test "raises when :comments is not a list" do + assert_raise ArgumentError, ~r/must be a keyword list/, fn -> + Planner.attach_comments(from(p in "posts"), comments: "nope") + end end test "is part of the query cache key" do cache = Planner.new_query_cache(__MODULE__) query = from(p in Post, where: p.title == ^"hello") - cache_query = fn label -> + cache_query = fn comment -> query - |> Planner.attach_label(label: label) + |> Planner.attach_comments(comments: [pre: comment]) |> Planner.query(:all, cache, Ecto.CachingTestAdapter, 0, true) end - # Different labels produce distinct cache entries... + # Different comments produce distinct cache entries... cache_query.("a") cache_query.("b") assert :ets.info(cache, :size) == 2 - # ...while the same label reuses one. + # ...while the same comment reuses one. cache_query.("a") assert :ets.info(cache, :size) == 2 end - test "raises on a non-string label" do - assert_raise ArgumentError, ~r/must be a string/, fn -> - Planner.attach_label(from(p in "posts"), label: 123) + test "query_cache: false keeps the cache from growing with dynamic comments" do + cache = Planner.new_query_cache(__MODULE__) + query = from(p in Post, where: p.title == ^"hello") + + for comment <- ["dyn_a", "dyn_b", "dyn_c"] do + query + |> Planner.attach_comments(comments: [pre: comment]) + |> Planner.query(:all, cache, Ecto.CachingTestAdapter, 0, false) end + + assert :ets.info(cache, :size) == 0 end - test "raises on a label that could break out of the comment" do - for bad <- ["a */ b", "a /* b", "a\0b"] do - assert_raise ArgumentError, ~r/cannot contain/, fn -> - Planner.attach_label(from(p in "posts"), label: bad) - end - end + test "stores comments verbatim, leaving validation to the adapter" do + assert Planner.attach_comments(from(p in "posts"), comments: [pre: "a */ b"]).comments == + [pre: "a */ b"] end end end