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

Change Quat-Verification to Strict in build. Fix Spark failure cases. #2406

Open
wants to merge 3 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
2 changes: 1 addition & 1 deletion build/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export CASSANDRA_DC=datacenter1
export ORIENTDB_HOST=127.0.0.1
export ORIENTDB_PORT=12424

export JVM_OPTS="-Dquill.macro.log=false -Dquill.scala.version=$SCALA_VERSION -Xms1024m -Xmx3g -Xss5m -XX:ReservedCodeCacheSize=256m -XX:+TieredCompilation -XX:+CMSClassUnloadingEnabled -XX:+UseConcMarkSweepGC"
export JVM_OPTS="-Dquill.macro.log=false -Dquill.quat.strict=true -Dquill.scala.version=$SCALA_VERSION -Xms1024m -Xmx3g -Xss5m -XX:ReservedCodeCacheSize=256m -XX:+TieredCompilation -XX:+CMSClassUnloadingEnabled -XX:+UseConcMarkSweepGC"

modules=$1
echo "Start build modules: $modules"
Expand Down
12 changes: 6 additions & 6 deletions quill-core/src/main/scala/io/getquill/quotation/Parsing.scala
Original file line number Diff line number Diff line change
Expand Up @@ -320,17 +320,17 @@ trait Parsing extends ValueComputation with QuatMaking {

val infixParser: Parser[Ast] = Parser[Ast] {
case q"$infix.pure.asCondition" =>
combinedInfixParser(true, Quat.BooleanExpression)(infix)
Quat.improveInfixQuat(combinedInfixParser(true, Quat.BooleanExpression)(infix))
case q"$infix.asCondition" =>
combinedInfixParser(false, Quat.BooleanExpression)(infix)
Quat.improveInfixQuat(combinedInfixParser(false, Quat.BooleanExpression)(infix))
case q"$infix.generic.pure.as[$t]" =>
combinedInfixParser(true, Quat.Generic)(infix)
Quat.improveInfixQuat(combinedInfixParser(true, Quat.Generic)(infix))
case q"$infix.pure.as[$t]" =>
combinedInfixParser(true, inferQuat(q"$t".tpe))(infix)
Quat.improveInfixQuat(combinedInfixParser(true, inferQuat(q"$t".tpe))(infix))
case q"$infix.as[$t]" =>
combinedInfixParser(false, inferQuat(q"$t".tpe))(infix)
Quat.improveInfixQuat(combinedInfixParser(false, inferQuat(q"$t".tpe))(infix))
case `impureInfixParser`(value) =>
value
Quat.improveInfixQuat(value)
}

val impureInfixParser = combinedInfixParser(false, Quat.Value) // TODO Verify Quat in what cases does this come up?
Expand Down
36 changes: 36 additions & 0 deletions quill-core/src/test/scala/io/getquill/quat/QuatSpec.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package io.getquill.quat

import io.getquill._
import io.getquill.norm.RepropagateQuats
import io.getquill.quotation.QuatException

import scala.language.reflectiveCalls

class QuatSpec extends Spec {
Expand Down Expand Up @@ -30,6 +32,40 @@ class QuatSpec extends Spec {
quote(query[MyPerson]).ast.quat mustEqual MyPersonQuat
}

"should refine quats from generic infixes" - {
case class MyPerson(name: String, age: Int)
val MyPersonQuat = Quat.Product("name" -> Quat.Value, "age" -> Quat.Value)

"from extension methods" in {
implicit class QueryOps[Q <: Query[_]](q: Q) {
def appendFoo = quote(infix"$q APPEND FOO".pure.as[Q])
}
val q = quote(query[MyPerson].appendFoo)
q.ast.quat mustEqual MyPersonQuat // I.e ReifyLiftings runs RepropagateQuats to take care of this
}

"from extension methods - generic marker" in {
implicit class QueryOps[Q <: Query[_]](q: Q) {
def appendFoo = quote(infix"$q APPEND FOO".generic.pure.as[Q])
}
val q = quote(query[MyPerson].appendFoo)
q.ast.quat mustEqual MyPersonQuat // I.e ReifyLiftings runs RepropagateQuats to take care of this
}

"should support query-ops function" in {
def appendFooFun[Q <: Query[_]] = quote { (q: Q) => infix"$q APPEND FOO".pure.as[Q] }
val q = quote(appendFooFun(query[MyPerson]))
q.ast.quat mustEqual MyPersonQuat
}

"should support query-ops function - dynamic function" in {
def appendFooFun[Q <: Query[_]]: Quoted[Q => Q] = quote { (q: Q) => infix"$q APPEND FOO".pure.as[Q] }
val q = quote(appendFooFun(query[MyPerson]))
q.ast.quat mustEqual Quat.Unknown
println(io.getquill.util.Messages.qprint(testContext.run(q)))
}
}

"should support multi-level embedded" in {
case class MyName(first: String, last: String) extends Embedded
case class MyId(name: MyName, memberNum: Int) extends Embedded
Expand Down
16 changes: 10 additions & 6 deletions quill-engine/src/main/scala/io/getquill/AstPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class AstPrinter(traceOpinions: Boolean, traceAstSimple: Boolean, traceQuats: Qu
}
def withQuat(q: Quat): treemake = toContent.andWith(treemake.Quat(q))
def withTree(t: pprint.Tree): treemake = toContent.andWith(treemake.Tree(t))
def withElem(a: Any): treemake = toContent.andWith(treemake.Elem(a))
private def treeifyList: List[Tree] =
toContent.list.flatMap {
case e: treemake.Quat =>
Expand All @@ -61,11 +62,11 @@ class AstPrinter(traceOpinions: Boolean, traceAstSimple: Boolean, traceQuats: Qu
case QuatTrace.Short => List(Tree.Literal(e.q.shortString.take(10)))
case QuatTrace.None => List()
}
case treemake.Elem(value) => List(pprint.treeify(value))
case treemake.Elem(value) => List(treeify(value))
case treemake.Tree(value) => List(value)
case treemake.Content(list) => list.flatMap(_.treeifyList)
}
def treeify: Iterator[Tree] = treeifyList.iterator
def make: Iterator[Tree] = treeifyList.iterator
}
private object treemake {
private case class Quat(q: io.getquill.quat.Quat) extends treemake
Expand All @@ -91,14 +92,17 @@ class AstPrinter(traceOpinions: Boolean, traceAstSimple: Boolean, traceQuats: Qu
Tree.Literal("" + past) // Do not blow up if it is null

case i: Ident =>
Tree.Apply("Id", treemake(i.name).withQuat(i.bestQuat).treeify)
Tree.Apply("Id", treemake(i.name).withQuat(i.bestQuat).make)

case i: Infix =>
Tree.Apply("Infix", treemake(i.parts).withElem(treemake(i.params)).withQuat(i.bestQuat).make)

case e: Entity if (!traceOpinions) =>
Tree.Apply("Entity", treemake(e.name, e.properties).withQuat(e.bestQuat).treeify)
Tree.Apply("Entity", treemake(e.name, e.properties).withQuat(e.bestQuat).make)

case q: Quat => Tree.Literal(q.shortString)

case s: ScalarValueLift => Tree.Apply("ScalarValueLift", treemake("..." + s.name.reverse.take(15).reverse).withQuat(s.bestQuat).treeify)
case s: ScalarValueLift => Tree.Apply("ScalarValueLift", treemake("..." + s.name.reverse.take(15).reverse).withQuat(s.bestQuat).make)

case p: Property if (traceOpinions) =>
TreeApplyList("Property", l(treeify(p.ast)) ++ l(treeify(p.name)) ++
Expand All @@ -116,7 +120,7 @@ class AstPrinter(traceOpinions: Boolean, traceAstSimple: Boolean, traceQuats: Qu
))

case e: Entity if (traceOpinions) =>
Tree.Apply("Entity", treemake(e.name, e.properties).withTree(printRenameable(e.renameable)).withQuat(e.bestQuat).treeify)
Tree.Apply("Entity", treemake(e.name, e.properties).withTree(printRenameable(e.renameable)).withQuat(e.bestQuat).make)

case ast: Ast =>
if (traceAllQuats)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package io.getquill.norm

import io.getquill.ast.{ Action, Assignment, AssignmentDual, Ast, ConcatMap, Filter, FlatJoin, FlatMap, GroupBy, Ident, Insert, Join, Map, OnConflict, Property, Query, Returning, ReturningGenerated, SortBy, StatelessTransformer, Update }
import io.getquill.ast.{ Action, Assignment, AssignmentDual, Ast, ConcatMap, Filter, FlatJoin, FlatMap, GroupBy, Ident, Infix, Insert, Join, Map, OnConflict, Property, Query, Returning, ReturningGenerated, SortBy, StatelessTransformer, Update }
import io.getquill.quat.Quat
import io.getquill.quat.Quat.Product
import io.getquill.util.Interpolator
Expand Down Expand Up @@ -64,6 +64,14 @@ object RepropagateQuats extends StatelessTransformer {
trace"Repropagate ${a.quat.suppress(msg)} from ${a} into:" andReturn f(ar, br, apply(cr))
}

override def apply(e: Ast): Ast =
e match {
case Infix(parts, params, pure, quat) =>
val newParams = params.map(apply)
Quat.improveInfixQuat(Infix(parts, newParams, pure, quat))
case _ => super.apply(e)
}

override def apply(e: Query): Query = {
e match {
case Filter(a, b, c) => applyBody(a, b, c)(Filter)
Expand Down
22 changes: 19 additions & 3 deletions quill-engine/src/main/scala/io/getquill/quat/Quat.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package io.getquill.quat

import io.getquill.ast.Ast
import io.getquill.ast.{ Ast, Infix }
import io.getquill.quotation.QuatException
import io.getquill.util.Messages.TraceType

Expand Down Expand Up @@ -126,7 +126,7 @@ sealed trait Quat {
def lookup(path: String, failNonExist: Boolean): Quat = (this, path) match {
case (cc @ Quat.Product(fields), fieldName) =>
fields.get(fieldName).getOrElse {
if (failNonExist)
if (failNonExist && cc.tpe == Quat.Product.Type.Concrete)
throw new IllegalArgumentException(s"The field '${fieldName}' does not exist in an SQL-level type ${cc}")
else {
io.getquill.util.Messages.trace(s"The field '${fieldName}' does not exist in an SQL-level type ${cc}. Assuming it's type is Quat.Unknown", traceType = TraceType.Warning)
Expand Down Expand Up @@ -156,6 +156,21 @@ object Quat {
def unapply(ast: Ast) = Some(ast.quat)
}

def improveInfixQuat(ast: Ast) =
ast match {
case Infix(parts, params, pure, Quat.Generic | Quat.Unknown) =>
val possiblyBetterQuat = params.foldLeft(Quat.Generic: Quat)((currQuat, param) => currQuat.leastUpperType(param.quat).getOrElse(currQuat))
val bestQuat =
possiblyBetterQuat match {
case Quat.Unknown => Quat.Unknown
case Quat.Value => Quat.Generic
case other => other
}
Infix(parts, params, pure, bestQuat)
case _ =>
ast
}

import LinkedHashMapOps._

def fromSerializedJVM(serial: String): Quat = KryoQuatSerializer.deserialize(serial)
Expand Down Expand Up @@ -203,6 +218,7 @@ object Quat {
}

def leastUpperTypeProduct(other: Quat.Product): Option[Quat.Product] = {
val oneQuatIsAbstract = this.tpe == Product.Type.Abstract || other.tpe == Product.Type.Abstract
val newFieldsIter =
fields.zipWith(other.fields) {
case (key, thisQuat, Some(otherQuat)) => (key, thisQuat.leastUpperType(otherQuat))
Expand All @@ -211,7 +227,7 @@ object Quat {
}
val newFields = mutable.LinkedHashMap(newFieldsIter.toList: _*)
val newTpe =
if (this.tpe == Product.Type.Abstract || other.tpe == Product.Type.Abstract)
if (oneQuatIsAbstract)
Product.Type.Abstract
else
Product.Type.Concrete
Expand Down
66 changes: 66 additions & 0 deletions quill-sql/src/test/scala/io/getquill/quat/QuatRunSpec.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package io.getquill.quat

import io.getquill._

class QuatRunSpec extends Spec {

val testContext = new SqlMirrorContext(PostgresDialect, Literal)
import testContext._

"should refine quats from generic infixes and express during execution" - {
case class MyPerson(name: String, age: Int)
val MyPersonQuat = Quat.Product("name" -> Quat.Value, "age" -> Quat.Value)

"from extension methods" in {
implicit class QueryOps[Q <: Query[_]](q: Q) {
def appendFoo = quote(infix"$q APPEND FOO".pure.as[Q])
}
val q = quote(query[MyPerson].appendFoo)
q.ast.quat mustEqual MyPersonQuat // I.e ReifyLiftings runs RepropagateQuats to take care of this
testContext.run(q).string mustEqual "SELECT x.name, x.age FROM MyPerson x APPEND FOO"
}

"from extension methods - generic marker" in {
implicit class QueryOps[Q <: Query[_]](q: Q) {
def appendFoo = quote(infix"$q APPEND FOO".generic.pure.as[Q])
}
val q = quote(query[MyPerson].appendFoo)
q.ast.quat mustEqual MyPersonQuat // I.e ReifyLiftings runs RepropagateQuats to take care of this
testContext.run(q).string mustEqual "SELECT x.name, x.age FROM MyPerson x APPEND FOO"
}

"should support query-ops function" in {
def appendFooFun[Q <: Query[_]]: Quoted[Q => Q] = quote { (q: Q) => infix"$q APPEND FOO".pure.as[Q] }
val q = quote(appendFooFun(query[MyPerson]))
q.ast.quat mustEqual Quat.Unknown
testContext.run(q).string mustEqual "SELECT x.name, x.age FROM MyPerson x APPEND FOO"
}

"should support query-ops function - dynamic function" in {
def appendFooFun[Q <: Query[_]] = quote { (q: Q) => infix"$q APPEND FOO".pure.as[Q] }
val q = quote(appendFooFun(query[MyPerson]))
q.ast.quat mustEqual MyPersonQuat
testContext.run(q).string mustEqual "SELECT x.name, x.age FROM MyPerson x APPEND FOO"
}

"merged with abstract quat" in {
trait AbstractPerson { def name: String }
case class MyPerson(name: String, age: Int) extends AbstractPerson
def filterPerson[P <: AbstractPerson] = quote {
(q: Query[P]) => q.filter(p => p.name == "Joe")
}
val p = quote(filterPerson(query[MyPerson]))
testContext.run(p).string mustEqual "SELECT p.name, p.age FROM MyPerson p WHERE p.name = 'Joe'"
}

"merged with abstract quat - dynamic" in {
trait AbstractPerson { def name: String }
case class MyPerson(name: String, age: Int) extends AbstractPerson
def filterPerson[P <: AbstractPerson]: Quoted[Query[P] => Query[P]] = quote {
(q: Query[P]) => q.filter(p => p.name == "Joe")
}
val p = quote(filterPerson(query[MyPerson]))
testContext.run(p) mustEqual "SELECT p.name, p.age FROM MyPerson p WHERE p.name = 'Joe'"
}
}
}