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

fail if expression uses a table #906

Merged
merged 1 commit into from
Sep 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions quill-core/src/main/scala/io/getquill/MirrorIdiom.scala
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ class MirrorIdiom extends Idiom {
case OptionForall(ast, alias, body) => stmt"${ast.token}.forall((${alias.token}) => ${body.token})"
case OptionExists(ast, alias, body) => stmt"${ast.token}.exists((${alias.token}) => ${body.token})"
case OptionContains(ast, body) => stmt"${ast.token}.contains(${body.token})"
case OptionIsEmpty(ast) => stmt"${ast.token}.isEmpty"
case OptionNonEmpty(ast) => stmt"${ast.token}.nonEmpty"
case OptionIsDefined(ast) => stmt"${ast.token}.isDefined"
}

implicit def traversableOperationTokenizer(implicit liftTokenizer: Tokenizer[Lift]): Tokenizer[TraversableOperation] = Tokenizer[TraversableOperation] {
Expand Down
3 changes: 3 additions & 0 deletions quill-core/src/main/scala/io/getquill/ast/Ast.scala
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ case class OptionMap(ast: Ast, alias: Ident, body: Ast) extends OptionOperation
case class OptionForall(ast: Ast, alias: Ident, body: Ast) extends OptionOperation
case class OptionExists(ast: Ast, alias: Ident, body: Ast) extends OptionOperation
case class OptionContains(ast: Ast, body: Ast) extends OptionOperation
case class OptionIsEmpty(ast: Ast) extends OptionOperation
case class OptionNonEmpty(ast: Ast) extends OptionOperation
case class OptionIsDefined(ast: Ast) extends OptionOperation

sealed trait TraversableOperation extends Ast
case class MapContains(ast: Ast, body: Ast) extends TraversableOperation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ trait StatefulTransformer[T] {
val (at, att) = apply(a)
val (ct, ctt) = att.apply(c)
(OptionContains(at, ct), ctt)
case OptionIsEmpty(a) =>
val (at, att) = apply(a)
(OptionIsEmpty(at), att)
case OptionNonEmpty(a) =>
val (at, att) = apply(a)
(OptionNonEmpty(at), att)
case OptionIsDefined(a) =>
val (at, att) = apply(a)
(OptionIsDefined(at), att)
}

def apply(e: TraversableOperation): (TraversableOperation, StatefulTransformer[T]) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ trait StatelessTransformer {
case OptionForall(a, b, c) => OptionForall(apply(a), b, apply(c))
case OptionExists(a, b, c) => OptionExists(apply(a), b, apply(c))
case OptionContains(a, b) => OptionContains(apply(a), apply(b))
case OptionIsEmpty(a) => OptionIsEmpty(apply(a))
case OptionNonEmpty(a) => OptionNonEmpty(apply(a))
case OptionIsDefined(a) => OptionIsDefined(apply(a))
}

def apply(o: TraversableOperation): TraversableOperation =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ trait Liftables {
case OptionForall(a, b, c) => q"$pack.OptionForall($a,$b,$c)"
case OptionExists(a, b, c) => q"$pack.OptionExists($a,$b,$c)"
case OptionContains(a, b) => q"$pack.OptionContains($a,$b)"
case OptionIsEmpty(a) => q"$pack.OptionIsEmpty($a)"
case OptionNonEmpty(a) => q"$pack.OptionNonEmpty($a)"
case OptionIsDefined(a) => q"$pack.OptionIsDefined($a)"
}

implicit val traversableOperationLiftable: Liftable[TraversableOperation] = Liftable[TraversableOperation] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ trait Parsing {
case `orderingParser`(value) => value
case `operationParser`(value) => value
case `identParser`(value) => value
case `optionOperationParser`(value) => value
case `propertyParser`(value) => value
case `stringInterpolationParser`(value) => value
case `optionOperationParser`(value) => value
case `traversableOperationParser`(value) => value
case `boxingParser`(value) => value
case `ifParser`(value) => value
Expand Down Expand Up @@ -295,6 +295,12 @@ trait Parsing {
OptionExists(astParser(o), identParser(alias), astParser(body))
case q"$o.contains[$t]($body)" if is[Option[Any]](o) =>
OptionContains(astParser(o), astParser(body))
case q"$o.isEmpty" if is[Option[Any]](o) =>
OptionIsEmpty(astParser(o))
case q"$o.nonEmpty" if is[Option[Any]](o) =>
OptionNonEmpty(astParser(o))
case q"$o.isDefined" if is[Option[Any]](o) =>
OptionIsDefined(astParser(o))
}

val traversableOperationParser: Parser[TraversableOperation] = Parser[TraversableOperation] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ trait Unliftables {
case q"$pack.OptionForall.apply(${ a: Ast }, ${ b: Ident }, ${ c: Ast })" => OptionForall(a, b, c)
case q"$pack.OptionExists.apply(${ a: Ast }, ${ b: Ident }, ${ c: Ast })" => OptionExists(a, b, c)
case q"$pack.OptionContains.apply(${ a: Ast }, ${ b: Ast })" => OptionContains(a, b)
case q"$pack.OptionIsEmpty.apply(${ a: Ast })" => OptionIsEmpty(a)
case q"$pack.OptionNonEmpty.apply(${ a: Ast })" => OptionNonEmpty(a)
case q"$pack.OptionIsDefined.apply(${ a: Ast })" => OptionIsDefined(a)
}

implicit val traversableOperationUnliftable: Unliftable[TraversableOperation] = Unliftable[TraversableOperation] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,30 @@ class StatefulTransformerSpec extends Spec {
att.state mustEqual List(Ident("a"), Ident("c"))
}
}
"isEmpty" in {
val ast: Ast = OptionIsEmpty(Ident("a"))
Subject(Nil, Ident("a") -> Ident("a'"))(ast) match {
case (at, att) =>
at mustEqual OptionIsEmpty(Ident("a'"))
att.state mustEqual List(Ident("a"))
}
}
"nonEmpty" in {
val ast: Ast = OptionNonEmpty(Ident("a"))
Subject(Nil, Ident("a") -> Ident("a'"))(ast) match {
case (at, att) =>
at mustEqual OptionNonEmpty(Ident("a'"))
att.state mustEqual List(Ident("a"))
}
}
"isDefined" in {
val ast: Ast = OptionIsDefined(Ident("a"))
Subject(Nil, Ident("a") -> Ident("a'"))(ast) match {
case (at, att) =>
at mustEqual OptionIsDefined(Ident("a'"))
att.state mustEqual List(Ident("a"))
}
}
}

