Upserting using insert on conflict

[garymardell]

I was looking into upsert for something I am working on and noticed that it is actually a find followed by create or update rather than a "true" upsert. This means it is prone to race conditions as we may have inserted a conflicting record between the find and the subsequent action.

For my use-case ideally I want to use INSERT ... ON CONFLICT UPDATE SET foo=$1 to do the upsert in a single query. The downside is that we wouldn't run any of the after callbacks as we will no longer know whether it was an update or a save. This matches the behaviour provided by other frameworks like Rails (https://apidock.com/rails/v6.0.0/ActiveRecord/Persistence/ClassMethods/upsert).

Looking for feedback whether this change would be accepted into Avram and along those lines should it replace the existing upsert method (although this would be a breaking change due to callbacks) or be a new API entirely?

[jwoertink]

Related: https://github.com/luckyframework/avram/issues/790

I'd definitely like to add the ON CONFLICT portion. I know we use a ton of upserts in my app, but we also rely on the after callbacks to run, so that would make things a bit tricky and become a breaking change.

The other thing is we have a method available to us that tells us if the record was created or updated https://github.com/luckyframework/avram/pull/904

Here's the original thread where we talked about building it as a normal upsert, but deciding to go a different direction https://github.com/luckyframework/avram/issues/298

I'm not opposed to it, but I think we have to also consider any potential breaking changes, or what we lose on (e.g. do we lose any type-safety? etc...)

[jwoertink]

I keep thinking about this and coming back to it. Maybe @grepsedawk was right about a rename... It would still be a breaking change which isn't good, but the current upsert could be renamed to create_or_update, then a new upsert could be added...

Where things get fuzzy is I don't really like the idea of having a method on SaveOperation that doesn't perform exactly like a SaveOperation... So if this upsert can't use updated? or created? methods... or it can't do after_save and after_commit callbacks, then I feel like it would need to be a different operation. UpsertOperation or something. Then there's the added level of complexity when you're dealing with several different types of operations just trying to get the same result :confused:

Maybe we can shine the @paulcsmith signal in the air and get some extra thoughts on this? :smile:

[grepsedawk]

I think this ticket should be JUST a rename ticket and we should make a new ticket for "add upsert", to reduce confusion moving forward.

[paulcsmith]

I like the idea of renaming it and adding a new upsert. It's been long enough that I don't quite remember the complexities of adding upsert, but it would be wonderful to keep it type-safe if possible. I also agree that having it working differently and not runing callbacks would be quite odd. I think if it didn't it would have to be a new operation, or would need to raise at compile time if you try to use any of those methods if you're using upsert (for example, if you add conflict_keys macro as described in https://github.com/luckyframework/avram/issues/298#issuecomment-584393698 maybe we also add callbacks methods that raise). Personally I don't love the idea of raising if you use on conflict but wanted to present as an option

I'm not sure this is helpful at all though 😆LMK if you have other questions and I'll try to help if I can.

[jwoertink]

Just came across this post which mentions a way to know if a record was inserted or not:

db=# WITH new_employees AS (
    SELECT * FROM (VALUES
        ('George', 'Sales',    'Manager',   1000),
        ('Jane',   'R&D',      'Developer', 1200)
    ) AS t(
         name,      department, role,       salary
    )
)
INSERT INTO employees (name, department, role, salary)
SELECT name, department, role, salary
FROM new_employees
ON CONFLICT (name) DO UPDATE SET
    department = EXCLUDED.department,
    role = EXCLUDED.role,
    salary = EXCLUDED.salary
RETURNING *, (xmax = 0) AS inserted;

  name  │ department │   role    │ salary │ inserted
────────┼────────────┼───────────┼────────┼──────────
 Jane   │ R&D        │ Developer │   1200 │ t
 George │ Sales      │ Manager   │   1000 │ f
INSERT 0 2

It's a little funky, and I think it would require adding some special property to models for it to work, but maybe this will spark some ideas.

[jwoertink]

Just came across a new NEW post that suggest using MERGE instead of insert with on conflict https://dev.to/rozhnev/merge-in-postgresql-15-2o7f

This may be a way to keep the current upsert but then add a new operation method merge that is much better in the end. It would require Postgres v15 as a minimum. v15 was released in October 2022, so that may be a hard sell.