Permalink
Browse files

Fix 3VL handling in a scalar optimization

The case `(if(p) a else b) == v` in `optimizeScalar` was unnecessarily
general and getting it right under 3VL is hard (as witnessed by the fact
that the old implementation was broken). We now restrict it to
`(if(p) a else b).isNull`, the more specific case that is actually
needed to get rid of unnecessary code produced by Option rewriting in
earlier phases.

Test in JoinTest.testDiscriminatorCheck. Fixes #1300.
  • Loading branch information...
szeiger committed Sep 30, 2015
1 parent 2cb76c3 commit d7ea32dc2d82f1bebee9b69091abf1f3c8216919
@@ -323,15 +323,17 @@ class JoinTest extends AsyncTest[RelationalTestDB] {
}
lazy val bs = TableQuery[B]
val q = for {
val q1 = for {
(a, b) <- as joinLeft bs on (_.id.? === _.id) if (b.isEmpty)
} yield (a.id)
val q2 = bs.joinLeft(as).on(_.id === _.id).filter(_._2.isEmpty).map(_._1.id)
DBIO.seq(
(as.schema ++ bs.schema).create,
as ++= Seq(1,2,3),
bs ++= Seq(1,2,4,5).map(Some.apply _),
q.result.map(_.toSet shouldBe Set(3))
q1.result.map(_.toSet shouldBe Set(3)),
q2.result.map(_.toSet shouldBe Set(Some(4), Some(5)))
)
}
}
@@ -10,44 +10,51 @@ class OptimizeScalar extends Phase {
val name = "optimizeScalar"
def apply(state: CompilerState) = state.map(_.tree.replace({
// (if(p) a else b) == v
case n @ Library.==(IfThenElse(ConstArray(p, Const(a), Const(b))), Const(v)) =>
val checkTrue = v == a
val checkFalse = v == b
case n @ Library.==(IfThenElse(ConstArray(p, Const(a), Const(b))), LiteralNode(null)) =>
logger.debug("Optimizing: (if(p) a else b).isNull", n)
val (checkTrue, checkFalse) = (a.isEmpty, b.isEmpty)
logger.debug(s"a=$a, b=$b")
val res =
if(checkTrue && checkFalse) LiteralNode(true)
else if(checkTrue && !checkFalse) p
else if(checkFalse) Library.Not.typed(p.nodeType, p)
else LiteralNode(false)
cast(n.nodeType, res).infer()
// if(v != null) v else null
case n @ IfThenElse(ConstArray(Library.Not(Library.==(v, LiteralNode(null))), v2, LiteralNode(z)))
if v == v2 && (z == null || z == None) =>
if v == v2 && (z == null || z == None) =>
logger.debug("Optimizing: if(v != null) v else null", n)
v
// if(!false) v else _
case n @ IfThenElse(ConstArray(Library.Not(LiteralNode(false)), v, _)) =>
logger.debug("Optimizing: if(!false) v else _", n)
v
// Redundant cast to non-nullable within OptionApply
case o @ OptionApply(Library.SilentCast(n)) if o.nodeType == n.nodeType => n
case o @ OptionApply(Library.SilentCast(n)) if o.nodeType == n.nodeType =>
logger.debug("Optimizing: Redundant cast to non-nullable within OptionApply", o)
n
// Rownum comparison with offset 1, arising from zipWithIndex
case n @ Library.<(Library.-(r: RowNumber, LiteralNode(1L)), v) =>
logger.debug("Optimizing: Rownum comparison with offset 1, arising from zipWithIndex", n)
Library.<=.typed(n.nodeType, r, v).infer()
// Some(v).getOrElse(_)
case n @ Library.IfNull(OptionApply(ch), _) =>
logger.debug("Optimizing: Some(v).getOrElse(_)", n)
cast(n.nodeType, ch)
case n: Comprehension if n.where == Some(LiteralNode(true)) =>
logger.debug("Optimizing: WHERE TRUE", n)
n.copy(where = None) :@ n.nodeType
}, keepType = true, bottomUp = true))
object Const {
def unapply(n: Node): Option[Node] = n match {
case _: LiteralNode => Some(n)
def unapply(n: Node): Option[Option[Any]] = n match {
case LiteralNode(null) => Some(None)
case LiteralNode(v) =>
Some(if(n.nodeType.structural.isInstanceOf[OptionType]) v.asInstanceOf[Option[Any]] else Some(v))
case Apply(Library.SilentCast, ConstArray(ch)) => unapply(ch)
case OptionApply(ch) => unapply(ch)
case OptionApply(ch) => unapply(ch).map(_.map(Option.apply _))
case _ => None
}
}

0 comments on commit d7ea32d

Please sign in to comment.