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

Top-Level quat passing to query expansions #2420

Merged
merged 1 commit into from
Feb 3, 2022
Merged

Conversation

deusaquilus
Copy link
Collaborator

@deusaquilus deusaquilus commented Feb 3, 2022

In #2381 quill's top-level query expansion using materializeQueryMeta was removed in favor of the existing functionality in ExpandNestedQueries expanding identifiers from their quat information.

That is to say, instead of doing this:

(Id -> Ident, Prop -> Property, Tup -> Tuple)

// materializeQueryMeta
query[Person]
  -> query[Person].map(x => (x.name, x.age)) 
// Parsing
query[Person].map(x => (x.name, x.age))
  -> Map(EntityQuery("Person"), Ident("x"), Tup(Prop(Id("p"),"name"), Prop(Id("p"),"age"))
// SQL Query Tokenization:
// SELECT x.name AS _1, x.age AS _2 FROM Person x

Instead we get the information by expanding the quat like this:

// Parsing
query[Person] 
  -> EntityQuery("Person", Quat.Product("name"->Quat.Value, "age"->Quat.Value))
// Query Expansion
// (QP: Quat.Product("name"->Quat.Value, "age"->Quat.Value) )
  -> EntityQuery("Person", QP) 
  -> Map(EntityQuery("Person", QP), Ident("x"), Tup(Prop(Id("p", QP),"name"), Prop(Id("p", QP),"age"))
// SQL Query Tokenization:
// SELECT x.name AS _1, x.age AS _2 FROM Person x

There are some situations however where the Quat inside the EntityQuery inferred as generic. For example in a query that looks like this:

implicit class QueryOps[Q <: Query[_]](q: Q) {
  def appendFoo = quote(infix"$q APPEND FOO".pure.as[Q])
}
val q = quote(query[Person].appendFoo)

It will get parsed to something like this:

Infix(
  parts = List("", "APPEND FOO"), 
  params = List(EntityQuery("Person", Quat.Product("name"->Quat.Value, "age"->Quat.Value))),
  quat = Quat.Generic
)

Since when the Infix is created the quat type Q is still generic, the Quat in the Infix element will be inferred as Generic. This means that we do not know to expand an identifier downstream of this index into it's corresponding components:

Map(Infix(..., Quat.Generic), Id("p", Quat.Generic), Id("p", Quat.Generic))
// SELECT p.? FROM Person APPEND FOO

This incomplete knowledge resulted in #2403 and would cause other similar issues if left unfixed.

First Solution: Transparent/Generic Infixes

One possible solution is to have a notion of a Transparent infix i.e. infix where the Quat of the Ast inside the params element is treated as the "Real" Quat instead of what is inside the infix. Originally in #2403 I thought to reduce all the Quats of all the params together using the Quat.leastUpperType function meaning that if as in the above case, there was a single params element (i.e. our EntityQuery("Person") it's Quat would be the one that would be swapped into the surrounding infix. This was to be done both in the parser as well as the RepropagateQuats transformation phase via the Quat.improveInfixQuat method.

Unfortunately this solution quite a few edge-cases in which it does not property function. For example, say that the infix has multiple params whose types have nothing to do with one another:

def appendFooFun[Q <: Query[_]] = quote { (q: Q, i: Int) => infix"$q APPEND $i FOO".transparent.pure.as[Q] }
val q = quote { 
  appendFooFun(query[MyPerson], 123) 
}

The infix element will look like this:

Infix(
  parts = List("", "APPEND "," FOO"), 
  params = List(EntityQuery("Person", Quat.Product("name"->Quat.Value, "age"->Quat.Value)), Ident("i", Quat.Value)),
  quat = Quat.Generic
)

When doing a Quat.leastUpperType across Quat.Product("name"->Quat.Value, "age"->Quat.Value) and Ident("i", Quat.Value) you can at best say that it is a Quat.Generic or Quat.Unknown which does not help expand the p.? value in the Select at all. The only place where transparent Infixes actually make sense is in the case where there is only one param. This may be useful to getting better quat-typing in Spark queries in the future (e.g. in #2406) but beyond that it has little value. Something else needs to replace the materializeQueryMeta functionality for these kinds of infixes.

Better Solution: Top Level Quats

The fundamental thing that materializeQueryMeta could was was that for some run(Query[T]) it would compute a .map(t => t.fieldA, t.fieldB, ...). This T is injected at the last moment of the query composition hence any abstract/generic types used in the query are changed to the concrete value of T (or something related to it). This has to be the case because T can only be a CaseClass, Tuple, or decodeable value, otherwise Quill will fail to compile the Query[T].

That means that if we modify the run function (or the quotation right before) to produce a Quat from T, the this "top-level" quat will always represent the final concrete type to which the Query needs to be decoded. This logic in ExpandNestedQueries (particularly in the SelectPropertyProtractor) will look roughly as follows:

Map(Infix(..., Quat.Generic), Id("p", Quat.Generic), Id("p", Quat.Generic)) // + topLevelQuat: QV := Quat.Product("name"->Quat.Value, "age"->Quat.Value)
 -> SqlQuery(SelectValue(Id("p", Quat.Generic)), from: InfixContext(Infix(...))) // SelectQuery.scala functionality
 -> SelectValue(Id("p", Quat.Generic)) -> SelectPropertyProtractor(Id("p", Quat.Generic)) // ExpandNestedQueries.scala
 -> Id("p", Quat.Generic) -> if (p.quat.isAbstract) QV else p.quat -> ProtractQuat(QV) // SelectPropertyProtractor.scala
 -> tokenization, etc....

// SELECT p.name, p.age FROM Person APPEND 123 FOO

That is to say, in the SelectPropertyProtractor if it is detected that an identifier has a generic quat and that identifier is at the top-level of an SQL query, use the topLevelQuat (QV or Quat.Product("name"->Quat.Value, "age"->Quat.Value) in our case above).

Thus since materializeQueryMeta expanded our Query with information only known at the top-level, previous functionality that expanded queries based on Quats had unhandled edge cases. Since the top-level-quat knows everything that materializeQueryMeta knew, they should be a complete replacement.

@deusaquilus deusaquilus merged commit f8bdc4d into master Feb 3, 2022
@deusaquilus deusaquilus deleted the top-level-quat branch February 3, 2022 12:31
jilen pushed a commit that referenced this pull request Jun 11, 2024
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 this pull request may close these issues.

None yet

1 participant