From 6ad227f020dae78c7c0df0754d9c094883b97da9 Mon Sep 17 00:00:00 2001 From: Tyler Young Date: Wed, 8 Apr 2026 15:10:21 -0500 Subject: [PATCH 1/2] feat (Repo): Raise an error on query-like keyword opts to Repo functions Consider the following, written by a new-to-Elixir developer: ```elixir Repo.delete_all(User, where: [account_id: user.account_id]) ``` The developer mistakenly believed that the `opts` to the Repo function worked like the Ecto.Query DSL, when in fact these keyword options were entirely ignored (making this an unscoped deletion of all users). With this change, Ecto will raise a runtime error when given `opts` keys that seem to indicate the user was trying to modify the queryable passed in to: - Repo.all/2 - Repo.all_by/3 - Repo.delete_all/2 - Repo.exists?/2 - Repo.get/3 - Repo.get_by/3 - Repo.one/2 - Repo.stream/2 --- lib/ecto/repo.ex | 48 +++++++++++++++++++++++++++++++++ test/ecto/repo_test.exs | 59 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/lib/ecto/repo.ex b/lib/ecto/repo.ex index 6a5ad7c8ec..5ac26a436c 100644 --- a/lib/ecto/repo.ex +++ b/lib/ecto/repo.ex @@ -326,6 +326,38 @@ defmodule Ecto.Repo do |> Keyword.merge(opts) end + # Keyword list keys a user may try to pass to something like Repo.all/2, under + # the mistaken belief that the keyword arguments operate like the Ecto.Query DSL. + # We want to proactively raise an error on something like: + # + # Repo.delete_all(User, where: [id: user.id]) + # + # ...since the query will be unscoped in a way that the caller almost certainly + # did not intend. + @unsupported_query_opts [ + :where, + :preload, + :order_by, + :limit, + :group_by, + :distinct, + :select, + :from, + :update + ] + + defp validate_query_opts!(opts, function_name) do + Enum.each(opts, fn + {key, _} when key in @unsupported_query_opts -> + raise ArgumentError, + "unsupported option #{inspect(key)} for Repo.#{function_name}. " <> + "Instead, use Ecto.Query to build a query, and pass that query into this function." + + _ -> + :ok + end) + end + ## Transactions if Ecto.Adapter.Transaction in behaviours do @@ -491,6 +523,8 @@ defmodule Ecto.Repo do end def delete_all(queryable, opts \\ []) do + validate_query_opts!(opts, :delete_all) + repo = get_dynamic_repo() Ecto.Repo.Queryable.delete_all( @@ -502,6 +536,8 @@ defmodule Ecto.Repo do end def all(queryable, opts \\ []) do + validate_query_opts!(opts, :all) + repo = get_dynamic_repo() Ecto.Repo.Queryable.all( @@ -512,6 +548,8 @@ defmodule Ecto.Repo do end def all_by(queryable, clauses, opts \\ []) do + validate_query_opts!(opts, :all_by) + repo = get_dynamic_repo() Ecto.Repo.Queryable.all_by( @@ -523,6 +561,8 @@ defmodule Ecto.Repo do end def stream(queryable, opts \\ []) do + validate_query_opts!(opts, :stream) + repo = get_dynamic_repo() Ecto.Repo.Queryable.stream( @@ -533,6 +573,8 @@ defmodule Ecto.Repo do end def get(queryable, id, opts \\ []) do + validate_query_opts!(opts, :get) + repo = get_dynamic_repo() Ecto.Repo.Queryable.get( @@ -555,6 +597,8 @@ defmodule Ecto.Repo do end def get_by(queryable, clauses, opts \\ []) do + validate_query_opts!(opts, :get_by) + repo = get_dynamic_repo() Ecto.Repo.Queryable.get_by( @@ -597,6 +641,8 @@ defmodule Ecto.Repo do end def one(queryable, opts \\ []) do + validate_query_opts!(opts, :one) + repo = get_dynamic_repo() Ecto.Repo.Queryable.one( @@ -657,6 +703,8 @@ defmodule Ecto.Repo do end def exists?(queryable, opts \\ []) do + validate_query_opts!(opts, :exists?) + repo = get_dynamic_repo() Ecto.Repo.Queryable.exists?( diff --git a/test/ecto/repo_test.exs b/test/ecto/repo_test.exs index ac41607d83..31991ef54a 100644 --- a/test/ecto/repo_test.exs +++ b/test/ecto/repo_test.exs @@ -1600,6 +1600,63 @@ defmodule Ecto.RepoTest do assert schema.__meta__.prefix == %{key: :private} end + test "get, get_by, one, all, all_by, exists?, stream, and delete_all raise an error when given Ecto.Query-like opts" do + for {unsupported_option, bad_opts} <- [ + {:where, [where: [user_id: Ecto.UUID.generate()]]}, + {:preload, [preload: :users]}, + {:order_by, [order_by: :name]}, + {:limit, [limit: 10]} + ] do + assert_raise ArgumentError, + ~r"unsupported option #{inspect(unsupported_option)} for Repo.get", + fn -> + TestRepo.get(MySchema, 123, bad_opts) + end + + assert_raise ArgumentError, + ~r"unsupported option #{inspect(unsupported_option)} for Repo.get_by", + fn -> + TestRepo.get_by(MySchema, [id: 123], bad_opts) + end + + assert_raise ArgumentError, + ~r"unsupported option #{inspect(unsupported_option)} for Repo.one", + fn -> + TestRepo.one(MySchema, bad_opts) + end + + assert_raise ArgumentError, + ~r"unsupported option #{inspect(unsupported_option)} for Repo.all", + fn -> + TestRepo.all(MySchema, bad_opts) + end + + assert_raise ArgumentError, + ~r"unsupported option #{inspect(unsupported_option)} for Repo.all_by", + fn -> + TestRepo.all_by(MySchema, [id: 123], bad_opts) + end + + assert_raise ArgumentError, + ~r"unsupported option #{inspect(unsupported_option)} for Repo.exists?", + fn -> + TestRepo.exists?(MySchema, bad_opts) + end + + assert_raise ArgumentError, + ~r"unsupported option #{inspect(unsupported_option)} for Repo.stream", + fn -> + TestRepo.stream(MySchema, bad_opts) + end + + assert_raise ArgumentError, + ~r"unsupported option #{inspect(unsupported_option)} for Repo.delete_all", + fn -> + TestRepo.delete_all(MySchema, bad_opts) + end + end + end + describe "changeset prepare" do defp prepare_changeset() do %MySchema{id: 1} @@ -1970,7 +2027,9 @@ defmodule Ecto.RepoTest do test "includes conflict target in :replace_all when replace_changed is false" do fields = [:map, :array, :z, :yyy, :x, :id] + TestRepo.insert(%MySchema{id: 1}, on_conflict: :replace_all, conflict_target: [:id], replace_changed: false) + assert_received {:insert, %{source: "my_schema", on_conflict: {^fields, [], [:id]}}} end From e428e7ee7ea03a83789ed6c578d1867c79bd3452 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 9 Apr 2026 09:32:36 +0200 Subject: [PATCH 2/2] Apply suggestion from @josevalim --- lib/ecto/repo.ex | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/ecto/repo.ex b/lib/ecto/repo.ex index 5ac26a436c..2cd90f7009 100644 --- a/lib/ecto/repo.ex +++ b/lib/ecto/repo.ex @@ -342,7 +342,6 @@ defmodule Ecto.Repo do :group_by, :distinct, :select, - :from, :update ]