Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.onConflictIgnore and onConflictUpdate breaks batch queries (with .liftQuery) #250

Open
arturaz opened this issue Mar 8, 2023 · 3 comments

Comments

@arturaz
Copy link
Contributor

arturaz commented Mar 8, 2023

Version: 4.6.0.1
Module: quill-sql

If you use .onConflictIgnore and onConflictUpdate on a batch query (with .liftQuery) it complains about a malformed batch query. This functionality worked in Quill 2.

import io.getquill._

case class Inventory(col: Int)

class MyContext extends SqlMirrorContext(MirrorSqlDialect, Literal) {
  extension [T](inline entity: Insert[T]) {
    inline def onConflictIgnoreForBatchQueries: Insert[T] =
      sql"$entity ON CONFLICT DO NOTHING".as[Insert[T]]
  }
}

val ctx = new MyContext()
import ctx.*

// This breaks with: Malformed batch entity
// println(ctx.run(quote {
//   liftQuery(Vector(Inventory(3))).foreach { row =>
//     query[Inventory].insertValue(row).onConflictIgnore
//   }
// }))

// This works.
println(ctx.run(quote {
  liftQuery(Vector(Inventory(3))).foreach { row =>
    query[Inventory].insertValue(row).onConflictIgnoreForBatchQueries
  }
}))

https://scastie.scala-lang.org/0D9gjC8VQ0C2PLX69kxxjw

Workaround

Define an extension and use that instead of .onConflictIgnore.

  extension [T](inline entity: Insert[T]) {
    inline def onConflictIgnoreForBatchQueries: Insert[T] =
      sql"$entity ON CONFLICT DO NOTHING".as[Insert[T]]
  }

I did not manage to find a workaround for .onConflictUpdate except writing the query manually with sql interpolator.

@getquill/maintainers

@arturaz arturaz changed the title .onConflictIgnore breaks batch queries (with .liftQuery) .onConflictIgnore and onConflictUpdate breaks batch queries (with .liftQuery) Mar 8, 2023
@pragmaxim
Copy link

pragmaxim commented Jul 9, 2023

Hi @arturaz, is it possible to "pimp" the existing H2ZioJdbcContext with your workaround? I'm totally new to quill and inlines, having troubles doing that.

@mattdavpir
Copy link

mattdavpir commented Sep 18, 2023

Hey, we've also hit this issue and just to try to provide a bit more context as to why the workaround for onConflictUpdate it doesn't seem to work for us:

object QuillContext extends PostgresZioJdbcContext(SnakeCase):
  final case class Employee(id: Option[Long], employer: String, employeeNumber: Long, name: String)

  inline def employeeQuery =
    querySchema[Employee](
      "employee",
      _.id -> "id",
      _.employer -> "employer",
      _.employeeNumber -> "employer_number",
      _.name -> "name",
    )

  extension (inline employeeInsert: Insert[Employee])
    inline def tempUpsertWorkAround: Insert[Employee] =
      sql"""$employeeInsert
           ON CONFLICT (employer, employee_number) DO UPDATE
           SET name = EXCLUDED.name
         """.as[Insert[Employee]]

  /*
   * Generates:
     Quill Query: INSERT INTO employee (id,employer,employer_number,name) VALUES (?, ?, ?, ?)
             ON CONFLICT (employer, employee_number) DO UPDATE
             SET name = EXCLUDED.name
            RETURNING id
      run(
   */
  def upsertEmployees(employees: Seq[Employee]) =
    run(
      liftQuery(employees)
        .foreach(e =>
          employeeQuery.insertValue(e).tempUpsertWorkAround.returningGenerated(_.id)
        )
    )

  /*
   * Generates 
      Quill Query: INSERT INTO employee (employer,employer_number,name) VALUES (?, ?, ?) RETURNING id
   */
  def insertEmployees(employees: Seq[Employee]) =
    run(
      liftQuery(employees)
        .foreach(e =>
          employeeQuery.insertValue(e).returningGenerated(_.id)
        )
    )

We're doing something a bit similar to the above where a conflict can occur on a non-primary key index, but when we do the workaround we see that a null id is inserted (which violates the primary key's not null constraint) whereas without the onConflict workaround the insert does not try to set the id and successfully returns the generated value.

edit - but actually as a workaround it appears that:

  def upsertEmployees(employees: Seq[Employee]) =
    @nowarn
    implicit inline def meta: InsertMeta[Employee] = insertMeta(_.id)
    run(
      liftQuery(employees)
        .foreach(e =>
          employeeQuery.insertValue(e).tempUpsertWorkAround.returningGenerated(_.id)
        )
    )

is generating the correct query.

@timzaak
Copy link
Contributor

timzaak commented Dec 6, 2023

#394 This may solve, my I don't write test case with returning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants