AugustNagro / magnum

A 'new look' for database access in Scala
Apache License 2.0
153 stars 10 forks source link

Upsert #29

Closed fdietze closed 1 month ago

fdietze commented 2 months ago

Hey, I really enjoy using magnum!

I was wondering if there is a possibility to implement upsert for Repo? Just wanted to upsert something and realized that it's not easily possbile with only insert or update.

AugustNagro commented 2 months ago

Thanks. You can certainly write an upsert statement with the sql interpolator.

val p = TableInfo[PersonCreator, Person, Long]
val pc = PersonCreator(...)

sql"INSERT INTO $p ${p.insertColumns} values ($pc) ON CONFLICT (${p.id}) DO UPDATE ${p.correlationId} = ${pc.correlationId}"

What's your usecase for wanting such a method in Repo?

AugustNagro commented 2 months ago

And what would the query be?

fdietze commented 2 months ago

I know it's possible in sql, but I wanted to have something more robust against refactoring. Thinking about it, I also realized that the on conflict clause has many ways to resolve the conflict, including:

Modeling all that would probably be too much and for precise control one should use sql. But very often on conflict do nothing is sufficient. How about extending those methods with that use-case?

  def insert(entityCreator: EC)(using DbCon): Unit
  def insertAll(entityCreators: Iterable[EC])(using DbCon): Unit
  def insertReturning(entityCreator: EC)(using DbCon): E
  def insertAllReturning(entityCreators: Iterable[EC])(using DbCon): Vector[E]

(same for update, I guess)

It could be additional methods, like:

  def insertOnConflictDoNothing(entityCreator: EC)(using DbCon): Unit

or a boolean flag:

  def insert(entityCreator: EC, onConflictDoNothing: Boolean = false)(using DbCon): Unit
AugustNagro commented 2 months ago

to have something more robust against refactoring

Can you not use TableInfo ( https://github.com/AugustNagro/magnum/?tab=readme-ov-file#future-proof-queries )? The goal of that abstraction is so you CAN write SQL queries that are refactorable. See my example above.

But very often on conflict do nothing is sufficient. How about extending those methods with that use-case

So, that would be a significant change from the default behavior. We can't do that. UPSERT is also not supported on many DBs other than Postgres. Finally, it should be very rare that you actually need to do an upsert. That's why I was asking about your use case earlier.. most of the time you can refactor your schema to a better design which doesn't require ON CONFLICT.

On Sat, Jun 29, 2024 at 9:45 AM Felix Dietze @.***> wrote:

I know it's possible in sql, but I wanted to have something more robust against refactoring. Thinking about it, I also realized that the on conflict clause has many ways to resolve the conflict, including:

  • selecting the conflicting fields
  • deciding what to do when a conflict happens

Modeling all that would probably be too much and for precise control one should use sql. But very often on conflict do nothing is sufficient. How about extending those methods with that use-case?

def insert(entityCreator: EC)(using DbCon): Unit def insertAll(entityCreators: Iterable[EC])(using DbCon): Unit def insertReturning(entityCreator: EC)(using DbCon): E def insertAllReturning(entityCreators: Iterable[EC])(using DbCon): Vector[E]

(same for update, I guess)

It could be additional methods, like:

def insertOnConflictDoNothing(entityCreator: EC)(using DbCon): Unit

or a boolean flag:

def insert(entityCreator: EC, onConflictDoNothing: Boolean = false)(using DbCon): Unit

— Reply to this email directly, view it on GitHub https://github.com/AugustNagro/magnum/issues/29#issuecomment-2198257901, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQOKNW2PZVH2AJX2IV2JXDZJ3QEPAVCNFSM6AAAAABJOIAN6GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJYGI2TOOJQGE . You are receiving this because you commented.Message ID: @.***>

fdietze commented 1 month ago

Thank you for your answer.

Can you not use TableInfo? The goal of that abstraction is so you CAN write SQL queries that are refactorable. See my example above.

I absolutely can use TableInfo, yes. But I was hoping for a simpler way. I already used an insert with an entity creator and then wanted to turn it into an upsert and hoped that there was a simpler solution than writing out the query.

My use-case was to place items at a geo-location create table item_location (item_id, lat, lon). Since the location is optional, putting an item at a location either means inserting or updating.

Different DB support is a very good argument to not implement it in this library, though. I understand now that it would complicate this library too much and it's better so leave it as simple as it is right now :+1:.

closing...