From 9769e9939890f4f4dcabe08145479da8a44049bb Mon Sep 17 00:00:00 2001 From: DohanKim Date: Wed, 3 Dec 2025 15:25:00 +0900 Subject: [PATCH] do not build query filter with or: [] when all records are successfully inserted, for return_skipped_upsert?: true --- lib/data_layer.ex | 67 +++++++++++++++++++++------------------ test/bulk_create_test.exs | 28 ++++++++++++++++ 2 files changed, 65 insertions(+), 30 deletions(-) diff --git a/lib/data_layer.ex b/lib/data_layer.ex index 095f2f62..c2161ae2 100644 --- a/lib/data_layer.ex +++ b/lib/data_layer.ex @@ -2059,39 +2059,46 @@ defmodule AshPostgres.DataLayer do {Map.take(r, keys), r} end) - ash_query = - resource - |> Ash.Query.do_filter( - or: - changesets - |> Enum.filter(fn changeset -> - not Map.has_key?( - results_by_identity, - Map.take(changeset.attributes, keys) - ) - end) - |> Enum.map(fn changeset -> - changeset.attributes - |> Map.take(keys) - |> Keyword.new() - end) - ) - |> then(fn - query when is_nil(identity) or is_nil(identity.where) -> query - query -> Ash.Query.do_filter(query, identity.where) + skipped_filter = + changesets + |> Enum.filter(fn changeset -> + not Map.has_key?( + results_by_identity, + Map.take(changeset.attributes, keys) + ) + end) + |> Enum.map(fn changeset -> + changeset.attributes + |> Map.take(keys) + |> Keyword.new() end) - |> Ash.Query.set_tenant(changeset.tenant) skipped_upserts = - with {:ok, ecto_query} <- Ash.Query.data_layer_query(ash_query), - {:ok, results} <- run_query(ecto_query, resource) do - results - |> Enum.map(fn result -> - Ash.Resource.put_metadata(result, :upsert_skipped, true) - end) - |> Enum.reduce(%{}, fn r, acc -> - Map.put(acc, Map.take(r, keys), r) - end) + case skipped_filter do + [] -> + # No skipped records to query for + %{} + + skipped_filter -> + ash_query = + resource + |> Ash.Query.do_filter(or: skipped_filter) + |> then(fn + query when is_nil(identity) or is_nil(identity.where) -> query + query -> Ash.Query.do_filter(query, identity.where) + end) + |> Ash.Query.set_tenant(changeset.tenant) + + with {:ok, ecto_query} <- Ash.Query.data_layer_query(ash_query), + {:ok, results} <- run_query(ecto_query, resource) do + results + |> Enum.map(fn result -> + Ash.Resource.put_metadata(result, :upsert_skipped, true) + end) + |> Enum.reduce(%{}, fn r, acc -> + Map.put(acc, Map.take(r, keys), r) + end) + end end results = diff --git a/test/bulk_create_test.exs b/test/bulk_create_test.exs index c18d8d1a..bb0de61f 100644 --- a/test/bulk_create_test.exs +++ b/test/bulk_create_test.exs @@ -237,6 +237,34 @@ defmodule AshPostgres.BulkCreateTest do refute Ash.Resource.get_metadata(no_conflict, :upsert_skipped) end + test "bulk upsert with return_skipped_upsert? when all records are not skipped" do + %Ash.BulkResult{records: records, errors: errors} = + Ash.bulk_create!( + [ + %{title: "fredfoo", uniq_if_contains_foo: "1foo", price: 100}, + %{title: "georgefoo", uniq_if_contains_foo: "2foo", price: 200} + ], + Post, + :upsert_with_no_filter, + upsert_condition: expr(price != upsert_conflict(:price)), + return_errors?: true, + return_records?: true, + return_skipped_upsert?: true + ) + + assert errors == [] + + assert [row1, row2] = records + + assert row1.title == "fredfoo" + assert row1.price == 100 + refute Ash.Resource.get_metadata(row1, :upsert_skipped) + + assert row2.title == "georgefoo" + assert row2.price == 200 + refute Ash.Resource.get_metadata(row2, :upsert_skipped) + end + # confirmed that this doesn't work because it can't. An upsert must map to a potentially successful insert. # leaving this test here for posterity # test "bulk creates can upsert with id" do