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

Continue solving sheathing issues in infix subqueries #2387

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -153,20 +153,20 @@ object SeedRenames extends StatelessTransformer {
// Represents a nested property path to an identity i.e. Property(Property(... Ident(), ...))
object PropertyMatroshka {

def traverse(initial: Property): Option[(Ast, List[String])] =
def traverse(initial: Property): Option[(Ast, List[String], List[Renameable])] =
initial match {
// If it's a nested-property walk inside and append the name to the result (if something is returned)
case Property(inner: Property, name) =>
traverse(inner).map { case (id, list) => (id, list :+ name) }
case Property.Opinionated(inner: Property, name, ren, _) =>
traverse(inner).map { case (id, list, renameable) => (id, list :+ name, renameable :+ ren) }
// If it's a property with ident in the core, return that
case Property(inner, name) if !inner.isInstanceOf[Property] =>
Some((inner, List(name)))
case Property.Opinionated(inner, name, ren, _) if !inner.isInstanceOf[Property] =>
Some((inner, List(name), List(ren)))
// Otherwise an ident property is not inside so don't return anything
case _ =>
None
}

def unapply(ast: Property): Option[(Ast, List[String])] =
def unapply(ast: Property): Option[(Ast, List[String], List[Renameable])] =
ast match {
case p: Property => traverse(p)
case _ => None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ object RepropagateQuats extends StatelessTransformer {
case OnConflict.Properties(props) =>
val propsR = props.map {
// Recreate the assignment with new idents but only if we need to repropagate
case prop @ PropertyMatroshka(ident: Ident, _) =>
case prop @ PropertyMatroshka(ident: Ident, _, _) =>
trace"Repropagate OnConflict.Properties Quat ${oca.quat.suppress(msg)} from $oca into:" andReturn
BetaReduction(prop, RWR, ident -> ident.retypeQuatFrom(oca.quat)).asInstanceOf[Property]
case other =>
Expand Down
33 changes: 31 additions & 2 deletions quill-engine/src/main/scala/io/getquill/sql/idiom/SqlIdiom.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import io.getquill.norm.ConcatBehavior.AnsiConcat
import io.getquill.norm.EqualityBehavior.AnsiEquality
import io.getquill.norm.{ ConcatBehavior, EqualityBehavior, ExpandReturning, NormalizeCaching, ProductAggregationToken }
import io.getquill.quat.Quat
import io.getquill.sql.norm.RemoveExtraAlias.TopLevelRemove
import io.getquill.sql.norm.{ RemoveExtraAlias, RemoveUnusedSelects }
import io.getquill.util.{ Interleave, Messages }
import io.getquill.util.Messages.{ fail, trace }
Expand Down Expand Up @@ -84,15 +85,43 @@ trait SqlIdiom extends Idiom {
def token(v: Ast) = stableTokenizer.token(v)
}

private[getquill] def sheathQuery(q: Query) =
q match {
case Quat.Is(Quat.Product(values)) =>
val id = Ident("x", q.quat)
val keyValues =
values.map {
case (k, quat) => (k, Property(id, k))
}.toList
Map(q, id, CaseClass(keyValues))

case Map(query, id, prop @ Property(_, propName)) =>
Map(query, id, CaseClass(List((propName, prop))))

case _ => q
}

private[getquill] def expandNestedSubQuery(q: Query, strategy: NamingStrategy) = {
val transformed = normalizeAst(q, concatBehavior, equalityBehavior)
val nestedExpanded = ExpandNestedQueries(SqlQuery(transformed))
RemoveExtraAlias(strategy, topLevel = TopLevelRemove.OnlyMatching)(nestedExpanded)
}

def astTokenizer(implicit astTokenizer: Tokenizer[Ast], strategy: NamingStrategy): Tokenizer[Ast] =
Tokenizer[Ast] {
case a: Query =>
// This case almost exclusively happens when you have a select inside of an insert.
// This case typically happens when you have a select inside of an insert
// infix or a set operation (e.g. query[Person].exists).
// have a look at the SqlDslSpec `forUpdate` and `insert with subselects` tests
// for more details.
// Right now we are not removing extra select clauses here (via RemoveUnusedSelects) since I am not sure what
// kind of impact that could have on selects. Can try to do that in the future.
RemoveExtraAlias(strategy)(ExpandNestedQueries(SqlQuery(a))).token
if (Messages.querySubexpand) {
val sheathed = sheathQuery(a)
val expanded = expandNestedSubQuery(sheathed, strategy)
expanded.token
} else
SqlQuery(a).token

case a: Operation => a.token
case a: Infix => a.token
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import io.getquill.norm.PropertyMatroshka

class ExpandSelection(from: List[FromContext]) {

def apply(values: List[SelectValue]): List[SelectValue] =
values.flatMap(apply(_))
def apply(values: List[SelectValue]): List[SelectValue] = {
val vls = values.flatMap(apply(_))
vls
}

implicit class AliasOp(alias: Option[String]) {
def concatWith(str: String): Option[String] =
Expand All @@ -22,24 +24,26 @@ class ExpandSelection(from: List[FromContext]) {
// the beta reduction would have unrolled them already
case SelectValue(ast @ PropertyOrCore(), alias, concat) =>
val exp = SelectPropertyProtractor(from)(ast)
exp.map {
case (p: Property, Nil) =>
// If the quat-path is nothing and there is some pre-existing alias (e.g. if we came from a case-class or quat)
// the use that. Otherwise the selection is of an individual element so use the element name (before the rename)
// as the alias.
alias match {
case None =>
SelectValue(p, Some(p.prevName.getOrElse(p.name)), concat)
case Some(value) =>
SelectValue(p, Some(value), concat)
}
case (p: Property, path) =>
// Append alias headers (i.e. _1,_2 from tuples and field names foo,bar from case classes) to the
// value of the Quat path
SelectValue(p, alias.concatWith(path.mkString), concat)
case (other, _) =>
SelectValue(other, alias, concat)
}
val mapped =
exp.map {
case (p: Property, Nil) =>
// If the quat-path is nothing and there is some pre-existing alias (e.g. if we came from a case-class or quat)
// the use that. Otherwise the selection is of an individual element so use the element name (before the rename)
// as the alias.
alias match {
case None =>
SelectValue(p, Some(p.prevName.getOrElse(p.name)), concat)
case Some(value) =>
SelectValue(p, Some(value), concat)
}
case (p: Property, path) =>
// Append alias headers (i.e. _1,_2 from tuples and field names foo,bar from case classes) to the
// value of the Quat path
SelectValue(p, alias.concatWith(path.mkString), concat)
case (other, _) =>
SelectValue(other, alias, concat)
}
mapped
case SelectValue(Tuple(values), alias, concat) =>
values.zipWithIndex.flatMap {
case (ast, i) =>
Expand All @@ -48,6 +52,7 @@ class ExpandSelection(from: List[FromContext]) {
case SelectValue(CaseClass(fields), alias, concat) =>
fields.flatMap {
case (name, ast) =>
val concated = SelectValue(ast, alias.concatWith(name), concat)
apply(SelectValue(ast, alias.concatWith(name), concat))
}
// Direct infix select, etc...
Expand All @@ -61,7 +66,9 @@ object ExpandNestedQueries extends StatelessQueryTransformer {
protected override def apply(q: SqlQuery, isTopLevel: Boolean = false): SqlQuery =
q match {
case q: FlattenSqlQuery =>
expandNested(q.copy(select = new ExpandSelection(q.from)(q.select))(q.quat), isTopLevel)
val selection = new ExpandSelection(q.from)(q.select)
val out = expandNested(q.copy(select = selection)(q.quat), isTopLevel)
out
case other =>
super.apply(q, isTopLevel)
}
Expand All @@ -71,10 +78,11 @@ object ExpandNestedQueries extends StatelessQueryTransformer {

def apply(p: Ast): Ast = {
p match {
case p @ PropertyMatroshka(inner, path) =>
case p @ PropertyMatroshka(inner, path, renameables) =>
val isSubselect = inContext.isSubselect(p)
val propsAlreadyFixed = renameables.forall(_ == Renameable.Fixed)
val renameable =
if (p.prevName.isDefined || isSubselect)
if (p.prevName.isDefined || isSubselect || propsAlreadyFixed)
Renameable.Fixed
else
Renameable.ByStrategy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ object RemoveUnusedSelects {

private def filterUnused(select: List[SelectValue], references: Set[Property]): List[SelectValue] = {
val usedAliases = references.map {
case PropertyMatroshka(_, list) => list.mkString
case PropertyMatroshka(_, list, _) => list.mkString
}.toSet
select.filter(sv =>
sv.alias.forall(aliasValue => usedAliases.contains(aliasValue)) ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ case class InContext(from: List[FromContext]) {
def contextReferenceType(ast: Ast) = {
val references = collectTableAliases(from)
ast match {
case Ident(v, _) => references.get(v)
case PropertyMatroshka(Ident(v, _), _) => references.get(v)
case _ => None
case Ident(v, _) => references.get(v)
case PropertyMatroshka(Ident(v, _), _, _) => references.get(v)
case _ => None
}
}

Expand Down Expand Up @@ -104,7 +104,7 @@ case class SelectPropertyProtractor(from: List[FromContext]) {
}
// Assuming a property contains only an Ident, Infix or Constant at this point
// and all situations where there is a case-class, tuple, etc... inside have already been beta-reduced
case prop @ PropertyMatroshka(id @ Core(), _) =>
case prop @ PropertyMatroshka(id @ Core(), _, _) =>
val isEntity = inContext.isEntityReference(id)
prop.quat match {
case p: Quat.Product =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ object Messages {
def traceAstSimple = cache("quill.trace.ast.simple", variable("quill.trace.ast.simple", "quill_trace_ast_simple", "false").toBoolean)
def traceQuats = cache("quill.trace.quat", QuatTrace(variable("quill.trace.quat", "quill_trace_quat", QuatTrace.None.value)))
def cacheDynamicQueries = cache("quill.query.cacheDaynamic", variable("quill.query.cacheDaynamic", "query_query_cacheDaynamic", "true").toBoolean)
def querySubexpand = cache("quill.query.subexpand", variable("quill.query.subexpand", "query_query_subexpand", "true").toBoolean)
def quillLogFile = cache("quill.log.file", LogToFile(variable("quill.log.file", "quill_log_file", "false")))

sealed trait LogToFile
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ class SqlDslSpec extends Spec {
testContext.run(q).string mustEqual "SELECT t.s, t.i, t.l, t.o, t.b FROM TestEntity t WHERE t.s = 'a' FOR UPDATE"
}

"forUpdate naming schema" in {

val q: Quoted[Query[TestEntity]] = quote {
query[TestEntity].filter(t => t.s == "a").forUpdate
}
testContext.run(q).string mustEqual "SELECT t.s, t.i, t.l, t.o, t.b FROM TestEntity t WHERE t.s = 'a' FOR UPDATE"
}

case class Person(name: String, age: Int)

"insert with subselects" - {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ package io.getquill.context.sql.norm
import io.getquill.ReturnAction.{ ReturnColumns, ReturnRecord }
import io.getquill.context.sql.testContextUpper
import io.getquill.context.sql.testContextUpper._
import io.getquill.{ MirrorSqlDialectWithReturnClause, Spec }
import io.getquill.Query
import io.getquill.{ EntityQuery, MirrorSqlDialectWithReturnClause, Query, Quoted, Spec }

class RenamePropertiesOverrideSpec extends Spec {

Expand Down Expand Up @@ -321,9 +320,9 @@ class RenamePropertiesOverrideSpec extends Spec {
}
}

"operation" - {
"operation" - { //
"unary" in {
val q = quote {
val q: Quoted[EntityQuery[Index]] = quote {
e.filter(a => e.filter(b => b.i > 0).isEmpty).map(_.i)
}
testContextUpper.run(q).string mustEqual
Expand Down