Permalink
Browse files

Fix two typing issues in `Case` expressions:

- One-argument OptionMappers (as implemented by OptionMapper2) were not
  always inferred correctly. Setting the first base and mapped type to
  `Boolean` instead of asking for the desired type twice helps the type
  inferencer find the correct OptionMapper2.

- `IfThenElse.nullExtend` was broken by design. We cannot reply on AST
  types in the Lifted Embedding DSL. Null-extending is now performed
  directly in `TypedCase` based on the embedding types. It also turns
  out to be much simpler than originally implemented in `nullExtend`
  because the typing rules of `Case` allow only three cases: Explicit
  `Else` clause -> use original types; missing `Else` and base type is
  an `Option` -> use original types; missing `Else` and base type is not
  an `Option` -> wrap *all* `Then` clauses in `OptionApply`.

Fixes #1364. Test in RelationalMiscTest.testConditional.
  • Loading branch information...
szeiger committed Nov 23, 2015
1 parent 73a0166 commit 06a6420fb83b1987c1716b24a8e13c8734f91c2c
@@ -87,15 +87,16 @@ class RelationalMiscTest extends AsyncTest[RelationalTestDB] {
}
def testConditional = {
class T1(tag: Tag) extends Table[Int](tag, "t1_conditional") {
class T1(tag: Tag) extends Table[(Int, Option[Int])](tag, "t1_conditional") {
def a = column[Int]("a")
def * = a
def b = column[Option[Int]]("b")
def * = (a, b)
}
val t1s = TableQuery[T1]
for {
_ <- t1s.schema.create
_ <- t1s ++= Seq(1, 2, 3, 4)
_ <- t1s ++= Seq((1, Some(11)), (2, None), (3, Some(33)), (4, None))
q1 = t1s.map { t1 => (t1.a, Case.If(t1.a < 3) Then 1 Else 0) }
_ <- q1.to[Set].result.map(_ shouldBe Set((1, 1), (2, 1), (3, 0), (4, 0)))
@@ -105,6 +106,9 @@ class RelationalMiscTest extends AsyncTest[RelationalTestDB] {
q3 = t1s.map { t1 => (t1.a, Case.If(t1.a < 3) Then 1 If(t1.a < 4) Then 2 Else 0) }
_ <- q3.to[Set].result.map(_ shouldBe Set((1, 1), (2, 1), (3, 2), (4, 0)))
q4 = t1s.map { t1 => Case.If(t1.a < 3) Then t1.b Else t1.a.? }.to[Set]
_ <- mark("q4", q4.result).map(_ shouldBe Set(Some(11), None, Some(3), Some(4)))
} yield ()
}
@@ -662,7 +662,7 @@ final case class IfThenElse(clauses: ConstArray[Node]) extends SimplyTypedNode {
clauses.iterator.grouped(2).withPartial(false).map { case List(i, t) => (i, t) }
def elseClause = clauses.last
/** Return a null-extended version of a single-column IfThenElse expression */
def nullExtend: IfThenElse = {
def nullExtend: IfThenElse = { //TODO 3.2: Remove this method. It is only preserved for binary compatibility in 3.1.1
def isOpt(n: Node) = n match {
case LiteralNode(null) => true
case _ :@ OptionType(_) => true
@@ -1,6 +1,6 @@
package slick.lifted
import slick.ast.{LiteralNode, IfThenElse, Node, BaseTypedType, OptionTypedType, TypedType}
import slick.ast.{LiteralNode, IfThenElse, Node, BaseTypedType, OptionType, TypedType, OptionApply}
import slick.SlickException
import slick.util.ConstArray
@@ -23,9 +23,14 @@ object Case {
}
final class TypedCase[B : TypedType, T : TypedType](clauses: ConstArray[Node]) extends Rep.TypedRep[Option[B]] {
def toNode = IfThenElse(clauses :+ LiteralNode(null)).nullExtend
def toNode: IfThenElse = {
val cl =
if(implicitly[TypedType[T]].isInstanceOf[OptionType]) clauses
else clauses.zipWithIndex.map { case (n, i) => if(i % 2 == 0) n else OptionApply(n) }
IfThenElse(cl :+ LiteralNode(null))
}
def If[C <: Rep[_] : CanBeQueryCondition](cond: C) = new TypedWhen[B,T](cond.toNode, clauses)
def Else(res: Rep[T]): Rep[T] = Rep.forNode(IfThenElse(clauses :+ res.toNode).nullExtend)
def Else(res: Rep[T]): Rep[T] = Rep.forNode(IfThenElse(clauses :+ res.toNode))
}
final class TypedWhen[B : TypedType, T : TypedType](cond: Node, parentClauses: ConstArray[Node]) {
@@ -63,8 +63,7 @@ object OptionMapper3 {
object OptionMapperDSL {
type arg[B1, P1] = {
type to[BR, PR] = OptionMapper2[B1, B1, BR, P1, P1, PR]
type toSame = OptionMapper2[B1, B1, B1, P1, P1, P1]
type to[BR, PR] = OptionMapper2[Boolean, B1, BR, Boolean, P1, PR]
type arg[B2, P2] = {
type to[BR, PR] = OptionMapper2[B1, B2, BR, P1, P2, PR]
type arg[B3, P3] = {

0 comments on commit 06a6420

Please sign in to comment.