Upsert should not take an "on_conflict" argument

[rwillians]

Upsert relies under the hood on Insert operation. Unlike the insert operation though, upsert shouldn't take an on_conflict argument because the action of upsert already expected the record to get replaced (updated) if there's a conflict.

https://github.com/TurtleAI/derive/blob/master/lib/derive/ecto/operation.ex#L46

[venkatd]

@rwillians good call. Maybe it isn't good to have an upsert operation anyway. Maybe it's a more obvious API to match the INSERT ... ON CONFLICT. Meaning we deprecate upsert in favor of insert(record, opts \\ []).

[rwillians]

@venkatd hm I'm not sure yet. We can probably keep it this way for now then revisit later.

I'm not sure of how complex the options for "ON CONFLICT" can get. For our current use case, we just need the record to get replaced if there's a conflict targeting the primary keys -- and I remember that part was already covered, I mean, "dynamically" getting the primary key of any model that specifies one (or a composed pk).

So my thinking is that upsert is an op that we use quite often, so I can't see a reason for removing it. Yes it's sugar code, but I think it's a nice small life improvement -- a.k.a DX? -- for us and whoever else wants to use this lib. Just call upsert/1 with a model struct and voilà, let the magic happen.

At the moment, I'd say we can constraint upsert/1 to have a single behavior (replace on conflict in pk) and, if needed, users can use insert/2 and tweak with the opts.

[venkatd]

Makes sense & good point

[venkatd]

Wait, I should also mention that there's a command called replace that does what you're describing haha:

https://github.com/TurtleAI/derive/blob/master/lib/derive/ecto/operation.ex#L68-L74

[rwillians]

ahh I see

In that case I suggest deleting upsert/2 (in favor of insert/2, like you said) and rename replace/1 to upsert/1. The rename is because “replace” is semantically wrong for what that op do: you can only replace something that’s “there”, it doesn’t create anything, it just replaces/updates. “Upsert” is a more appropriate term for that insert or update operation in our field — commonly used term.