"traversable operations" - {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,21 @@ class StatelessTransformerSpec extends Spec {
Subject(Ident("a") -> Ident("a'"), Ident("c") -> Ident("c'"))(ast) mustEqual
OptionContains(Ident("a'"), Ident("c'"))
}
"isEmpty" in {
val ast: Ast = OptionIsEmpty(Ident("a"))
Subject(Ident("a") -> Ident("a'"))(ast) mustEqual
OptionIsEmpty(Ident("a'"))
}
"nonEmpty" in {
val ast: Ast = OptionNonEmpty(Ident("a"))
Subject(Ident("a") -> Ident("a'"))(ast) mustEqual
OptionNonEmpty(Ident("a'"))
}
"isDefined" in {
val ast: Ast = OptionIsDefined(Ident("a"))
Subject(Ident("a") -> Ident("a'"))(ast) mustEqual
OptionIsDefined(Ident("a'"))
}
}

"traversable operations" - {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,24 @@ class QuotationSpec extends Spec {
}
quote(unquote(q)).ast.body mustEqual OptionContains(Ident("o"), Ident("v"))
}
"isEmpty" in {
val q = quote {
(o: Option[Boolean]) => o.isEmpty
}
quote(unquote(q)).ast.body mustEqual OptionIsEmpty(Ident("o"))
}
"nonEmpty" in {
val q = quote {
(o: Option[Boolean]) => o.nonEmpty
}
quote(unquote(q)).ast.body mustEqual OptionNonEmpty(Ident("o"))
}
"isDefined" in {
val q = quote {
(o: Option[Boolean]) => o.isDefined
}
quote(unquote(q)).ast.body mustEqual OptionIsDefined(Ident("o"))
}
}
"traversable operations" - {
"map.contains" in {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,17 @@ trait SqlIdiom extends Idiom {

def astTokenizer(implicit astTokenizer: Tokenizer[Ast], strategy: NamingStrategy): Tokenizer[Ast] =
Tokenizer[Ast] {
case a: Query => SqlQuery(a).token
case a: Operation => a.token
case a: Infix => a.token
case a: Action => a.token
case a: Ident => a.token
case a: Property => a.token
case a: Value => a.token
case a: If => a.token
case a: Lift => a.token
case a: Assignment => a.token
case a: Query => SqlQuery(a).token
case a: Operation => a.token
case a: Infix => a.token
case a: Action => a.token
case a: Ident => a.token
case a: Property => a.token
case a: Value => a.token
case a: If => a.token
case a: Lift => a.token
case a: Assignment => a.token
case a: OptionOperation => a.token
case a @ (
_: Function | _: FunctionApply | _: Dynamic | _: OptionOperation | _: Block |
_: Val | _: Ordering | _: QuotedReference | _: TraversableOperation
Expand Down Expand Up @@ -177,6 +178,13 @@ trait SqlIdiom extends Idiom {
case e: FunctionApply => fail(s"Can't translate the ast to sql: '$e'")
}

implicit def optionOperationTokenizer(implicit astTokenizer: Tokenizer[Ast], strategy: NamingStrategy): Tokenizer[OptionOperation] = Tokenizer[OptionOperation] {
case OptionIsEmpty(ast) => stmt"${ast.token} IS NULL"
case OptionNonEmpty(ast) => stmt"${ast.token} IS NOT NULL"
case OptionIsDefined(ast) => stmt"${ast.token} IS NOT NULL"
case other => fail(s"Malformed or unsupported construct: $other.")
}

implicit val setOperationTokenizer: Tokenizer[SetOperation] = Tokenizer[SetOperation] {
case UnionOperation => stmt"UNION"
case UnionAllOperation => stmt"UNION ALL"
Expand Down Expand Up @@ -264,9 +272,6 @@ trait SqlIdiom extends Idiom {
case a => (a, "")
}
Tokenizer[Property] {
case Property(ast, "isEmpty") => stmt"${ast.token} IS NULL"
case Property(ast, "nonEmpty") => stmt"${ast.token} IS NOT NULL"
case Property(ast, "isDefined") => stmt"${ast.token} IS NOT NULL"
case ast =>
unnest(ast) match {
case (ast, name) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import io.getquill.context.sql.SqlQuery
import io.getquill.context.sql.TableContext
import io.getquill.context.sql.UnaryOperationSqlQuery
import io.getquill.context.sql.FlatJoinContext
import io.getquill.ast.CollectAst
import io.getquill.ast.Property

case class Error(free: List[Ident], ast: Ast)
case class InvalidSqlQuery(errors: List[Error]) {
Expand All @@ -37,25 +39,45 @@ object VerifySqlQuery {

val aliases = query.from.map(this.aliases).flatten.map(Ident(_)) :+ Ident("*") :+ Ident("?")

def verifyTableReference(ast: Ast) =
(CollectAst(ast) {
case p: Property => None
case i: Ident => Some(Error(i :: Nil, ast))
}).flatten

def verifyFreeVars(ast: Ast) =
(FreeVariables(ast) -- aliases).toList match {
case Nil => None
case free => Some(Error(free, ast))
}

val errors: List[Error] =
val tableReferenceErrors =
query.where.toList.flatMap(verifyTableReference) ++
query.orderBy.map(_.ast).flatMap(verifyTableReference) ++
query.limit.toList.flatMap(verifyTableReference) ++
query.from.flatMap {
case j: JoinContext => verifyTableReference(j.on)
case j: FlatJoinContext => verifyTableReference(j.on)
case _ => Nil
}

val freeVariableErrors: List[Error] =
query.where.flatMap(verifyFreeVars).toList ++
query.orderBy.map(_.ast).flatMap(verifyFreeVars) ++
query.limit.flatMap(verifyFreeVars) ++
query.select.map(_.ast).map(verifyFreeVars).flatten
query.select.map(_.ast).map(verifyFreeVars).flatten ++
query.from.flatMap {
case j: JoinContext => verifyFreeVars(j.on)
case j: FlatJoinContext => verifyFreeVars(j.on)
case _ => Nil
}

val nestedErrors =
query.from.collect {
case QueryContext(query, alias) => verify(query).map(_.errors)
}.flatten.flatten

(errors ++ nestedErrors) match {

(tableReferenceErrors ++ freeVariableErrors ++ nestedErrors) match {
case Nil => None
case errors => Some(InvalidSqlQuery(errors))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,15 @@ class VerifySqlQuerySpec extends Spec {
VerifySqlQuery(SqlQuery(q.ast)).toString mustEqual
"Some(The monad composition can't be expressed using applicative joins. Faulty expression: 'b.s == a.s'. Free variables: 'List(a)'.)"
}

"invalid table reference" in {
val q = quote {
qr1.leftJoin(qr2).on((a, b) => a.i == b.i).filter {
case (a, b) => b.isDefined
}
}
VerifySqlQuery(SqlQuery(q.ast)).toString mustEqual
"Some(The monad composition can't be expressed using applicative joins. Faulty expression: 'x01._2.isDefined'. Free variables: 'List(x01)'., Faulty expression: 'x01'. Free variables: 'List(x01)'.)"
}
}
}