Fix for non-fallthrough null operations in map/flatMap/exists

.gitignore

@@ -15,5 +15,7 @@ quill-sql/io/
 MyTest.scala
 MySparkTest.scala
 MyTestJdbc.scala
+MyTestSql.scala
 quill-core/src/main/resources/logback.xml
 quill-jdbc/src/main/resources/logback.xml
+log.txt*

README.md

@@ -735,6 +735,9 @@ only a specific subset. This is mostly due to the inherent limitations of SQL it
 'Optional Tables' section.
 
 ### Optionals / Nullable Fields
+
+> Note that the behavior of Optionals has recently changed to include stricter null-checks. See the [orNull / getOrNull](#ornull--getornull) section for more details.
+
 Option objects are used to encode nullable fields.
 Say you have the following schema:
 ```sql
@@ -812,7 +815,7 @@ typical database behavior of immediately falsifying a statement that has `null`
 Use this method in boolean conditions that should succeed in the null case.
 ```scala
 val q = quote {
-  query[Person].join(query[Address]).on((p, a)=> a.fk.forall(_ == p.id))
+  query[Person].join(query[Address]).on((p, a) => a.fk.forall(_ == p.id))
 }
 ctx.run(q)
 // SELECT p.id, p.name, a.fk, a.street, a.zip FROM Person p INNER JOIN Address a ON a.fk IS NULL OR a.fk = p.id
@@ -838,7 +841,7 @@ val q = quote {
 }
 
 ctx.run(q)
-// SELECT p.id, 'Dear ' || p.name FROM Person p
+// SELECT p.id, CASE WHEN p.name IS NOT NULL THEN 'Dear ' || p.name ELSE null END FROM Person p
 ```
 
 Additionally, this this method is useful when you want to get a non-optional field out of a outer-joined table
@@ -848,10 +851,13 @@ Additionally, this this method is useful when you want to get a non-optional fie
 val q = quote {
   query[Company].leftJoin(query[Address])
     .on((c, a) => c.zip == a.zip)
-    .map({case(c,a) =>                          // Row type is (Company, Option[Address])
+    .map {case(c,a) =>                          // Row type is (Company, Option[Address])
       (c.name, a.map(_.street), a.map(_.zip))   // Use `Option.map` to get `street` and `zip` fields
-    }) 
+    }
 }
+
+run(q)
+// SELECT c.name, a.street, a.zip FROM Company c LEFT JOIN Address a ON c.zip = a.zip
 ```
 
 For more details about this operation (and some caveats), see the 'Optional Tables' section.
@@ -873,7 +879,7 @@ val q = quote {
 }
 
 ctx.run(q) //: List[Option[String]]
-// SELECT (a.name || ' comes after ') || b.name FROM Person a, Person b WHERE a.id > b.id
+// SELECT CASE WHEN a.name IS NOT NULL AND b.name IS NOT NULL THEN (a.name || ' comes after ') || b.name ELSE null END FROM Person a, Person b WHERE a.id > b.id
 
 // Alternatively, you can use `flatten`
 val q = quote {
@@ -887,20 +893,87 @@ val q = quote {
 }
 
 ctx.run(q) //: List[Option[String]]
-// SELECT (a.name || ' comes after ') || b.name FROM Person a, Person b WHERE a.id > b.id
+// SELECT CASE WHEN a.name IS NOT NULL AND b.name IS NOT NULL THEN (a.name || ' comes after ') || b.name ELSE null END FROM Person a, Person b WHERE a.id > b.id
 ``` 
 This is also very useful when selecting from outer-joined tables i.e. where the entire table
 is inside of an `Option` object. Note how below we get the `fk` field from `Option[Address]`.
 
 ```scala
 val q = quote {
   query[Person].leftJoin(query[Address])
-    .on((p, a)=> a.fk.exists(_ == p.id))
-    .map({case (p /*Person*/, a /*Option[Address]*/) => (p.name, a.flatMap(_.fk))})
+    .on((p, a) => a.fk.exists(_ == p.id))
+    .map {case (p /*Person*/, a /*Option[Address]*/) => (p.name, a.flatMap(_.fk))}
 }
 
 ctx.run(q) //: List[(Option[String], Option[Int])]
-// SELECT p.name, a.fk FROM Person p LEFT JOIN Address a ON a.fk = p.id
+// SELECT p.name, a.fk FROM Person p LEFT JOIN Address a ON a.fk IS NOT NULL AND a.fk = p.id
+```
+
+#### orNull / getOrNull
+
+The `orNull` method can be used to convert an Option-enclosed row back into a regular row.
+Since `Option[T].orNull` does not work for primitive types (e.g. `Int`, `Double`, etc...),
+you can use the `getOrNull` method inside of quoted blocks to do the same thing.
+
+> Note that since the presence of null columns can cause queries to break in some data sources (e.g. Spark), so use this operation very carefully.
+
+```scala
+val q = quote {
+  query[Person].join(query[Address])
+    .on((p, a) => a.fk.exists(_ == p.id))
+    .filter {case (p /*Person*/, a /*Option[Address]*/) => 
+      a.fk.getOrNull != 123 } // Exclude a particular value from the query.
+                              // Since we already did an inner-join on this value, we know it is not null.
+}
+
+ctx.run(q) //: List[(Address, Person)]
+// SELECT p.id, p.name, a.fk, a.street, a.zip FROM Person p INNER JOIN Address a ON a.fk IS NOT NULL AND a.fk = p.id WHERE a.fk <> 123
+```
+
+In previous versions of Quill, null values were not explicitly checked and ANSI-null behavior was relied upon
+in order for option-enclosed values to work properly.
+
+```scala
+val q = quote {
+  query[Person].map(p => p.name.map(n => n + " suffix"))
+}
+
+ctx.run(q)
+// Used to be this:
+// SELECT p.name || ' suffix' FROM Person p
+// Now is this:
+// SELECT CASE WHEN p.name IS NOT NULL THEN p.name || ' suffix' ELSE null END FROM Person p
+```
+
+If you wish to simulate this previous behavior, you can use a combination of `Option.apply` and `orNull` (or `getOrNull` where needed).
+
+```scala
+val q = quote {
+  query[Person].map(p => Option(p.name.orNull + " suffix"))
+}
+
+ctx.run(q)
+// SELECT p.name || ' suffix' FROM Person p 
+// i.e. same as the previous behavior
+```
+
+Due to the aforementioned change, `case.. if` conditionals will now work correctly when used together with `map` and `getOrElse`.
+However, since technically this changes functionality, the following compile-time warning message has been introduced:
+
+> Conditionals inside of Option.[map | flatMap | exists | forall] will create a `CASE` statement in order to properly null-check the sub-query (...)
+
+```
+val q = quote {
+  query[Person].map(p => p.name.map(n => if (n == "Joe") "foo" else "bar").getOrElse("baz"))
+}
+// Information:(16, 15) Conditionals inside of Option.map will create a `CASE` statement in order to properly null-check the sub-query: `p.name.map((n) => if(n == "Joe") "foo" else "bar")`. 
+// Expressions like Option(if (v == "foo") else "bar").getOrElse("baz") will now work correctly, but expressions that relied on the broken behavior (where "bar" would be returned instead) need to be modified  (see the "orNull / getOrNull" section of the documentation of more detail).
+
+ctx.run(a)
+// Used to be this:
+// SELECT CASE WHEN CASE WHEN p.name = 'Joe' THEN 'foo' ELSE 'bar' END IS NOT NULL THEN CASE WHEN p.name = 'Joe' THEN 'foo' ELSE 'bar' END ELSE 'baz' END FROM Person p
+// Now is this:
+// SELECT CASE WHEN p.name IS NOT NULL AND CASE WHEN p.name = 'Joe' THEN 'foo' ELSE 'bar' END IS NOT NULL THEN CASE WHEN p.name = 'Joe' THEN 'foo' ELSE 'bar' END ELSE 'baz' END FROM Person p
 ```
 
 #### Optional Tables

quill-cassandra/src/main/scala/io/getquill/context/cassandra/CqlNormalize.scala

@@ -1,9 +1,7 @@
 package io.getquill.context.cassandra
 
 import io.getquill.ast._
-import io.getquill.norm.RenameProperties
-import io.getquill.norm.Normalize
-import io.getquill.norm.FlattenOptionOperation
+import io.getquill.norm.{ FlattenOptionOperation, Normalize, RenameProperties, SimplifyNullChecks }
 
 object CqlNormalize {
 
@@ -13,6 +11,7 @@ object CqlNormalize {
   private[this] val normalize =
     (identity[Ast] _)
       .andThen(FlattenOptionOperation.apply _)
+      .andThen(SimplifyNullChecks.apply _)
       .andThen(Normalize.apply _)
       .andThen(RenameProperties.apply _)
       .andThen(ExpandMappedInfix.apply _)

quill-core/src/main/scala/io/getquill/MirrorIdiom.scala

@@ -125,16 +125,25 @@ class MirrorIdiom extends Idiom {
   }
 
   implicit def optionOperationTokenizer(implicit liftTokenizer: Tokenizer[Lift]): Tokenizer[OptionOperation] = Tokenizer[OptionOperation] {
-    case OptionFlatten(ast)              => stmt"${ast.token}.flatten"
-    case OptionGetOrElse(ast, body)      => stmt"${ast.token}.getOrElse(${body.token})"
-    case OptionFlatMap(ast, alias, body) => stmt"${ast.token}.flatMap((${alias.token}) => ${body.token})"
-    case OptionMap(ast, alias, body)     => stmt"${ast.token}.map((${alias.token}) => ${body.token})"
-    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"
+    case OptionTableFlatMap(ast, alias, body) => stmt"${ast.token}.flatMap((${alias.token}) => ${body.token})"
+    case OptionTableMap(ast, alias, body)     => stmt"${ast.token}.map((${alias.token}) => ${body.token})"
+    case OptionTableExists(ast, alias, body)  => stmt"${ast.token}.exists((${alias.token}) => ${body.token})"
+    case OptionTableForall(ast, alias, body)  => stmt"${ast.token}.forall((${alias.token}) => ${body.token})"
+    case OptionFlatten(ast)                   => stmt"${ast.token}.flatten"
+    case OptionGetOrElse(ast, body)           => stmt"${ast.token}.getOrElse(${body.token})"
+    case OptionFlatMap(ast, alias, body)      => stmt"${ast.token}.flatMap((${alias.token}) => ${body.token})"
+    case OptionMap(ast, alias, body)          => stmt"${ast.token}.map((${alias.token}) => ${body.token})"
+    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"
+    case OptionSome(ast)                      => stmt"Some(${ast.token})"
+    case OptionApply(ast)                     => stmt"Option(${ast.token})"
+    case OptionOrNull(ast)                    => stmt"${ast.token}.orNull"
+    case OptionGetOrNull(ast)                 => stmt"${ast.token}.getOrNull"
+    case OptionNone                           => stmt"None"
   }
 
   implicit def traversableOperationTokenizer(implicit liftTokenizer: Tokenizer[Lift]): Tokenizer[TraversableOperation] = Tokenizer[TraversableOperation] {

quill-core/src/main/scala/io/getquill/ast/Ast.scala

@@ -82,6 +82,15 @@ 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
+case class OptionTableFlatMap(ast: Ast, alias: Ident, body: Ast) extends OptionOperation
+case class OptionTableMap(ast: Ast, alias: Ident, body: Ast) extends OptionOperation
+case class OptionTableExists(ast: Ast, alias: Ident, body: Ast) extends OptionOperation
+case class OptionTableForall(ast: Ast, alias: Ident, body: Ast) extends OptionOperation
+object OptionNone extends OptionOperation
+case class OptionSome(ast: Ast) extends OptionOperation
+case class OptionApply(ast: Ast) extends OptionOperation
+case class OptionOrNull(ast: Ast) extends OptionOperation
+case class OptionGetOrNull(ast: Ast) extends OptionOperation
 
 sealed trait TraversableOperation extends Ast
 case class MapContains(ast: Ast, body: Ast) extends TraversableOperation

quill-core/src/main/scala/io/getquill/ast/AstOps.scala

@@ -0,0 +1,78 @@
+package io.getquill.ast
+
+object Implicits {
+  implicit class AstOpsExt(body: Ast) {
+    def +||+(other: Ast) = BinaryOperation(body, BooleanOperator.`||`, other)
+    def +&&+(other: Ast) = BinaryOperation(body, BooleanOperator.`&&`, other)
+    def +==+(other: Ast) = BinaryOperation(body, EqualityOperator.`==`, other)
+  }
+}
+
+object +||+ {
+  def unapply(a: Ast): Option[(Ast, Ast)] = {
+    a match {
+      case BinaryOperation(one, BooleanOperator.`||`, two) => Some((one, two))
+      case _ => None
+    }
+  }
+}
+
+object +&&+ {
+  def unapply(a: Ast): Option[(Ast, Ast)] = {
+    a match {
+      case BinaryOperation(one, BooleanOperator.`&&`, two) => Some((one, two))
+      case _ => None
+    }
+  }
+}
+
+object +==+ {
+  def unapply(a: Ast): Option[(Ast, Ast)] = {
+    a match {
+      case BinaryOperation(one, EqualityOperator.`==`, two) => Some((one, two))
+      case _ => None
+    }
+  }
+}
+
+object IsNotNullCheck {
+  def apply(ast: Ast) = BinaryOperation(ast, EqualityOperator.`!=`, NullValue)
+
+  def unapply(ast: Ast): Option[Ast] = {
+    ast match {
+      case BinaryOperation(cond, EqualityOperator.`!=`, NullValue) => Some(cond)
+      case _ => None
+    }
+  }
+}
+
+object IsNullCheck {
+  def apply(ast: Ast) = BinaryOperation(ast, EqualityOperator.`==`, NullValue)
+
+  def unapply(ast: Ast): Option[Ast] = {
+    ast match {
+      case BinaryOperation(cond, EqualityOperator.`==`, NullValue) => Some(cond)
+      case _ => None
+    }
+  }
+}
+
+object IfExistElseNull {
+  def apply(exists: Ast, `then`: Ast) =
+    If(IsNotNullCheck(exists), `then`, NullValue)
+
+  def unapply(ast: Ast) = ast match {
+    case If(IsNotNullCheck(exists), t, NullValue) => Some((exists, t))
+    case _                                        => None
+  }
+}
+
+object IfExist {
+  def apply(exists: Ast, `then`: Ast, otherwise: Ast) =
+    If(IsNotNullCheck(exists), `then`, otherwise)
+
+  def unapply(ast: Ast) = ast match {
+    case If(IsNotNullCheck(exists), t, e) => Some((exists, t, e))
+    case _                                => None
+  }
+}

quill-core/src/main/scala/io/getquill/ast/StatefulTransformer.scala

@@ -53,6 +53,22 @@ trait StatefulTransformer[T] {
 
   def apply(o: OptionOperation): (OptionOperation, StatefulTransformer[T]) =
     o match {
+      case OptionTableFlatMap(a, b, c) =>
+        val (at, att) = apply(a)
+        val (ct, ctt) = att.apply(c)
+        (OptionTableFlatMap(at, b, ct), ctt)
+      case OptionTableMap(a, b, c) =>
+        val (at, att) = apply(a)
+        val (ct, ctt) = att.apply(c)
+        (OptionTableMap(at, b, ct), ctt)
+      case OptionTableExists(a, b, c) =>
+        val (at, att) = apply(a)
+        val (ct, ctt) = att.apply(c)
+        (OptionTableExists(at, b, ct), ctt)
+      case OptionTableForall(a, b, c) =>
+        val (at, att) = apply(a)
+        val (ct, ctt) = att.apply(c)
+        (OptionTableForall(at, b, ct), ctt)
       case OptionFlatten(a) =>
         val (at, att) = apply(a)
         (OptionFlatten(at), att)
@@ -89,6 +105,19 @@ trait StatefulTransformer[T] {
       case OptionIsDefined(a) =>
         val (at, att) = apply(a)
         (OptionIsDefined(at), att)
+      case OptionSome(a) =>
+        val (at, att) = apply(a)
+        (OptionSome(at), att)
+      case OptionApply(a) =>
+        val (at, att) = apply(a)
+        (OptionApply(at), att)
+      case OptionOrNull(a) =>
+        val (at, att) = apply(a)
+        (OptionOrNull(at), att)
+      case OptionGetOrNull(a) =>
+        val (at, att) = apply(a)
+        (OptionGetOrNull(at), att)
+      case OptionNone => (o, this)
     }
 
   def apply(e: TraversableOperation): (TraversableOperation, StatefulTransformer[T]) =

quill-core/src/main/scala/io/getquill/ast/StatelessTransformer.scala

@@ -28,16 +28,25 @@ trait StatelessTransformer {
 
   def apply(o: OptionOperation): OptionOperation =
     o match {
-      case OptionFlatten(a)       => OptionFlatten(apply(a))
-      case OptionGetOrElse(a, b)  => OptionGetOrElse(apply(a), apply(b))
-      case OptionFlatMap(a, b, c) => OptionFlatMap(apply(a), b, apply(c))
-      case OptionMap(a, b, c)     => OptionMap(apply(a), b, apply(c))
-      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))
+      case OptionTableFlatMap(a, b, c) => OptionTableFlatMap(apply(a), b, apply(c))
+      case OptionTableMap(a, b, c)     => OptionTableMap(apply(a), b, apply(c))
+      case OptionTableExists(a, b, c)  => OptionTableExists(apply(a), b, apply(c))
+      case OptionTableForall(a, b, c)  => OptionTableForall(apply(a), b, apply(c))
+      case OptionFlatten(a)            => OptionFlatten(apply(a))
+      case OptionGetOrElse(a, b)       => OptionGetOrElse(apply(a), apply(b))
+      case OptionFlatMap(a, b, c)      => OptionFlatMap(apply(a), b, apply(c))
+      case OptionMap(a, b, c)          => OptionMap(apply(a), b, apply(c))
+      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))
+      case OptionSome(a)               => OptionSome(apply(a))
+      case OptionApply(a)              => OptionApply(apply(a))
+      case OptionOrNull(a)             => OptionOrNull(apply(a))
+      case OptionGetOrNull(a)          => OptionGetOrNull(apply(a))
+      case OptionNone                  => OptionNone
     }
 
   def apply(o: TraversableOperation): TraversableOperation =