Skip to content

Commit 76da993

Browse files
committed
Make :label a repo option instead of a query expression.
1 parent cc857f4 commit 76da993

8 files changed

Lines changed: 86 additions & 237 deletions

File tree

‎lib/ecto/query.ex‎

Lines changed: 1 addition & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -956,7 +956,6 @@ defmodule Ecto.Query do
956956
Ecto.Query.exclude(query, :limit)
957957
Ecto.Query.exclude(query, :offset)
958958
Ecto.Query.exclude(query, :lock)
959-
Ecto.Query.exclude(query, :label)
960959
Ecto.Query.exclude(query, :preload)
961960
Ecto.Query.exclude(query, :update)
962961
Ecto.Query.exclude(query, :windows)
@@ -1026,7 +1025,6 @@ defmodule Ecto.Query do
10261025
defp do_exclude(%Ecto.Query{} = query, :limit), do: %{query | limit: nil}
10271026
defp do_exclude(%Ecto.Query{} = query, :offset), do: %{query | offset: nil}
10281027
defp do_exclude(%Ecto.Query{} = query, :lock), do: %{query | lock: nil}
1029-
defp do_exclude(%Ecto.Query{} = query, :label), do: %{query | label: nil}
10301028
defp do_exclude(%Ecto.Query{} = query, :preload), do: %{query | preloads: [], assocs: []}
10311029
defp do_exclude(%Ecto.Query{} = query, :update), do: %{query | updates: []}
10321030
defp do_exclude(%Ecto.Query{} = query, :windows), do: %{query | windows: []}
@@ -1153,7 +1151,7 @@ defmodule Ecto.Query do
11531151
end
11541152

11551153
@from_join_opts [:as, :prefix, :hints]
1156-
@no_binds [:union, :union_all, :except, :except_all, :intersect, :intersect_all, :label]
1154+
@no_binds [:union, :union_all, :except, :except_all, :intersect, :intersect_all]
11571155
@binds [:lock, :where, :or_where, :select, :distinct, :order_by, :group_by, :windows] ++
11581156
[:having, :or_having, :limit, :offset, :preload, :update, :select_merge, :with_ctes]
11591157

@@ -2540,39 +2538,6 @@ defmodule Ecto.Query do
25402538
Builder.Lock.build(query, binding, expr, __CALLER__)
25412539
end
25422540

2543-
@doc ~S"""
2544-
A label query expression.
2545-
2546-
Adds the given text to the generated statement as a leading SQL comment,
2547-
immediately before the statement keyword:
2548-
2549-
/* get_username_q */ SELECT ...
2550-
2551-
This is useful to tag and identify queries in database logs and monitoring
2552-
tools. The label is rendered *leading* (rather than trailing) so it survives
2553-
truncation of long statements in logs.
2554-
2555-
Because the label becomes part of the generated SQL, it is also part of the
2556-
query cache key. Avoid highly dynamic values (such as per-request ids) as they
2557-
would defeat Ecto's prepared-statement caching; prefer a stable identifier per
2558-
call site.
2559-
2560-
## Keywords example
2561-
2562-
from(p in Post, label: "get_post_titles_q", select: p.title)
2563-
2564-
## Expressions example
2565-
2566-
Post |> label("get_post_titles_q") |> select([p], p.title)
2567-
2568-
## Interpolation
2569-
2570-
report = "get_posts_q"
2571-
from(p in Post, label: ^report)
2572-
"""
2573-
defmacro label(query, expr) do
2574-
Builder.Label.build(query, expr, __CALLER__)
2575-
end
25762541

25772542
@doc ~S"""
25782543
An update query expression.

‎lib/ecto/query/builder/label.ex‎

Lines changed: 0 additions & 82 deletions
This file was deleted.

‎lib/ecto/query/inspect.ex‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,6 @@ defimpl Inspect, for: Ecto.Query do
124124
updates = kw_exprs(:update, query.updates, names)
125125

126126
lock = kw_inspect(:lock, query.lock)
127-
label = kw_inspect(:label, query.label)
128127
offset = kw_expr(:offset, query.offset, names)
129128
select = kw_expr(:select, query.select, names)
130129
distinct = kw_expr(:distinct, query.distinct, names)
@@ -141,7 +140,6 @@ defimpl Inspect, for: Ecto.Query do
141140
limit,
142141
offset,
143142
lock,
144-
label,
145143
distinct,
146144
updates,
147145
select,

‎lib/ecto/query/planner.ex‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2383,6 +2383,36 @@ defmodule Ecto.Query.Planner do
23832383

23842384
def attach_prefix(query, _), do: query
23852385

2386+
@doc """
2387+
Puts the label given via `opts` into the given query, if available.
2388+
2389+
The label is rendered by the adapter as a leading `/* ... */` SQL comment
2390+
and becomes part of the query cache key. It is embedded verbatim, so it may
2391+
not contain `/*`, `*/`, or null bytes.
2392+
"""
2393+
def attach_label(%{label: nil} = query, opts) when is_list(opts) do
2394+
case Keyword.fetch(opts, :label) do
2395+
{:ok, label} -> %{query | label: validate_label!(label)}
2396+
:error -> query
2397+
end
2398+
end
2399+
2400+
def attach_label(query, _), do: query
2401+
2402+
defp validate_label!(label) when is_binary(label) do
2403+
if String.contains?(label, ["/*", "*/", <<0>>]) do
2404+
raise ArgumentError,
2405+
"a label cannot contain `/*`, `*/`, or null bytes, got: #{inspect(label)}. " <>
2406+
"Allowing them would let the label break out of the surrounding `/* */` comment"
2407+
end
2408+
2409+
label
2410+
end
2411+
2412+
defp validate_label!(other) do
2413+
raise ArgumentError, "a label must be a string, got: #{inspect(other)}"
2414+
end
2415+
23862416
## Helpers
23872417

23882418
@all_exprs [

‎lib/ecto/repo.ex‎

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,12 @@ defmodule Ecto.Repo do
141141
in the cache and no cache update function will not be passed to the adapter. Note that
142142
this doesn't necessarily disable the database cache, it only affects Ecto's internal
143143
cache of normalized queries and adapter prepared statements. Defaults to `true`.
144+
* `:label` - A string embedded into the generated statement as a leading SQL comment,
145+
such as `/* import_users */ INSERT ...`, to identify the query in database logs and
146+
monitoring tools. It is rendered leading (rather than trailing) so it survives truncation
147+
of long statements in logs. The string is embedded verbatim and therefore cannot contain
148+
`/*`, `*/`, or null bytes. The label becomes part of the query cache key, so prefer a
149+
stable identifier per call site over highly dynamic values such as per-request ids.
144150
145151
## Adapter-Specific Errors
146152
@@ -1581,13 +1587,6 @@ defmodule Ecto.Repo do
15811587
in the schema. For more information see the ["Query Prefix"](`m:Ecto.Query#module-query-prefix`) section of the
15821588
`Ecto.Query` documentation.
15831589
1584-
* `:label` - A string to tag the generated `UPDATE` with a leading SQL
1585-
comment, such as `/* bump_visits */ UPDATE ...`, to identify the statement
1586-
in database logs and monitoring tools. The string is embedded verbatim into
1587-
the SQL and therefore cannot contain `/*`, `*/`, or null bytes. Prefer a
1588-
stable identifier per call site, as the label becomes part of the query
1589-
cache key. See `Ecto.Query.label/2` to set it on the query itself instead.
1590-
15911590
See the ["Shared options"](#module-shared-options) section at the module
15921591
documentation for remaining options.
15931592
@@ -1637,13 +1636,6 @@ defmodule Ecto.Repo do
16371636
in the schema. For more information see the ["Query Prefix"](`m:Ecto.Query#module-query-prefix`) section of the
16381637
`Ecto.Query` documentation.
16391638
1640-
* `:label` - A string to tag the generated `DELETE` with a leading SQL
1641-
comment, such as `/* purge_stale */ DELETE ...`, to identify the statement
1642-
in database logs and monitoring tools. The string is embedded verbatim into
1643-
the SQL and therefore cannot contain `/*`, `*/`, or null bytes. Prefer a
1644-
stable identifier per call site, as the label becomes part of the query
1645-
cache key. See `Ecto.Query.label/2` to set it on the query itself instead.
1646-
16471639
See the ["Shared options"](#module-shared-options) section at the module
16481640
documentation for remaining options.
16491641
@@ -1746,13 +1738,6 @@ defmodule Ecto.Repo do
17461738
* `:placeholders` - A map with placeholders. This feature is not supported
17471739
by all databases. See the ["Placeholders" section](#c:insert_all/3-placeholders) for more information.
17481740
1749-
* `:label` - A string to tag the generated `INSERT` with a leading SQL
1750-
comment, such as `/* import_users */ INSERT ...`, to identify the statement
1751-
in database logs and monitoring tools. The string is embedded verbatim into
1752-
the SQL and therefore cannot contain `/*`, `*/`, or null bytes. Prefer a
1753-
stable identifier per call site, as the label becomes part of the query
1754-
cache key.
1755-
17561741
See the ["Shared options"](#module-shared-options) section at the module
17571742
documentation for remaining options.
17581743

‎lib/ecto/repo/queryable.ex‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ defmodule Ecto.Repo.Queryable do
66
alias Ecto.Query.Planner
77
alias Ecto.Query.SelectExpr
88

9-
import Ecto.Query.Planner, only: [attach_prefix: 2]
9+
import Ecto.Query.Planner, only: [attach_prefix: 2, attach_label: 2]
1010

1111
require Ecto.Query
1212

@@ -37,7 +37,7 @@ defmodule Ecto.Repo.Queryable do
3737
|> Ecto.Query.Planner.ensure_select(true)
3838

3939
{query, opts} = repo.prepare_query(:stream, query, opts)
40-
query = attach_prefix(query, opts)
40+
query = query |> attach_prefix(opts) |> attach_label(opts)
4141

4242
query_cache? = Keyword.get(opts, :query_cache, true)
4343

@@ -222,7 +222,7 @@ defmodule Ecto.Repo.Queryable do
222222
%{adapter: adapter, cache: cache, repo: repo} = adapter_meta
223223

224224
{query, opts} = repo.prepare_query(operation, query, opts)
225-
query = attach_prefix(query, opts)
225+
query = query |> attach_prefix(opts) |> attach_label(opts)
226226

227227
query_cache? = Keyword.get(opts, :query_cache, true)
228228

‎test/ecto/query/builder/label_test.exs‎

Lines changed: 0 additions & 93 deletions
This file was deleted.

0 commit comments

Comments
 (0)