Add Ecto.Query.label/2 query expression for tagging queries with an S…

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.
 

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

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,

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]

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.
 

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