Support `ON CONFLICT DO NOTHING` when `upsert_fields` is empty

NduatiK commented 3 weeks ago

Describe the bug According to Zach from 1 year ago Ecto's on_conflict: :nothing is not supported in Ash. From conversations in Discord, it seems like the implication of the statement is that upsert_fields [] should cause it to, on an update, update nothing.

However, this it isn't what I see in practice with AshPostgres.

After doing some code splunking, I think that when the upsert_fields is [], the data layer overwrites the upsert_keys (the identity used for the upsert) or the primary key with their current value.

To Reproduce

create :create do
  accept [:serial_no]
  upsert? true
  upsert_identity :unique_serial_no
  upsert_fields []
end

Will produce the query (note the DO UPDATE):

INSERT INTO "items" ("serial_no") VALUES ($1),($2) ON CONFLICT ("serial_no") DO UPDATE SET "serial_no" = EXCLUDED."serial_no" RETURNING "serial_no","id" ["87a0672", "000000"]

Expected behavior

While this is usually fine (we are overwriting a column with the same value it had before); however, in my case, it was causing PG deadlocks because (I think) ON CONFLICT DO UPDATES require row locking on existing rows while ON CONFLICT DO NOTHING does not (since it only inserts rows that don't exist). I would expect to have Ash generate ON CONFLICT DO NOTHING clauses since using the UPDATES when doing a lot of reads/writes the distinction has implications on database locking and therefore performance.

Additional context This might also apply to AshSqlite

zachdaniel commented 3 weeks ago

Alright, so I've added some tools that could be used to handle these issues. You will need to use ash main and ash_postgres main until they are released.

less transactions in general.

You can get this by defining

def prefer_transactions?, do: false

in your repo module.

A way to get `ON CONFLICT DO NOTHING` in your upserts

For this one, you'll want to use Ash.bulk_create instead of Ash.create. Ash.bulk_create allows specifying not to return records, which will get you an on_conflict :nothing.

The first thing might just get you what you need though, you may not need this. Keep in mind that passing notify?: true to Ash.bulk_create will force returning records.

zachdaniel commented 3 weeks ago

If the above does not resolve the issue, LMK :)

NduatiK commented 3 weeks ago

Been busy, I'll check it out and get back to you.

On Mon, Oct 28, 2024, 3:51 PM Zach Daniel @.***> wrote:

If the above does not resolve the issue, LMK :)

— Reply to this email directly, view it on GitHub https://github.com/ash-project/ash_postgres/issues/411#issuecomment-2441503186, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG36YP3BFOWYCPL66EYSEH3Z5YXMDAVCNFSM6AAAAABQVXUNYWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBRGUYDGMJYGY . You are receiving this because you authored the thread.Message ID: @.***>

ash-project / ash_postgres