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

Reducing list of queries (dynamic) #318

Open
ex0ns opened this issue Aug 30, 2023 · 2 comments · May be fixed by #398
Open

Reducing list of queries (dynamic) #318

ex0ns opened this issue Aug 30, 2023 · 2 comments · May be fixed by #398

Comments

@ex0ns
Copy link

ex0ns commented Aug 30, 2023

We have a part of the code that apply a filter on a dynamic query, for that we use a map that looks like that

val regions = nameFilter.map(name => quote(p.name.like(lift(name))))

But only the last element of the nameFilter is kept (it is actually duplicated). In the reproducer I have remove the like part as the behavior is the same with == as well.

I don't have enough knowledge about quill's internal but it seems that the variable that is capture is always the latest in the list, but I can not determine if this is coming from the map or the reduce

Version: 4.6.0.1
Module: sql
Database: all

Expected behavior

This should output the following query:

SELECT v0.name FROM Custom v0 WHERE v0.name = 'a' OR v0.name = 'b'

Actual behavior

Steps to reproduce the behavior

//> using scala "3.3"
//> using dep io.getquill::quill-sql::4.6.0.1

import io.getquill.*

@main
def main = 
  case class Custom(name: String)
  val nameFilter = Seq("a", "b")

  val ctx = new SqlMirrorContext(MirrorSqlDialect, Literal)
  import ctx._

  inline def selectName(p: Quoted[Custom]) =
    val regions = nameFilter.map(name => quote(p.name == lift(name)))
    regions.reduce((l, r) => quote(l || r))

  val query = dynamicQuery[Custom].filter: p =>
    selectName(p)

  println(ctx.translate(query))
  // # SELECT v0.name FROM Custom v0 WHERE v0.name = 'b' OR v0.name = 'b'

Workaround

None found yet, but maybe an infix operator could fix that.

More infos

When printing the list of generated queries, I get the following:

List(QuotedId(
  BinaryOperation(quoted(e16867a0-faad-490e-8182-31122b8362f7).name, _==, ScalarTag("74f64d15-44dc-4bc6
-a7dc-ac6eb8b47da3", Parser)),
  List(
    EagerPlanter(
      "a",
      MirrorEncoder(io.getquill.context.mirror.MirrorEncoders$$Lambda$13/0x00000008000fc440@221a3fa4),
      "74f64d15-44dc-4bc6-a7dc-ac6eb8b47da3"
    )
  ),
  List(QuotationVase(QuotedId(Id("v0"), List(), List()), "e16867a0-faad-490e-8182-31122b8362f7"))
), QuotedId(
  BinaryOperation(quoted(e16867a0-faad-490e-8182-31122b8362f7).name, _==, ScalarTag("74f64d15-44dc-4bc6
-a7dc-ac6eb8b47da3", Parser)),
  List(
    EagerPlanter(
      "b",
      MirrorEncoder(io.getquill.context.mirror.MirrorEncoders$$Lambda$13/0x00000008000fc440@221a3fa4),
      "74f64d15-44dc-4bc6-a7dc-ac6eb8b47da3"
    )
  ),
  List(QuotationVase(QuotedId(Id("v0"), List(), List()), "e16867a0-faad-490e-8182-31122b8362f7"))
))

And as far as I can tell, the a and b seems to be correct here however (I don't know if this is relevant or not) the ScalarTag 74f64d15-44dc-4bc6-a7dc-ac6eb8b47da3 is the same in both cases

My guess is that as we have a single lift (as opposed to liftQuery) for the collection, we are considering the last lift
@getquill/maintainers

@timzaak
Copy link
Contributor

timzaak commented Dec 7, 2023

QueryExecution.scala => processLifts function has bugs to handle the same quote used more than one time.

@ex0ns
Copy link
Author

ex0ns commented Dec 8, 2023

I wanted to dig a bit deeper in order to solve this issue but I could not find the time to, our solution (for those interested) is pretty pretty ugly and might failed in other setup is to manually update the UUID so they are unique in the final query:

//> using scala "3.3"
//> using dep io.getquill::quill-sql::4.8.0


import io.getquill.*
import io.getquill.ast.*

@main
def main = 
  case class Custom(name: String)
  val names = Seq("a", "b")

  val ctx = new SqlMirrorContext(MirrorSqlDialect, Literal)
  import ctx.*

  def createNameFilters(names: Seq[String])(p: Quoted[Custom]) =
    val nameFilters = names.map(name => quote(p.name == lift(name)))
    // val nameFilters = names.map(name => quote(p.name.like(lift(name))))
    val uuids = names.map(_ => java.util.UUID.randomUUID.toString)

    // this part should be way more robust, a better way of doing that would be to:
    // 1. retrieve all the current `lifts` (filters.map(_.lifts))
    // 2. keep a map of old uuid => new uuid
    // 3. Traverse the AST and replace old uuid by new uuid
    val filters: Seq[Quoted[Boolean]] = nameFilters.zip(uuids).map: (filter, uuid) =>
      filter.copy(
        ast = filter.ast match 
          case infix @ Infix(_, _ @head :: (last: ScalarTag) :: Nil , _, _, _) =>
            infix.copy(params = head :: last.copy(uid = uuid) :: Nil)
          case bop @ BinaryOperation(_, _ , op: ScalarTag) =>
            bop.copy(b = op.copy(uid = uuid))
          case other => other
        ,
        lifts = filter.lifts.map:
          case lift: EagerPlanter[_, _, _] => lift.copy(uid = uuid)
          case other => other
      )

    filters.reduce((l, r) => quote(l || r))

  val filters = createNameFilters(names)
  val query = dynamicQuery[Custom].filter(p => filters(p))

  println(ctx.translate(query))
  // # SELECT v0.name FROM Custom v0 WHERE v0.name = 'a' OR v0.name = 'b'

I would really not recommend using this code, but this might give some hints on the fix

timzaak added a commit to timzaak/zio-protoquill that referenced this issue Dec 8, 2023
@timzaak timzaak linked a pull request Dec 8, 2023 that will close this issue
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

Successfully merging a pull request may close this issue.

2 participants