Repository: elixir-ecto/ecto
Pull request #4243 (2023-07-24 15:09:08)

Expand Multi transaction error message
Opening a draft to serve as a starting point for discussion. I have run into this error a few times and I've never been able to decipher exactly what it means. It usually appears to result from invalid error handling inside a Multi. Usually that has been unintentional but in a recent case I ran into when handling a nested multi transaction that failed as a success in the parent multi. I've added that case as an example in the docs to point future wayward souls in the right direction. Please advise on accuracy and clarity, although I do think clearly _something_ should be added to this error because it doesn't seem very helpful/instructive currently.


Patch:
diff --git a/examples/friends/config/config.exs b/examples/friends/config/config.exs
index 16f620e7..db814ddb 100644
--- a/examples/friends/config/config.exs
+++ b/examples/friends/config/config.exs
@@ -1,36 +1,7 @@
-# This file is responsible for configuring your application
-# and its dependencies with the aid of the Mix.Config module.
-use Mix.Config
+import Config
 
 config :friends, Friends.Repo,
   database: "friends_repo",
   hostname: "localhost"
 
 config :friends, ecto_repos: [Friends.Repo]
-
-# This configuration is loaded before any dependency and is restricted
-# to this project. If another project depends on this project, this
-# file won't be loaded nor affect the parent project. For this reason,
-# if you want to provide default values for your application for
-# 3rd-party users, it should be done in your "mix.exs" file.
-
-# You can configure for your application as:
-#
-#     config :friends, key: :value
-#
-# And access this configuration in your application as:
-#
-#     Application.get_env(:friends, :key)
-#
-# Or configure a 3rd-party app:
-#
-#     config :logger, level: :info
-#
-
-# It is also possible to import configuration files, relative to this
-# directory. For example, you can emulate configuration per environment
-# by uncommenting the line below and defining dev.exs, test.exs and such.
-# Configuration from the imported file will override the ones defined
-# here (which is why it is important to import them last).
-#
-#     import_config "#{Mix.env}.exs"
diff --git a/examples/friends/mix.exs b/examples/friends/mix.exs
index d66b98a4..7d93a9e3 100644
--- a/examples/friends/mix.exs
+++ b/examples/friends/mix.exs
@@ -5,7 +5,7 @@ defmodule Friends.Mixfile do
     [
       app: :friends,
       version: "0.1.0",
-      elixir: "~> 1.4",
+      elixir: "~> 1.12",
       build_embedded: Mix.env() == :prod,
       deps: deps(),
       aliases: aliases()
@@ -28,13 +28,13 @@ defmodule Friends.Mixfile do
   defp deps do
     # Normally you would specify a version of Ecto, like this:
     #
-    # {:ecto, "~> 3.0"}
+    # {:ecto, "~> 3.10"}
     #
     # It is not done in this instance because we want to refer to the local Ecto.
     [
       {:ecto, path: "../..", override: true},
       {:postgrex, ">= 0.0.0"},
-      {:ecto_sql, "~> 3.0"},
+      {:ecto_sql, "~> 3.10"},
       {:jason, "~> 1.0"}
     ]
   end
diff --git a/examples/friends/mix.lock b/examples/friends/mix.lock
index 28d9c974..3ad7503c 100644
--- a/examples/friends/mix.lock
+++ b/examples/friends/mix.lock
@@ -1,12 +1,12 @@
 %{
-  "connection": {:hex, :connection, "1.0.4", "a1cae72211f0eef17705aaededacac3eb30e6625b04a6117c1b2db6ace7d5976", [:mix], [], "hexpm"},
-  "db_connection": {:hex, :db_connection, "2.0.6", "bde2f85d047969c5b5800cb8f4b3ed6316c8cb11487afedac4aa5f93fd39abfa", [:mix], [{:connection, "~> 1.0.2", [hex: :connection, repo: "hexpm", optional: false]}], "hexpm"},
-  "decimal": {:hex, :decimal, "1.7.0", "30d6b52c88541f9a66637359ddf85016df9eb266170d53105f02e4a67e00c5aa", [:mix], [], "hexpm"},
+  "connection": {:hex, :connection, "1.1.0", "ff2a49c4b75b6fb3e674bfc5536451607270aac754ffd1bdfe175abe4a6d7a68", [:mix], [], "hexpm", "722c1eb0a418fbe91ba7bd59a47e28008a189d47e37e0e7bb85585a016b2869c"},
+  "db_connection": {:hex, :db_connection, "2.5.0", "bb6d4f30d35ded97b29fe80d8bd6f928a1912ca1ff110831edcd238a1973652c", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "c92d5ba26cd69ead1ff7582dbb860adeedfff39774105a4f1c92cbb654b55aa2"},
+  "decimal": {:hex, :decimal, "2.1.1", "5611dca5d4b2c3dd497dec8f68751f1f1a54755e8ed2a966c2633cf885973ad6", [:mix], [], "hexpm", "53cfe5f497ed0e7771ae1a475575603d77425099ba5faef9394932b35020ffcc"},
   "ecto": {:hex, :ecto, "2.0.1", "cf97a4d353e14af3d3cc3b4452cfbd18b3aeee1fb4075475efeccec3853444a9", [:mix], [{:poison, "~> 1.5 or ~> 2.0", [hex: :poison, optional: true]}, {:sbroker, "~> 1.0-beta", [hex: :sbroker, optional: true]}, {:mariaex, "~> 0.7.7", [hex: :mariaex, optional: true]}, {:postgrex, "~> 0.11.2", [hex: :postgrex, optional: true]}, {:db_connection, "~> 1.0-rc.2", [hex: :db_connection, optional: true]}, {:decimal, "~> 1.0", [hex: :decimal, optional: false]}, {:poolboy, "~> 1.5", [hex: :poolboy, optional: false]}]},
-  "ecto_sql": {:hex, :ecto_sql, "3.1.4", "ac36c453542bae69e93a6882c8448743b2bb353cae3587910e5eb3aa8e96a52d", [:mix], [{:db_connection, "~> 2.0", [hex: :db_connection, repo: "hexpm", optional: false]}, {:ecto, "~> 3.1.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:mariaex, "~> 0.9.1", [hex: :mariaex, repo: "hexpm", optional: true]}, {:myxql, "~> 0.2.0", [hex: :myxql, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.14.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm"},
-  "jason": {:hex, :jason, "1.1.2", "b03dedea67a99223a2eaf9f1264ce37154564de899fd3d8b9a21b1a6fd64afe7", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm"},
+  "ecto_sql": {:hex, :ecto_sql, "3.10.1", "6ea6b3036a0b0ca94c2a02613fd9f742614b5cfe494c41af2e6571bb034dd94c", [:mix], [{:db_connection, "~> 2.4.1 or ~> 2.5", [hex: :db_connection, repo: "hexpm", optional: false]}, {:ecto, "~> 3.10.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:myxql, "~> 0.6.0", [hex: :myxql, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.16.0 or ~> 0.17.0 or ~> 1.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:tds, "~> 2.1.1 or ~> 2.2", [hex: :tds, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.0 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "f6a25bdbbd695f12c8171eaff0851fa4c8e72eec1e98c7364402dda9ce11c56b"},
+  "jason": {:hex, :jason, "1.4.1", "af1504e35f629ddcdd6addb3513c3853991f694921b1b9368b0bd32beb9f1b63", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "fbb01ecdfd565b56261302f7e1fcc27c4fb8f32d56eab74db621fc154604a7a1"},
   "poison": {:hex, :poison, "4.0.1", "bcb755a16fac91cad79bfe9fc3585bb07b9331e50cfe3420a24bcc2d735709ae", [:mix], [], "hexpm"},
   "poolboy": {:hex, :poolboy, "1.5.1", "6b46163901cfd0a1b43d692657ed9d7e599853b3b21b95ae5ae0a777cf9b6ca8", [], [], "hexpm"},
-  "postgrex": {:hex, :postgrex, "0.14.3", "5754dee2fdf6e9e508cbf49ab138df964278700b764177e8f3871e658b345a1e", [:mix], [{:connection, "~> 1.0", [hex: :connection, repo: "hexpm", optional: false]}, {:db_connection, "~> 2.0", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}], "hexpm"},
-  "telemetry": {:hex, :telemetry, "0.4.0", "8339bee3fa8b91cb84d14c2935f8ecf399ccd87301ad6da6b71c09553834b2ab", [:rebar3], [], "hexpm"},
+  "postgrex": {:hex, :postgrex, "0.17.2", "a3ec9e3239d9b33f1e5841565c4eb200055c52cc0757a22b63ca2d529bbe764c", [:mix], [{:db_connection, "~> 2.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "80a918a9e9531d39f7bd70621422f3ebc93c01618c645f2d91306f50041ed90c"},
+  "telemetry": {:hex, :telemetry, "1.2.1", "68fdfe8d8f05a8428483a97d7aab2f268aaff24b49e0f599faa091f1d4e7f61c", [:rebar3], [], "hexpm", "dad9ce9d8effc621708f99eac538ef1cbe05d6a874dd741de2e689c47feafed5"},
 }
diff --git a/lib/ecto/query.ex b/lib/ecto/query.ex
index 031af90f..1a52be65 100644
--- a/lib/ecto/query.ex
+++ b/lib/ecto/query.ex
@@ -761,8 +761,12 @@ defmodule Ecto.Query do
   If any other value is given, it is converted to a query via
   `Ecto.Queryable` and wrapped in the `Ecto.SubQuery` struct.
 
-  `subquery` is supported in `from`, `join`, and `where`, in the
-  form `p.x in subquery(q)`.
+  `subquery` is supported in:
+
+    * `from`,
+    * `join`,
+    * `where`, in the form `p.x in subquery(q)`,
+    * `select` and `select_merge`, in the form of `%{field: subquery(...)}`.
 
   ## Examples
 
@@ -809,6 +813,11 @@ defmodule Ecto.Query do
 
   If you need to refer to a parent binding which is not known when writing the subquery,
   you can use `parent_as` as shown in the examples under "Named bindings" in this module doc.
+
+  You can also use subquery directly in `select` and `select_merge`:
+
+      comments_count = from(c in Comment, where: c.post_id == parent_as(:post).id, select: count())
+      from(p in Post, as: :post, select: %{id: p.id, comments: subquery(comments_count)})
   """
   def subquery(query, opts \\ []) do
     subquery = wrap_in_subquery(query)
@@ -939,6 +948,36 @@ defmodule Ecto.Query do
   a value that implements the `Ecto.Queryable` protocol
   and the second argument the expression.
 
+  ## Hints
+
+  The `hints` keyword can be used to specify query hints:
+
+      from p in Post,
+        hints: ["USE INDEX FOO"],
+        where: p.title == "title"
+
+  It can also be used as a general mechanism for adding statements that
+  come after the `from` clause. For example, it can be used to enable
+  table sampling:
+
+      from p in Post,
+        hints: "TABLESAMPLE SYSTEM(1)"
+
+  `from` hints must be a (list of) compile-time strings or unsafe fragments. An unsafe
+  fragment can be used to specify dynamic hints:
+
+      sample = "SYSTEM_ROWS(1)"
+
+      from p in Post,
+        hints: ["TABLESAMPLE", unsafe_fragment(^sample)]
+
+  > ### Unsafe Fragments {: .warning}
+  >
+  > The output of `unsafe_fragment/1` will be injected directly into the
+  > resulting SQL statement without being escaped. For this reason, input
+  > from uncontrolled sources, such as user input, should **never** be used.
+  > Otherwise, it could lead to harmful SQL injection attacks.
+
   ## Keywords examples
 
       # `in` expression
@@ -948,7 +987,7 @@ defmodule Ecto.Query do
       from(City, limit: 1)
 
       # Fragment with user-defined function and predefined columns
-      from(f in fragment("my_table_valued_function(arg)"), select: f.x))
+      from(f in fragment("my_table_valued_function(arg)"), select: f.x)
 
       # Fragment with built-in function and undefined columns
       from(f in fragment("select generate_series(?::integer, ?::integer) as x", ^0, ^10), select: f.x)
@@ -1272,7 +1311,7 @@ defmodule Ecto.Query do
 
   ## Hints
 
-  `from` and `join` also support index hints, as found in databases such as
+  `join` also supports table hints, as found in databases such as
   [MySQL](https://dev.mysql.com/doc/refman/8.0/en/index-hints.html),
   [MSSQL](https://docs.microsoft.com/en-us/sql/t-sql/queries/hints-transact-sql-table?view=sql-server-2017) and
   [Clickhouse](https://clickhouse.tech/docs/en/sql-reference/statements/select/sample/).
@@ -1288,12 +1327,7 @@ defmodule Ecto.Query do
   Keep in mind you want to use hints rarely, so don't forget to read the database
   disclaimers about such functionality.
 
-  Hints must be static compile-time strings when they are specified as (list of) strings.
-  Certain Ecto adapters may also accept dynamic hints using the tuple form:
-
-      from e in Event,
-        hints: [sample: sample_threshold()],
-        select: e
+  Join hints must be static compile-time strings when they are specified as (list of) strings.
 
   ## Array joins
 
@@ -1847,6 +1881,12 @@ defmodule Ecto.Query do
   Combines result sets of multiple queries. The `select` of each query
   must be exactly the same, with the same types in the same order.
 
+  > ### Selecting literal atoms {: .warning}
+  >
+  > When selecting a literal atom, its value must be the same across
+  > all queries. Otherwise, the value from the parent query will be
+  > applied to all other queries.
+
   Union expression returns only unique rows as if each query returned
   distinct results. This may cause a performance penalty. If you need
   to combine multiple result sets without removing duplicate rows
@@ -1893,6 +1933,12 @@ defmodule Ecto.Query do
   Combines result sets of multiple queries. The `select` of each query
   must be exactly the same, with the same types in the same order.
 
+  > ### Selecting literal atoms {: .warning}
+  >
+  > When selecting a literal atom, its value must be the same across
+  > all queries. Otherwise, the value from the parent query will be
+  > applied to all other queries.
+
   Note that the operations `order_by`, `limit` and `offset` of the
   current `query` apply to the result of the union. `order_by` must
   be specified in one of the following ways, since the union of two
@@ -1934,6 +1980,12 @@ defmodule Ecto.Query do
   `select` of each query must be exactly the same, with the same
   types in the same order.
 
+  > ### Selecting literal atoms {: .warning}
+  >
+  > When selecting a literal atom, its value must be the same across
+  > all queries. Otherwise, the value from the parent query will be
+  > applied to all other queries.
+
   Except expression returns only unique rows as if each query returned
   distinct results. This may cause a performance penalty. If you need
   to take the difference of multiple result sets without
@@ -1980,6 +2032,12 @@ defmodule Ecto.Query do
   `select` of each query must be exactly the same, with the same
   types in the same order.
 
+  > ### Selecting literal atoms {: .warning}
+  >
+  > When selecting a literal atom, its value must be the same across
+  > all queries. Otherwise, the value from the parent query will be
+  > applied to all other queries.
+
   Note that the operations `order_by`, `limit` and `offset` of the
   current `query` apply to the result of the set difference. `order_by`
   must be specified in one of the following ways, since the set difference
@@ -2021,6 +2079,12 @@ defmodule Ecto.Query do
   `select` of each query must be exactly the same, with the same
   types in the same order.
 
+  > ### Selecting literal atoms {: .warning}
+  >
+  > When selecting a literal atom, its value must be the same across
+  > all queries. Otherwise, the value from the parent query will be
+  > applied to all other queries.
+
   Intersect expression returns only unique rows as if each query returned
   distinct results. This may cause a performance penalty. If you need
   to take the intersection of multiple result sets without
@@ -2067,6 +2131,12 @@ defmodule Ecto.Query do
   `select` of each query must be exactly the same, with the same
   types in the same order.
 
+  > ### Selecting literal atoms {: .warning}
+  >
+  > When selecting a literal atom, its value must be the same across
+  > all queries. Otherwise, the value from the parent query will be
+  > applied to all other queries.
+
   Note that the operations `order_by`, `limit` and `offset` of the
   current `query` apply to the result of the set difference. `order_by`
   must be specified in one of the following ways, since the intersection
diff --git a/lib/ecto/query/builder/from.ex b/lib/ecto/query/builder/from.ex
index e76b29bd..606ddb0b 100644
--- a/lib/ecto/query/builder/from.ex
+++ b/lib/ecto/query/builder/from.ex
@@ -60,8 +60,8 @@ defmodule Ecto.Query.Builder.From do
         escape_source(other, env)
     end
   end
-  
-  @typep hints :: [String.t() | {atom, term}]
+
+  @typep hints :: [String.t() | Macro.t()]
 
   @doc """
   Builds a quoted expression.
@@ -73,12 +73,7 @@ defmodule Ecto.Query.Builder.From do
   @spec build(Macro.t(), Macro.Env.t(), atom, {:ok, String.t | nil} | nil, hints) ::
           {Macro.t(), Keyword.t(), non_neg_integer | nil}
   def build(query, env, as, prefix, hints) do
-    unless Enum.all?(hints, &is_valid_hint/1) do
-      Builder.error!(
-        "`hints` must be a compile time string, list of strings, a tuple, or a list of tuples " <>
-          "got: `#{Macro.to_string(hints)}`"
-      )
-    end
+    hints = Enum.map(hints, &hint!(&1))
 
     prefix = case prefix do
       nil -> nil
@@ -145,6 +140,31 @@ defmodule Ecto.Query.Builder.From do
   def prefix!(prefix) when is_binary(prefix) or is_nil(prefix), do: prefix
   def prefix!(prefix), do: raise("`prefix` must be a string, got: #{inspect(prefix)}")
 
+  @doc """
+  Validates hints at compile time and runtime
+  """
+  def hint!(hint) when is_binary(hint), do: hint
+
+  def hint!({:unsafe_fragment, _, [fragment]}) do
+    case fragment do
+      {:^, _, [value]} ->
+        quote do: Ecto.Query.Builder.From.hint!(unquote(value))
+
+      _ ->
+        Builder.error!(
+          "`unsafe_fragment/1` in `hints` expects an interpolated value, such as " <>
+            "unsafe_fragment(^value), got: `#{Macro.to_string(fragment)}`"
+        )
+    end
+  end
+
+  def hint!(other) do
+    Builder.error!(
+      "`hints` must be a compile time string, unsafe fragment of the form `unsafe_fragment(^...)`, " <>
+        "or list containing either, got: `#{Macro.to_string(other)}`"
+    )
+  end
+
   @doc """
   The callback applied by `build/2` to build the query.
   """
@@ -194,10 +214,6 @@ defmodule Ecto.Query.Builder.From do
   defp maybe_apply_hints(query, []), do: query
   defp maybe_apply_hints(query, hints), do: update_in(query.from.hints, &(&1 ++ hints))
 
-  defp is_valid_hint(hint) when is_binary(hint), do: true
-  defp is_valid_hint({_key, _val}), do: true
-  defp is_valid_hint(_), do: false
-
   defp check_binds(query, count) do
     if count > 1 and count > Builder.count_binds(query) do
       Builder.error!(
diff --git a/lib/ecto/repo.ex b/lib/ecto/repo.ex
index 316b8d8b..e7739163 100644
--- a/lib/ecto/repo.ex
+++ b/lib/ecto/repo.ex
@@ -1143,6 +1143,9 @@ defmodule Ecto.Repo do
     * `:prefix` - the prefix to fetch preloads from. By default, queries
       will use the same prefix as the first struct in the given collection.
       This option allows the prefix to be changed.
+    * `:on_preloader_spawn` - when preloads are done in parallel, this function
+      will be called in the processes that perform the preloads. This can be useful
+      for context propagation for traces.
 
   See the ["Shared options"](#module-shared-options) section at the module
   documentation for more options.
diff --git a/lib/ecto/repo/preloader.ex b/lib/ecto/repo/preloader.ex
index db3b41d8..8ca2e28e 100644
--- a/lib/ecto/repo/preloader.ex
+++ b/lib/ecto/repo/preloader.ex
@@ -133,9 +133,13 @@ defmodule Ecto.Repo.Preloader do
       # it does not do so in automatic mode, hence this line is
       # still necessary.
       opts = Keyword.put_new(opts, :caller, self())
+      on_preloader_spawn = Keyword.get(opts, :on_preloader_spawn, fn -> :ok end)
 
       preloaders
-      |> Task.async_stream(&(&1.({adapter_meta, opts})), timeout: :infinity)
+      |> Task.async_stream(fn preloader ->
+        on_preloader_spawn.()
+        preloader.({adapter_meta, opts})
+      end, timeout: :infinity)
       |> Enum.map(fn {:ok, assoc} -> assoc end)
     else
       Enum.map(preloaders, &(&1.({adapter_meta, opts})))
diff --git a/lib/ecto/repo/transaction.ex b/lib/ecto/repo/transaction.ex
index a4691c18..2daa0cb2 100644
--- a/lib/ecto/repo/transaction.ex
+++ b/lib/ecto/repo/transaction.ex
@@ -16,9 +16,22 @@ defmodule Ecto.Repo.Transaction do
     return = &adapter.rollback(adapter_meta, &1)
 
     case Ecto.Multi.__apply__(multi, repo, wrap, return) do
-      {:ok, values} -> {:ok, values}
-      {:error, {key, error_value, values}} -> {:error, key, error_value, values}
-      {:error, operation} -> raise "operation #{inspect operation} is manually rolling back, which is not supported by Ecto.Multi"
+      {:ok, values} -> 
+        {:ok, values}
+
+      {:error, {key, error_value, values}} -> 
+        {:error, key, error_value, values}
+
+      {:error, operation} -> 
+        raise """
+        operation #{inspect operation} is rolling back unexpectedly.
+
+        This can happen if `repo.rollback/1` is manually called, which is not \
+        supported by `Ecto.Multi`. It can also occur if a nested transaction \
+        has rolled back and its error is not bubbled up to the outer multi. \
+        Nested transactions are discouraged when using `Ecto.Multi`. Consider \
+        flattening out the transaction instead.
+        """
     end
   end
 


Test patch:
diff --git a/test/ecto/multi_test.exs b/test/ecto/multi_test.exs
index f509f40e..8c0532a0 100644
--- a/test/ecto/multi_test.exs
+++ b/test/ecto/multi_test.exs
@@ -489,7 +489,7 @@ defmodule Ecto.MultiTest do
           repo.rollback(:bar)
         end)
 
-      assert_raise RuntimeError, ~r"operation :bar is manually rolling back, which is not supported by Ecto.Multi", fn ->
+      assert_raise RuntimeError, ~r"operation :bar is rolling back unexpectedly", fn ->
         TestRepo.transaction(multi)
       end
     end
diff --git a/test/ecto/query_test.exs b/test/ecto/query_test.exs
index d1bbdbef..534b2ec0 100644
--- a/test/ecto/query_test.exs
+++ b/test/ecto/query_test.exs
@@ -554,20 +554,34 @@ defmodule Ecto.QueryTest do
       assert query.from.hints == ["hello", "world"]
     end
 
-    test "binary values are expected to be compile-time strings or list of strings" do
+    test "supports interpolation through unsafe_fragment" do
+      hint = "fragment"
+      query = from "posts", hints: unsafe_fragment(^hint)
+      assert query.from.hints == ["fragment"]
+
+      hint = "hint"
+      query = from "posts", hints: ["string", unsafe_fragment(^hint)]
+      assert query.from.hints == ["string", "hint"]
+    end
+
+    test "are expected to be strings or unsafe fragments that evaluate to strings" do
       assert_raise Ecto.Query.CompileError, ~r"`hints` must be a compile time string", fn ->
         quote_and_eval(from "posts", hints: 123)
       end
 
       assert_raise Ecto.Query.CompileError, ~r"`hints` must be a compile time string", fn ->
-        quote_and_eval(from "posts", join: "comments", on: true, hints: 123)
+        quote_and_eval(from "posts", hints: ["string", :atom])
       end
-    end
 
-    test "tuple values are not checked for contents" do
-      hint = "hint_from_config"
-      query = from "posts", hints: [dynamic: hint, number: 123]
-      assert query.from.hints == [dynamic: hint, number: 123]
+      assert_raise Ecto.Query.CompileError, ~r"`hints` must be a compile time string", fn ->
+        quote_and_eval(from "posts", hints: unsafe_fragment(^123))
+      end
+
+      msg = ~r"`unsafe_fragment/1` in `hints` expects an interpolated value"
+
+      assert_raise Ecto.Query.CompileError, msg, fn ->
+        quote_and_eval(from "posts", hints: ["string", unsafe_fragment("123")])
+      end
     end
   end
 
diff --git a/test/ecto/repo_test.exs b/test/ecto/repo_test.exs
index 85a99aa9..aa5c5528 100644
--- a/test/ecto/repo_test.exs
+++ b/test/ecto/repo_test.exs
@@ -85,6 +85,16 @@ defmodule Ecto.RepoTest do
     end
   end
 
+  defmodule MySchemaWithMultiAssoc do
+    use Ecto.Schema
+
+    schema "my_schema" do
+      field :n, :integer
+      belongs_to :parent, MyParent
+      belongs_to :mother, MyParent
+    end
+  end
+
   defmodule MySchemaWithPrefixedAssoc do
     use Ecto.Schema
 
@@ -1785,6 +1795,18 @@ defmodule Ecto.RepoTest do
       assert_received {:all, query, _}
       assert query.from.source == {"my_parent", Ecto.RepoTest.MyParent}
     end
+
+    test "preload with :on_preloader_spawn" do
+      test_process = self()
+      fun = fn -> send(test_process, {:callback_ran, self()}) end
+
+      %MySchemaWithMultiAssoc{parent_id: 1, mother_id: 2}
+      |> PrepareRepo.preload([:parent, :mother], [on_preloader_spawn: fun])
+
+      assert_received {:callback_ran, pid1} when pid1 != self()
+      assert_received {:callback_ran, pid2} when pid2 != self()
+      assert pid1 != pid2
+    end
   end
 
   describe "transaction" do