-
Notifications
You must be signed in to change notification settings - Fork 348
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix for non-fallthrough null operations in map/flatMap/exists
- Loading branch information
1 parent
5424ef7
commit 982bc47
Showing
34 changed files
with
827 additions
and
159 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
48 changes: 36 additions & 12 deletions
48
quill-core/src/main/scala/io/getquill/norm/FlattenOptionOperation.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
65 changes: 65 additions & 0 deletions
65
quill-core/src/main/scala/io/getquill/norm/SimplifyNullChecks.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| package io.getquill.norm | ||
|
|
||
| import io.getquill.ast._ | ||
| import io.getquill.ast.Implicits._ | ||
|
|
||
| /** | ||
| * Due to the introduction of null checks in `map`, `flatMap`, and `exists`, in | ||
| * `FlattenOptionOperation` in order to resolve #1053, as well as to support non-ansi | ||
| * compliant string concatenation as outlined in #1295, large conditional composites | ||
| * became common. For example: | ||
| * <code><pre> | ||
| * case class Holder(value:Option[String]) | ||
| * | ||
| * // The following statement | ||
| * query[Holder].map(h => h.value.map(_ + "foo")) | ||
| * // Will yield the following result | ||
| * SELECT CASE WHEN h.value IS NOT NULL THEN h.value || 'foo' ELSE null END FROM Holder h | ||
| * </pre></code> | ||
| * Now, let's add a <code>getOrElse</code> statement to the clause that requires an additional | ||
| * wrapped null check. We cannot rely on there being a <code>map</code> call beforehand | ||
| * since we could be reading <code>value</code> as a nullable field directly from the database). | ||
| * <code><pre> | ||
| * // The following statement | ||
| * query[Holder].map(h => h.value.map(_ + "foo").getOrElse("bar")) | ||
| * // Yields the following result: | ||
| * SELECT CASE WHEN | ||
| * CASE WHEN h.value IS NOT NULL THEN h.value || 'foo' ELSE null END | ||
| * IS NOT NULL THEN | ||
| * CASE WHEN h.value IS NOT NULL THEN h.value || 'foo' ELSE null END | ||
| * ELSE 'bar' END FROM Holder h | ||
| * </pre></code> | ||
| * This of course is highly redundant and can be reduced to simply: | ||
| * <code><pre> | ||
| * SELECT CASE WHEN h.value IS NOT NULL AND (h.value || 'foo') IS NOT NULL THEN h.value || 'foo' ELSE 'bar' END FROM Holder h | ||
| * </pre></code> | ||
| * This reduction is done by the "Center Rule." There are some other simplification | ||
| * rules as well. Note how we are force to null-check both `h.value` as well as `(h.value || 'foo')` because | ||
| * a user may use `Option[T].flatMap` and explicitly transform a particular value to `null`. | ||
| */ | ||
| object SimplifyNullChecks extends StatelessTransformer { | ||
|
|
||
| override def apply(ast: Ast): Ast = | ||
| ast match { | ||
|
|
||
| // Center rule | ||
| case IfExist( | ||
| IfExistElseNull(condA, thenA), | ||
| IfExistElseNull(condB, thenB), | ||
| otherwise | ||
| ) if (condA == condB && thenA == thenB) => | ||
| apply(If(IsNotNullCheck(condA) +&&+ IsNotNullCheck(thenA), thenA, otherwise)) | ||
|
|
||
| // Left hand rule | ||
| case IfExist(IfExistElseNull(check, affirm), value, otherwise) => | ||
| apply(If(IsNotNullCheck(check) +&&+ IsNotNullCheck(affirm), value, otherwise)) | ||
|
|
||
| // Right hand rule | ||
| case IfExistElseNull(cond, IfExistElseNull(innerCond, innerThen)) => | ||
| apply(If(IsNotNullCheck(cond) +&&+ IsNotNullCheck(innerCond), innerThen, NullValue)) | ||
|
|
||
| case other => | ||
| super.apply(other) | ||
| } | ||
|
|
||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.