Remove unnecessary if-not-null checks (#137)

* Remove unnecessary if-not-null checks
* Remove more cases of reselect

Signed-off-by: Dimitry Solovyov <dimitry@solovyov.lv>

translation/src/main/scala/org/opencypher/gremlin/translation/ir/rewrite/RemoveImmediateReselect.scala → translation/src/main/scala/org/opencypher/gremlin/translation/ir/rewrite/RemoveIdentityReselect.scala

@@ -19,15 +19,29 @@ import org.opencypher.gremlin.translation.ir.TraversalHelper._
 import org.opencypher.gremlin.translation.ir.model._
 
 /**
-  * This rule removes `select` steps that immediately follow an `as` step with the same label.
+  * This rule removes `select` steps that immediately follow an `as` step with the same label,
+  * or are separated from the `as` step by one or more `has` steps.
   * Since the expected value is already in the traverser, this is a useless operation.
   * This rewrite also enables some cases of [[RemoveUnusedAliases]] rewrites.
   */
-object RemoveImmediateReselect extends GremlinRewriter {
+object RemoveIdentityReselect extends GremlinRewriter {
   override def apply(steps: Seq[GremlinStep]): Seq[GremlinStep] = {
     mapTraversals(replace({
-      case As(stepLabel) :: SelectK(selectKey) :: rest if stepLabel == selectKey =>
-        As(stepLabel) :: rest
+      case As(stepLabel) :: rest =>
+        As(stepLabel) +: removeReselect(rest, stepLabel)
     }))(steps)
   }
+
+  private def removeReselect(steps: Seq[GremlinStep], stepLabel: String): Seq[GremlinStep] = {
+    val (filters, suffix) = steps.span {
+      case _: HasP | _: HasLabel => true
+      case _                     => false
+    }
+    suffix match {
+      case SelectK(selectKey) :: rest if stepLabel == selectKey =>
+        filters ++ rest
+      case _ =>
+        steps
+    }
+  }
 }

translation/src/main/scala/org/opencypher/gremlin/translation/ir/rewrite/RemoveUselessNullChecks.scala

@@ -0,0 +1,76 @@
+/*
+ * Copyright (c) 2018 "Neo4j, Inc." [https://neo4j.com]
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.opencypher.gremlin.translation.ir.rewrite
+
+import org.opencypher.gremlin.translation.Tokens._
+import org.opencypher.gremlin.translation.ir.TraversalHelper._
+import org.opencypher.gremlin.translation.ir.model._
+
+import scala.annotation.tailrec
+
+/**
+  * This rule finds instances of if-not-null pattern and removes them
+  * if there are no prior steps in the traversal that may produce nulls.
+  */
+object RemoveUselessNullChecks extends GremlinRewriter {
+  override def apply(steps: Seq[GremlinStep]): Seq[GremlinStep] = {
+    @tailrec def splitterAcc(acc: Seq[GremlinStep], steps: Seq[GremlinStep]): Seq[GremlinStep] = {
+      val (segment, rest) = splitSegment(steps)
+      val rewritten = rewriteSegment(acc ++ segment)
+      if (rest.nonEmpty) splitterAcc(rewritten, rest) else rewritten
+    }
+    splitterAcc(Nil, steps)
+  }
+
+  private def splitSegment(steps: Seq[GremlinStep]): (Seq[GremlinStep], Seq[GremlinStep]) = {
+    val (segment, rest) = steps.span {
+      case By(SelectK(_) :: ChooseP(Neq(NULL), _, Constant(NULL) :: Nil) :: Nil, None) => false
+      case By(ChooseP(Neq(NULL), _, Constant(NULL) :: Nil) :: Nil, None)               => false
+      case ChooseP(Neq(NULL), _, Constant(NULL) :: Nil)                                => false
+      case _                                                                           => true
+    }
+    rest match {
+      case head :: tail => (segment :+ head, tail)
+      case _            => (segment, Nil)
+    }
+  }
+
+  private def rewriteSegment(steps: Seq[GremlinStep]): Seq[GremlinStep] = {
+    val mapsToNull = foldTraversals(false)({ (acc, steps) =>
+      acc || steps.exists {
+        case MapF(_)        => true
+        case Constant(NULL) => true
+        case _              => false
+      }
+    })(steps.init)
+
+    if (mapsToNull) {
+      return steps
+    }
+
+    val last = steps.last match {
+      case By(SelectK(key) :: ChooseP(Neq(NULL), t, Constant(NULL) :: Nil) :: Nil, None) =>
+        By(SelectK(key) +: t, None) :: Nil
+      case By(ChooseP(Neq(NULL), t, Constant(NULL) :: Nil) :: Nil, None) =>
+        By(t, None) :: Nil
+      case ChooseP(Neq(NULL), traversal, Constant(NULL) :: Nil) =>
+        traversal
+      case step =>
+        step :: Nil
+    }
+    steps.init ++ last
+  }
+}

translation/src/main/scala/org/opencypher/gremlin/translation/translator/TranslatorFlavor.scala

@@ -44,7 +44,8 @@ object TranslatorFlavor {
       SimplifyRenamedAliases,
       GroupStepFilters,
       SimplifySingleProjections,
-      RemoveImmediateReselect,
+      RemoveUselessNullChecks,
+      RemoveIdentityReselect,
       RemoveUnusedAliases,
       SimplifyEdgeTraversal,
       RemoveUnusedAliases,

translation/src/test/scala/org/opencypher/gremlin/translation/ir/rewrite/RemoveImmediateReselectTest.scala → translation/src/test/scala/org/opencypher/gremlin/translation/ir/rewrite/RemoveIdentityReselectTest.scala

@@ -17,15 +17,16 @@ package org.opencypher.gremlin.translation.ir.rewrite
 
 import org.junit.Test
 import org.opencypher.gremlin.translation.CypherAst.parse
-import org.opencypher.gremlin.translation.ir.helpers.CypherAstAssert.__
+import org.opencypher.gremlin.translation.ir.helpers.CypherAstAssert._
 import org.opencypher.gremlin.translation.ir.helpers.CypherAstAssertions.assertThat
 import org.opencypher.gremlin.translation.translator.TranslatorFlavor
 
-class RemoveImmediateReselectTest {
+class RemoveIdentityReselectTest {
 
   val flavor = new TranslatorFlavor(
     rewriters = Seq(
-      InlineFlatMapTraversal
+      InlineFlatMapTraversal,
+      GroupStepFilters
     ),
     postConditions = Nil
   )
@@ -38,9 +39,22 @@ class RemoveImmediateReselectTest {
         |RETURN l
       """.stripMargin))
       .withFlavor(flavor)
-      .rewritingWith(RemoveImmediateReselect)
+      .rewritingWith(RemoveIdentityReselect)
       .removes(__.as("n").select("n"))
       .keeps(__.as("n"))
   }
 
+  @Test
+  def aliasProjectionWithHas(): Unit = {
+    assertThat(parse("""
+        |MATCH (n:L {foo: 'bar'})
+        |UNWIND labels(n) as l
+        |RETURN l
+      """.stripMargin))
+      .withFlavor(flavor)
+      .rewritingWith(RemoveIdentityReselect)
+      .removes(__.as("n").hasLabel("L").has("foo", P.isEq("bar")).select("n"))
+      .keeps(__.as("n").hasLabel("L").has("foo", P.isEq("bar")))
+  }
+
 }

translation/src/test/scala/org/opencypher/gremlin/translation/ir/rewrite/RemoveUselessNullChecksTest.scala

@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2018 "Neo4j, Inc." [https://neo4j.com]
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.opencypher.gremlin.translation.ir.rewrite
+
+import org.junit.Test
+import org.opencypher.gremlin.translation.CypherAst.parse
+import org.opencypher.gremlin.translation.Tokens._
+import org.opencypher.gremlin.translation.ir.helpers.CypherAstAssert._
+import org.opencypher.gremlin.translation.ir.helpers.CypherAstAssertions.assertThat
+import org.opencypher.gremlin.translation.translator.TranslatorFlavor
+
+class RemoveUselessNullChecksTest {
+
+  val flavor = new TranslatorFlavor(
+    rewriters = Seq(
+      InlineFlatMapTraversal,
+      SimplifySingleProjections
+    ),
+    postConditions = Nil
+  )
+
+  @Test
+  def singleProjection(): Unit = {
+    assertThat(parse("""
+        |MATCH (n)
+        |RETURN n
+      """.stripMargin))
+      .withFlavor(flavor)
+      .rewritingWith(RemoveUselessNullChecks)
+      .removes(__.by(__.choose(P.neq(NULL), __.valueMap(true), __.constant(NULL))))
+      .adds(__.by(__.valueMap(true)))
+  }
+
+  @Test
+  def multipleProjections(): Unit = {
+    assertThat(parse("""
+        |MATCH (n)-->(m)
+        |RETURN n, m
+      """.stripMargin))
+      .withFlavor(flavor)
+      .rewritingWith(RemoveUselessNullChecks)
+      .removes(__.by(__.select("n").choose(P.neq(NULL), __.valueMap(true), __.constant(NULL))))
+      .removes(__.by(__.select("m").choose(P.neq(NULL), __.valueMap(true), __.constant(NULL))))
+      .adds(__.by(__.select("n").valueMap(true)))
+      .adds(__.by(__.select("m").valueMap(true)))
+  }
+}