From 401efa8b848d5a982c7defee6a09af23d632c622 Mon Sep 17 00:00:00 2001 From: David Green <134172184+green-david@users.noreply.github.com> Date: Tue, 16 Jun 2026 11:58:27 -0400 Subject: [PATCH 01/11] Do not reflect changes ignored due to :writable in returned struct --- lib/ecto/repo/schema.ex | 2 +- test/ecto/repo_test.exs | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/ecto/repo/schema.ex b/lib/ecto/repo/schema.ex index c80e62f920..4adf494378 100644 --- a/lib/ecto/repo/schema.ex +++ b/lib/ecto/repo/schema.ex @@ -630,7 +630,7 @@ defmodule Ecto.Repo.Schema do case changes do %{^field => _change} -> handle_writable_violation(field, schema, action) - changes + Map.delete(changes, field) %{} -> changes end diff --git a/test/ecto/repo_test.exs b/test/ecto/repo_test.exs index 2a71ad107b..2fc61455ad 100644 --- a/test/ecto/repo_test.exs +++ b/test/ecto/repo_test.exs @@ -2439,6 +2439,20 @@ defmodule Ecto.RepoTest do ] end + test "update with on_writable_violation: :nothing/warn does not reflect ignored changes in returned struct" do + {:ok, %MySchemaWritable{always: 10, never: nil, insert: nil}} = + %MySchemaWritable{id: 1} + |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) + |> TestRepo.update() + + capture_log(fn -> + {:ok, %MySchemaWritableWarn{always: 10, never: nil, insert: nil}} = + %MySchemaWritableWarn{id: 1} + |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) + |> TestRepo.update() + end) + end + test "update with on_writable_violation: :nothing saves changes for writable: :always and ignores changes for writable: :insert/:never" do %MySchemaWritable{id: 1} |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) @@ -2513,6 +2527,20 @@ defmodule Ecto.RepoTest do end end + test "insert with on_writable_violation: :nothing/warn does not reflect ignored changes in returned struct" do + {:ok, %MySchemaWritable{always: 10, never: nil, insert: 12}} = + %MySchemaWritable{id: 1} + |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) + |> TestRepo.insert() + + capture_log(fn -> + {:ok, %MySchemaWritableWarn{always: 10, never: nil, insert: 12}} = + %MySchemaWritableWarn{id: 2} + |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) + |> TestRepo.insert() + end) + end + test "insert with surfaced changes on_writable_violation: :nothing saves changes for writable: :always/:insert and ignores changes for writable: :never" do %MySchemaWritable{id: 1, always: 10, never: 11, insert: 12} |> Ecto.Changeset.change(%{}) From bfe9ab7e26bdc797db38749b73222eb5707c37d1 Mon Sep 17 00:00:00 2001 From: David Green <134172184+green-david@users.noreply.github.com> Date: Tue, 16 Jun 2026 12:04:42 -0400 Subject: [PATCH 02/11] Fix incorrect docs --- lib/ecto/schema.ex | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ecto/schema.ex b/lib/ecto/schema.ex index b8bdd26938..831d300103 100644 --- a/lib/ecto/schema.ex +++ b/lib/ecto/schema.ex @@ -706,8 +706,7 @@ defmodule Ecto.Schema do attempts to modify a field that should not be modified according to it's `:writable` value. Must be one of `:nothing`, `:warn`, or `:raise`. If set to `:nothing`, the modification is silently ignored. If set to `:warn`, the modification is ignored and a warning is logged. If set - to `:raise`, an exception is raised and the operation is aborted. If `:writable` is set to `:always`, - `:on_writable_violation` must be set to `:nothing`. Defaults to `:nothing`. + to `:raise`, an exception is raised and the operation is aborted. Defaults to `:nothing`. """ defmacro field(name, type \\ :string, opts \\ []) do From 58413a66d94ae901c5205b1b25dc04536746c6d0 Mon Sep 17 00:00:00 2001 From: David Green <134172184+green-david@users.noreply.github.com> Date: Tue, 16 Jun 2026 20:46:14 -0400 Subject: [PATCH 03/11] Ensure that the returned struct from an insert with surfaced changes accurately reflects skipped non-writable fields --- lib/ecto/repo/schema.ex | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/lib/ecto/repo/schema.ex b/lib/ecto/repo/schema.ex index 4adf494378..f7ed36600f 100644 --- a/lib/ecto/repo/schema.ex +++ b/lib/ecto/repo/schema.ex @@ -453,7 +453,7 @@ defmodule Ecto.Repo.Schema do # changeset as changes, except the primary key if it is nil. changeset = put_repo_and_action(changeset, :insert, repo, tuplet) changeset = Relation.surface_changes(changeset, struct, keep_fields ++ drop_fields ++ assocs) - changeset = update_in(changeset.changes, &drop_non_writable_changes!(&1, drop_fields, schema, :insert)) + changeset = process_non_writable_fields!(changeset, drop_fields, schema, :insert) wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn -> assoc_opts = assoc_opts(assocs, opts) @@ -561,7 +561,7 @@ defmodule Ecto.Repo.Schema do # fields into the changeset. All changes must be in the # changeset before hand. changeset = put_repo_and_action(changeset, :update, repo, tuplet) - changeset = update_in(changeset.changes, &drop_non_writable_changes!(&1, drop_fields, schema, :update)) + changeset = process_non_writable_fields!(changeset, drop_fields, schema, :update) if changeset.changes != %{} or force? do wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn -> @@ -625,16 +625,31 @@ defmodule Ecto.Repo.Schema do {:error, put_repo_and_action(changeset, :update, repo, tuplet)} end - defp drop_non_writable_changes!(changes, non_writable_fields, schema, action) do - Enum.reduce(non_writable_fields, changes, fn field, changes -> - case changes do - %{^field => _change} -> - handle_writable_violation(field, schema, action) - Map.delete(changes, field) - %{} -> - changes - end - end) + defp process_non_writable_fields!(changeset, non_writable_fields, schema, action) do + changes = + Enum.reduce(non_writable_fields, changeset.changes, fn field, changes -> + case changes do + %{^field => _change} -> + handle_writable_violation(field, schema, action) + Map.delete(changes, field) + %{} -> + changes + end + end) + + # On insert, if the underlying struct has a value for a non-writable + # field, we need to nilify it so that the struct returned from the + # insert operation reflects that the write was not actually performed. + # Discarding the change above only prevents the actual write from happening. + data = case action do + :insert -> + updates = Enum.map(non_writable_fields, fn field -> {field, nil} end) + struct!(changeset.data, updates) + _ -> + changeset.data + end + + %{changeset | data: data, changes: changes} end defp handle_writable_violation(field, schema, action) do From 73fac166c5b7f1c0d34713c222f7be8eff17f5e5 Mon Sep 17 00:00:00 2001 From: David Green <134172184+green-david@users.noreply.github.com> Date: Tue, 16 Jun 2026 20:46:31 -0400 Subject: [PATCH 04/11] Simplify tests and test that the returned struct values are accurate for non-writable fields --- test/ecto/repo_test.exs | 99 +++++++++++++++++------------------------ 1 file changed, 40 insertions(+), 59 deletions(-) diff --git a/test/ecto/repo_test.exs b/test/ecto/repo_test.exs index 2fc61455ad..4f1858b7c8 100644 --- a/test/ecto/repo_test.exs +++ b/test/ecto/repo_test.exs @@ -2439,33 +2439,21 @@ defmodule Ecto.RepoTest do ] end - test "update with on_writable_violation: :nothing/warn does not reflect ignored changes in returned struct" do - {:ok, %MySchemaWritable{always: 10, never: nil, insert: nil}} = + test "update with on_writable_violation: :nothing saves changes for writable: :always and ignores changes for writable: :insert/:never" do + %{always: 10, never: nil, insert: nil} = %MySchemaWritable{id: 1} |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) - |> TestRepo.update() - - capture_log(fn -> - {:ok, %MySchemaWritableWarn{always: 10, never: nil, insert: nil}} = - %MySchemaWritableWarn{id: 1} - |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) - |> TestRepo.update() - end) - end - - test "update with on_writable_violation: :nothing saves changes for writable: :always and ignores changes for writable: :insert/:never" do - %MySchemaWritable{id: 1} - |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) - |> TestRepo.update() + |> TestRepo.update!() assert_received {:update, %{changes: [always: 10]}} end test "update with on_writable_violation: :warn saves changes for writable: :always, ignores changes for writable: :insert/:never, and logs a warning" do log = capture_log(fn -> - %MySchemaWritableWarn{id: 1} - |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) - |> TestRepo.update() + %{always: 10, never: nil, insert: nil} = + %MySchemaWritableWarn{id: 1} + |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) + |> TestRepo.update!() assert_received {:update, %{changes: [always: 10]}} end) @@ -2478,18 +2466,19 @@ defmodule Ecto.RepoTest do assert_raise ArgumentError, "attempted to write to non-writable field :never during update", fn -> %MySchemaWritableRaise{id: 1} |> Ecto.Changeset.change(%{never: 10}) - |> TestRepo.update() + |> TestRepo.update!() end assert_raise ArgumentError, "attempted to write to non-writable field :insert during update", fn -> %MySchemaWritableRaise{id: 2} |> Ecto.Changeset.change(%{insert: 11}) - |> TestRepo.update() + |> TestRepo.update!() end - %MySchemaWritableRaise{id: 3} - |> Ecto.Changeset.change(%{always: 12}) - |> TestRepo.update() + %{always: 12, never: nil, insert: nil} = + %MySchemaWritableRaise{id: 3} + |> Ecto.Changeset.change(%{always: 12}) + |> TestRepo.update!() assert_received {:update, %{changes: [always: 12]}} end @@ -2527,43 +2516,32 @@ defmodule Ecto.RepoTest do end end - test "insert with on_writable_violation: :nothing/warn does not reflect ignored changes in returned struct" do - {:ok, %MySchemaWritable{always: 10, never: nil, insert: 12}} = - %MySchemaWritable{id: 1} - |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) - |> TestRepo.insert() - - capture_log(fn -> - {:ok, %MySchemaWritableWarn{always: 10, never: nil, insert: 12}} = - %MySchemaWritableWarn{id: 2} - |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) - |> TestRepo.insert() - end) - end - test "insert with surfaced changes on_writable_violation: :nothing saves changes for writable: :always/:insert and ignores changes for writable: :never" do - %MySchemaWritable{id: 1, always: 10, never: 11, insert: 12} - |> Ecto.Changeset.change(%{}) - |> TestRepo.insert() + %{always: 10, never: nil, insert: 12} = + %MySchemaWritable{id: 1, always: 10, never: 11, insert: 12} + |> Ecto.Changeset.change(%{}) + |> TestRepo.insert!() assert_received {:insert, %{fields: inserted_fields}} assert Enum.sort(inserted_fields) == [always: 10, id: 1, insert: 12] end test "insert with on_writable_violation: :nothing saves changes for writable: :always/:insert and ignores changes for writable: :never" do - %MySchemaWritable{id: 1} - |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) - |> TestRepo.insert() + %{always: 10, never: nil, insert: 12} = + %MySchemaWritable{id: 1} + |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) + |> TestRepo.insert!() assert_received {:insert, %{fields: inserted_fields}} assert Enum.sort(inserted_fields) == [always: 10, id: 1, insert: 12] end - test "insert with with surfaced changes and on_writable_violation: :warn saves changes for writable: :always/:insert, ignores changes for writable: :never, and logs a warning" do + test "insert with surfaced changes and on_writable_violation: :warn saves changes for writable: :always/:insert, ignores changes for writable: :never, and logs a warning" do log = capture_log(fn -> - %MySchemaWritableWarn{id: 1, always: 10, never: 11, insert: 12} - |> Ecto.Changeset.change(%{}) - |> TestRepo.insert() + %{always: 10, never: nil, insert: 12} = + %MySchemaWritableWarn{id: 1, always: 10, never: 11, insert: 12} + |> Ecto.Changeset.change(%{}) + |> TestRepo.insert!() assert_received {:insert, %{fields: inserted_fields}} assert Enum.sort(inserted_fields) == [always: 10, id: 1, insert: 12] @@ -2574,9 +2552,10 @@ defmodule Ecto.RepoTest do test "insert with on_writable_violation: :warn saves changes for writable: :always/:insert, ignores changes for writable: :never, and logs a warning" do log = capture_log(fn -> - %MySchemaWritableWarn{id: 1} - |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) - |> TestRepo.insert() + %{always: 10, never: nil, insert: 12} = + %MySchemaWritableWarn{id: 1} + |> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12}) + |> TestRepo.insert!() assert_received {:insert, %{fields: inserted_fields}} assert Enum.sort(inserted_fields) == [always: 10, id: 1, insert: 12] @@ -2589,12 +2568,13 @@ defmodule Ecto.RepoTest do assert_raise ArgumentError, "attempted to write to non-writable field :never during insert", fn -> %MySchemaWritableRaise{id: 1, never: 10} |> Ecto.Changeset.change(%{}) - |> TestRepo.insert() + |> TestRepo.insert!() end - %MySchemaWritableRaise{id: 2, insert: 11, always: 12} - |> Ecto.Changeset.change(%{}) - |> TestRepo.insert() + %{always: 12, never: nil, insert: 11} = + %MySchemaWritableRaise{id: 2, insert: 11, always: 12} + |> Ecto.Changeset.change(%{}) + |> TestRepo.insert!() assert_received {:insert, %{fields: inserted_fields}} assert Enum.sort(inserted_fields) == [always: 12, id: 2, insert: 11] @@ -2604,12 +2584,13 @@ defmodule Ecto.RepoTest do assert_raise ArgumentError, "attempted to write to non-writable field :never during insert", fn -> %MySchemaWritableRaise{id: 1} |> Ecto.Changeset.change(%{never: 10}) - |> TestRepo.insert() + |> TestRepo.insert!() end - %MySchemaWritableRaise{id: 2} - |> Ecto.Changeset.change(%{insert: 11, always: 12}) - |> TestRepo.insert() + %{always: 12, never: nil, insert: 11} = + %MySchemaWritableRaise{id: 2} + |> Ecto.Changeset.change(%{insert: 11, always: 12}) + |> TestRepo.insert!() assert_received {:insert, %{fields: inserted_fields}} assert Enum.sort(inserted_fields) == [always: 12, id: 2, insert: 11] From 51183d821488c26970ce6734acff1e65cf4cb9c2 Mon Sep 17 00:00:00 2001 From: David Green <134172184+green-david@users.noreply.github.com> Date: Wed, 17 Jun 2026 02:20:50 -0400 Subject: [PATCH 05/11] Optimize non-writable field processing --- lib/ecto/repo/schema.ex | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/ecto/repo/schema.ex b/lib/ecto/repo/schema.ex index f7ed36600f..2674c6a24d 100644 --- a/lib/ecto/repo/schema.ex +++ b/lib/ecto/repo/schema.ex @@ -626,27 +626,29 @@ defmodule Ecto.Repo.Schema do end defp process_non_writable_fields!(changeset, non_writable_fields, schema, action) do - changes = - Enum.reduce(non_writable_fields, changeset.changes, fn field, changes -> + {changes, data_updates} = + Enum.reduce(non_writable_fields, {changeset.changes, []}, fn field, {changes, data_updates} -> case changes do %{^field => _change} -> handle_writable_violation(field, schema, action) - Map.delete(changes, field) + + # On insert, we need to nilify non-writable fields in + # the underlying data so that the returned struct reflects + # that the write was not actually performed. + data_updates = case action do + :insert -> [{field, nil} | data_updates] + _ -> data_updates + end + + {Map.delete(changes, field), data_updates} %{} -> - changes + {changes, data_updates} end end) - # On insert, if the underlying struct has a value for a non-writable - # field, we need to nilify it so that the struct returned from the - # insert operation reflects that the write was not actually performed. - # Discarding the change above only prevents the actual write from happening. - data = case action do - :insert -> - updates = Enum.map(non_writable_fields, fn field -> {field, nil} end) - struct!(changeset.data, updates) - _ -> - changeset.data + data = case data_updates do + [] -> changeset.data + data_updates -> struct!(changeset.data, data_updates) end %{changeset | data: data, changes: changes} From f4bb632f93092bb4581648497ceb8aab1d61ea2d Mon Sep 17 00:00:00 2001 From: David Green <134172184+green-david@users.noreply.github.com> Date: Wed, 17 Jun 2026 02:24:43 -0400 Subject: [PATCH 06/11] Remove extraneous test assertions --- test/ecto/repo_test.exs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/test/ecto/repo_test.exs b/test/ecto/repo_test.exs index 4f1858b7c8..f8b48fc431 100644 --- a/test/ecto/repo_test.exs +++ b/test/ecto/repo_test.exs @@ -2475,10 +2475,9 @@ defmodule Ecto.RepoTest do |> TestRepo.update!() end - %{always: 12, never: nil, insert: nil} = - %MySchemaWritableRaise{id: 3} - |> Ecto.Changeset.change(%{always: 12}) - |> TestRepo.update!() + %MySchemaWritableRaise{id: 3} + |> Ecto.Changeset.change(%{always: 12}) + |> TestRepo.update!() assert_received {:update, %{changes: [always: 12]}} end @@ -2571,10 +2570,10 @@ defmodule Ecto.RepoTest do |> TestRepo.insert!() end - %{always: 12, never: nil, insert: 11} = - %MySchemaWritableRaise{id: 2, insert: 11, always: 12} - |> Ecto.Changeset.change(%{}) - |> TestRepo.insert!() + + %MySchemaWritableRaise{id: 2, insert: 11, always: 12} + |> Ecto.Changeset.change(%{}) + |> TestRepo.insert!() assert_received {:insert, %{fields: inserted_fields}} assert Enum.sort(inserted_fields) == [always: 12, id: 2, insert: 11] @@ -2587,10 +2586,9 @@ defmodule Ecto.RepoTest do |> TestRepo.insert!() end - %{always: 12, never: nil, insert: 11} = - %MySchemaWritableRaise{id: 2} - |> Ecto.Changeset.change(%{insert: 11, always: 12}) - |> TestRepo.insert!() + %MySchemaWritableRaise{id: 2} + |> Ecto.Changeset.change(%{insert: 11, always: 12}) + |> TestRepo.insert!() assert_received {:insert, %{fields: inserted_fields}} assert Enum.sort(inserted_fields) == [always: 12, id: 2, insert: 11] From 241b3eea6f36475db4fade957bb11ee4645e4f19 Mon Sep 17 00:00:00 2001 From: David Green <134172184+green-david@users.noreply.github.com> Date: Wed, 17 Jun 2026 10:02:50 -0400 Subject: [PATCH 07/11] Special handle non-writable fields in data for insert --- lib/ecto/repo/schema.ex | 61 +++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/lib/ecto/repo/schema.ex b/lib/ecto/repo/schema.ex index 2674c6a24d..5d8a49ce03 100644 --- a/lib/ecto/repo/schema.ex +++ b/lib/ecto/repo/schema.ex @@ -452,8 +452,12 @@ defmodule Ecto.Repo.Schema do # On insert, we always merge the whole struct into the # changeset as changes, except the primary key if it is nil. changeset = put_repo_and_action(changeset, :insert, repo, tuplet) - changeset = Relation.surface_changes(changeset, struct, keep_fields ++ drop_fields ++ assocs) - changeset = process_non_writable_fields!(changeset, drop_fields, schema, :insert) + changeset = Relation.surface_changes(changeset, struct, keep_fields ++ assocs) + # On insert, we need to nilify non-writable fields in + # the underlying data so that the returned struct reflects + # that the write was not actually performed. + changeset = update_in(changeset.data, &nilify_unsurfaced_non_writable_data!(&1, drop_fields, schema)) + changeset = update_in(changeset.changes, &drop_non_writable_changes!(&1, drop_fields, schema, :insert)) wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn -> assoc_opts = assoc_opts(assocs, opts) @@ -522,6 +526,21 @@ defmodule Ecto.Repo.Schema do {:error, put_repo_and_action(changeset, :insert, repo, tuplet)} end + defp nilify_unsurfaced_non_writable_data!(data, non_writable_fields, schema) do + updates = Enum.reduce(non_writable_fields, [], fn field, updates -> + case data do + %{^field => value} when value != nil -> + handle_writable_violation(field, schema, :insert) + + [{field, nil} | updates] + %{} -> + updates + end + end) + + struct!(data, updates) + end + @doc """ Implementation for `Ecto.Repo.update/2`. """ @@ -561,7 +580,7 @@ defmodule Ecto.Repo.Schema do # fields into the changeset. All changes must be in the # changeset before hand. changeset = put_repo_and_action(changeset, :update, repo, tuplet) - changeset = process_non_writable_fields!(changeset, drop_fields, schema, :update) + changeset = update_in(changeset.changes, &drop_non_writable_changes!(&1, drop_fields, schema, :update)) if changeset.changes != %{} or force? do wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn -> @@ -625,33 +644,17 @@ defmodule Ecto.Repo.Schema do {:error, put_repo_and_action(changeset, :update, repo, tuplet)} end - defp process_non_writable_fields!(changeset, non_writable_fields, schema, action) do - {changes, data_updates} = - Enum.reduce(non_writable_fields, {changeset.changes, []}, fn field, {changes, data_updates} -> - case changes do - %{^field => _change} -> - handle_writable_violation(field, schema, action) - - # On insert, we need to nilify non-writable fields in - # the underlying data so that the returned struct reflects - # that the write was not actually performed. - data_updates = case action do - :insert -> [{field, nil} | data_updates] - _ -> data_updates - end - - {Map.delete(changes, field), data_updates} - %{} -> - {changes, data_updates} - end - end) + defp drop_non_writable_changes!(changes, non_writable_fields, schema, action) do + Enum.reduce(non_writable_fields, changes, fn field, changes -> + case changes do + %{^field => _change} -> + handle_writable_violation(field, schema, action) - data = case data_updates do - [] -> changeset.data - data_updates -> struct!(changeset.data, data_updates) - end - - %{changeset | data: data, changes: changes} + Map.delete(changes, field) + %{} -> + changes + end + end) end defp handle_writable_violation(field, schema, action) do From f3971157680f2ffe525bbedc684474de5618de37 Mon Sep 17 00:00:00 2001 From: David Green <134172184+green-david@users.noreply.github.com> Date: Wed, 17 Jun 2026 18:40:40 -0400 Subject: [PATCH 08/11] Manipulate changeset within subfunction rather than using update_in --- lib/ecto/repo/schema.ex | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/ecto/repo/schema.ex b/lib/ecto/repo/schema.ex index 5d8a49ce03..567c46e4d3 100644 --- a/lib/ecto/repo/schema.ex +++ b/lib/ecto/repo/schema.ex @@ -456,8 +456,8 @@ defmodule Ecto.Repo.Schema do # On insert, we need to nilify non-writable fields in # the underlying data so that the returned struct reflects # that the write was not actually performed. - changeset = update_in(changeset.data, &nilify_unsurfaced_non_writable_data!(&1, drop_fields, schema)) - changeset = update_in(changeset.changes, &drop_non_writable_changes!(&1, drop_fields, schema, :insert)) + changeset = nilify_unsurfaced_non_writable_data!(changeset, drop_fields, schema) + changeset = drop_non_writable_changes!(changeset, drop_fields, schema, :insert) wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn -> assoc_opts = assoc_opts(assocs, opts) @@ -526,9 +526,9 @@ defmodule Ecto.Repo.Schema do {:error, put_repo_and_action(changeset, :insert, repo, tuplet)} end - defp nilify_unsurfaced_non_writable_data!(data, non_writable_fields, schema) do + defp nilify_unsurfaced_non_writable_data!(changeset, non_writable_fields, schema) do updates = Enum.reduce(non_writable_fields, [], fn field, updates -> - case data do + case changeset.data do %{^field => value} when value != nil -> handle_writable_violation(field, schema, :insert) @@ -538,7 +538,7 @@ defmodule Ecto.Repo.Schema do end end) - struct!(data, updates) + %{changeset | data: struct!(changeset.data, updates)} end @doc """ @@ -580,7 +580,7 @@ defmodule Ecto.Repo.Schema do # fields into the changeset. All changes must be in the # changeset before hand. changeset = put_repo_and_action(changeset, :update, repo, tuplet) - changeset = update_in(changeset.changes, &drop_non_writable_changes!(&1, drop_fields, schema, :update)) + changeset = drop_non_writable_changes!(changeset, drop_fields, schema, :update) if changeset.changes != %{} or force? do wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn -> @@ -644,8 +644,8 @@ defmodule Ecto.Repo.Schema do {:error, put_repo_and_action(changeset, :update, repo, tuplet)} end - defp drop_non_writable_changes!(changes, non_writable_fields, schema, action) do - Enum.reduce(non_writable_fields, changes, fn field, changes -> + defp drop_non_writable_changes!(changeset, non_writable_fields, schema, action) do + changes = Enum.reduce(non_writable_fields, changeset.changes, fn field, changes -> case changes do %{^field => _change} -> handle_writable_violation(field, schema, action) @@ -655,6 +655,8 @@ defmodule Ecto.Repo.Schema do changes end end) + + %{changeset | changes: changes} end defp handle_writable_violation(field, schema, action) do From 5f7297345cafc6139bbdb2913f474e6392009ca2 Mon Sep 17 00:00:00 2001 From: David Green <134172184+green-david@users.noreply.github.com> Date: Wed, 17 Jun 2026 18:42:09 -0400 Subject: [PATCH 09/11] Short-circuit non-writable logic if there are no non-writable fields --- lib/ecto/repo/schema.ex | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/ecto/repo/schema.ex b/lib/ecto/repo/schema.ex index 567c46e4d3..7792532f4c 100644 --- a/lib/ecto/repo/schema.ex +++ b/lib/ecto/repo/schema.ex @@ -526,6 +526,10 @@ defmodule Ecto.Repo.Schema do {:error, put_repo_and_action(changeset, :insert, repo, tuplet)} end + defp nilify_unsurfaced_non_writable_data!(changeset, [], _schema) do + changeset + end + defp nilify_unsurfaced_non_writable_data!(changeset, non_writable_fields, schema) do updates = Enum.reduce(non_writable_fields, [], fn field, updates -> case changeset.data do @@ -644,6 +648,10 @@ defmodule Ecto.Repo.Schema do {:error, put_repo_and_action(changeset, :update, repo, tuplet)} end + defp drop_non_writable_changes!(changeset, [], _schema, _action) do + changeset + end + defp drop_non_writable_changes!(changeset, non_writable_fields, schema, action) do changes = Enum.reduce(non_writable_fields, changeset.changes, fn field, changes -> case changes do From 73371830ef4362ceae8f37e8d448cc42b7a9634b Mon Sep 17 00:00:00 2001 From: David Green <134172184+green-david@users.noreply.github.com> Date: Wed, 17 Jun 2026 19:36:53 -0400 Subject: [PATCH 10/11] Restore previous behavior of reflecting surfaced changes in returned struct --- lib/ecto/repo/schema.ex | 14 ++++++-------- test/ecto/repo_test.exs | 10 ++++++++-- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/ecto/repo/schema.ex b/lib/ecto/repo/schema.ex index 7792532f4c..157a4575af 100644 --- a/lib/ecto/repo/schema.ex +++ b/lib/ecto/repo/schema.ex @@ -456,7 +456,7 @@ defmodule Ecto.Repo.Schema do # On insert, we need to nilify non-writable fields in # the underlying data so that the returned struct reflects # that the write was not actually performed. - changeset = nilify_unsurfaced_non_writable_data!(changeset, drop_fields, schema) + changeset = detect_unsurfaced_non_writable_data!(changeset, drop_fields, schema) changeset = drop_non_writable_changes!(changeset, drop_fields, schema, :insert) wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn -> @@ -526,23 +526,21 @@ defmodule Ecto.Repo.Schema do {:error, put_repo_and_action(changeset, :insert, repo, tuplet)} end - defp nilify_unsurfaced_non_writable_data!(changeset, [], _schema) do + defp detect_unsurfaced_non_writable_data!(changeset, [], _schema) do changeset end - defp nilify_unsurfaced_non_writable_data!(changeset, non_writable_fields, schema) do - updates = Enum.reduce(non_writable_fields, [], fn field, updates -> + defp detect_unsurfaced_non_writable_data!(changeset, non_writable_fields, schema) do + Enum.each(non_writable_fields, fn field -> case changeset.data do %{^field => value} when value != nil -> handle_writable_violation(field, schema, :insert) - - [{field, nil} | updates] %{} -> - updates + :ok end end) - %{changeset | data: struct!(changeset.data, updates)} + changeset end @doc """ diff --git a/test/ecto/repo_test.exs b/test/ecto/repo_test.exs index f8b48fc431..cd90759701 100644 --- a/test/ecto/repo_test.exs +++ b/test/ecto/repo_test.exs @@ -2516,7 +2516,10 @@ defmodule Ecto.RepoTest do end test "insert with surfaced changes on_writable_violation: :nothing saves changes for writable: :always/:insert and ignores changes for writable: :never" do - %{always: 10, never: nil, insert: 12} = + # For surfaced changes from the underlying struct, the value in the returned struct is + # maintained even though the underlying write was not performed, as opposed to "normal" changes + # provided via a changeset. + %{always: 10, never: 11, insert: 12} = %MySchemaWritable{id: 1, always: 10, never: 11, insert: 12} |> Ecto.Changeset.change(%{}) |> TestRepo.insert!() @@ -2537,7 +2540,10 @@ defmodule Ecto.RepoTest do test "insert with surfaced changes and on_writable_violation: :warn saves changes for writable: :always/:insert, ignores changes for writable: :never, and logs a warning" do log = capture_log(fn -> - %{always: 10, never: nil, insert: 12} = + # For surfaced changes from the underlying struct, the value in the returned struct is + # maintained even though the underlying write was not performed, as opposed to "normal" changes + # provided via a changeset. + %{always: 10, never: 11, insert: 12} = %MySchemaWritableWarn{id: 1, always: 10, never: 11, insert: 12} |> Ecto.Changeset.change(%{}) |> TestRepo.insert!() From c74527b1e9a583193d565d8a509efd34e746b527 Mon Sep 17 00:00:00 2001 From: David Green <134172184+green-david@users.noreply.github.com> Date: Wed, 17 Jun 2026 19:42:09 -0400 Subject: [PATCH 11/11] Remove outdated comment --- lib/ecto/repo/schema.ex | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/ecto/repo/schema.ex b/lib/ecto/repo/schema.ex index 157a4575af..6a9e2921ef 100644 --- a/lib/ecto/repo/schema.ex +++ b/lib/ecto/repo/schema.ex @@ -453,9 +453,6 @@ defmodule Ecto.Repo.Schema do # changeset as changes, except the primary key if it is nil. changeset = put_repo_and_action(changeset, :insert, repo, tuplet) changeset = Relation.surface_changes(changeset, struct, keep_fields ++ assocs) - # On insert, we need to nilify non-writable fields in - # the underlying data so that the returned struct reflects - # that the write was not actually performed. changeset = detect_unsurfaced_non_writable_data!(changeset, drop_fields, schema) changeset = drop_non_writable_changes!(changeset, drop_fields, schema, :insert)