Merge pull request #8842 from systay/3.2-top-lp

Support Top in the compiled runtime

community/cypher/cypher-compiler-3.2/src/main/scala/org/neo4j/cypher/internal/compiler/v3_2/planner/execution/PipeExecutionPlanBuilder.scala

@@ -293,17 +293,14 @@ case class ActualPipeBuilder(monitors: Monitors, recurse: LogicalPlan => Pipe, r
       case SkipPlan(_, count) =>
         SkipPipe(source, buildExpression(count))(id = id)
 
-      case LimitPlan(_, count, DoNotIncludeTies) =>
-        (source, count) match {
-          case (SortPipe(inner, sortDescription), SignedDecimalIntegerLiteral("1")) =>
-            Top1Pipe(inner, sortDescription.toList)(id = id)
+      case Top(_, sortItems, SignedDecimalIntegerLiteral("1")) =>
+        Top1Pipe(source, sortItems.map(translateSortDescription).toList)(id = id)
 
-          case (SortPipe(inner, sortDescription), _) =>
-            TopNPipe(inner, sortDescription.toList, buildExpression(count))(id = id)
+      case Top(_, sortItems, limit) =>
+        TopNPipe(source, sortItems.map(translateSortDescription).toList, buildExpression(limit))(id = id)
 
-          case _ =>
-            LimitPipe(source, buildExpression(count))(id = id)
-        }
+      case LimitPlan(_, count, DoNotIncludeTies) =>
+        LimitPipe(source, buildExpression(count))(id = id)
 
       case LimitPlan(_, count, IncludeTies) =>
         (source, count) match {

community/cypher/cypher-compiler-3.2/src/main/scala/org/neo4j/cypher/internal/compiler/v3_2/planner/logical/LogicalPlan2PlanDescription.scala

@@ -249,6 +249,9 @@ case class LogicalPlan2PlanDescription(idMap: Map[LogicalPlan, Id], readOnly: Bo
       case Sort(_, orderBy) =>
         PlanDescriptionImpl(id, "Sort", children, Seq(KeyNames(orderBy.map(_.id.name))), variables)
 
+      case Top(_, orderBy, limit) =>
+        PlanDescriptionImpl(id, "Top", children, Seq(KeyNames(orderBy.map(_.id.name)), Expression(limit)), variables)
+
       case UnwindCollection(_, _, expression) =>
         PlanDescriptionImpl(id, "Unwind", children, Seq(Expression(expression)), variables)
 

community/cypher/cypher-compiler-3.2/src/main/scala/org/neo4j/cypher/internal/compiler/v3_2/planner/logical/plans/Top.scala

@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2002-2017 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.cypher.internal.compiler.v3_2.planner.logical.plans
+
+import org.neo4j.cypher.internal.compiler.v3_2.planner.logical.SortDescription
+import org.neo4j.cypher.internal.compiler.v3_2.planner.{CardinalityEstimation, PlannerQuery}
+import org.neo4j.cypher.internal.frontend.v3_2.ast.Expression
+import org.neo4j.cypher.internal.ir.v3_2.IdName
+
+case class Top(left: LogicalPlan, sortItems: Seq[SortDescription], limit: Expression)
+              (val solved: PlannerQuery with CardinalityEstimation) extends LogicalPlan with EagerLogicalPlan {
+  override def lhs: Option[LogicalPlan] = Some(left)
+
+  override def rhs: Option[LogicalPlan] = None
+
+  override def availableSymbols: Set[IdName] = left.availableSymbols
+}

community/cypher/cypher-compiler-3.2/src/main/scala/org/neo4j/cypher/internal/compiler/v3_2/planner/logical/plans/rewriter/LogicalPlanRewriter.scala

@@ -45,7 +45,8 @@ case class PlanRewriter(rewriterSequencer: String => RewriterStepSequencer) exte
     unnestOptional,
     predicateRemovalThroughJoins,
     removeIdenticalPlans,
-    pruningVarExpander
+    pruningVarExpander,
+    useTop
   ).rewriter)
 }
 

community/cypher/cypher-compiler-3.2/src/main/scala/org/neo4j/cypher/internal/compiler/v3_2/planner/logical/plans/rewriter/useTop.scala

@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2002-2017 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.cypher.internal.compiler.v3_2.planner.logical.plans.rewriter
+
+import org.neo4j.cypher.internal.compiler.v3_2.planner.logical.plans.{DoNotIncludeTies, Limit, Sort, Top}
+import org.neo4j.cypher.internal.frontend.v3_2.{Rewriter, bottomUp}
+
+/**
+  * When doing ORDER BY c1,c2,...,cn LIMIT e, we don't have to sort the full result in one go
+  */
+case object useTop extends Rewriter {
+
+  private val instance: Rewriter = bottomUp(Rewriter.lift {
+    case o @ Limit(Sort(src, sortDescriptions), limit, DoNotIncludeTies) =>
+      Top(src, sortDescriptions, limit)(o.solved)
+  })
+
+  override def apply(input: AnyRef): AnyRef = instance.apply(input)
+}

community/cypher/cypher-compiler-3.2/src/test/scala/org/neo4j/cypher/internal/compiler/v3_2/planner/logical/plans/rewriter/UseTopTest.scala

@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2002-2017 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.cypher.internal.compiler.v3_2.planner.logical.plans.rewriter
+
+import org.neo4j.cypher.internal.compiler.v3_2.planner.LogicalPlanningTestSupport
+import org.neo4j.cypher.internal.compiler.v3_2.planner.logical.Ascending
+import org.neo4j.cypher.internal.compiler.v3_2.planner.logical.plans._
+import org.neo4j.cypher.internal.frontend.v3_2.ast.AstConstructionTestSupport
+import org.neo4j.cypher.internal.frontend.v3_2.helpers.fixedPoint
+import org.neo4j.cypher.internal.frontend.v3_2.test_helpers.CypherFunSuite
+
+class UseTopTest extends CypherFunSuite with LogicalPlanningTestSupport with AstConstructionTestSupport {
+  private val leaf = newMockedLogicalPlan()
+  private val sortDescription = Seq(Ascending("x"))
+  private val sort = Sort(leaf, sortDescription)(solved)
+  private val lit10 = literalInt(10)
+
+  test("should use Top when possible") {
+    val limit = Limit(sort, lit10, DoNotIncludeTies)(solved)
+
+    rewrite(limit) should equal(Top(leaf, sortDescription, lit10)(solved))
+  }
+
+  test("should not use Top when including ties") {
+    val original = Limit(sort, lit10, IncludeTies)(solved)
+
+    rewrite(original) should equal(original)
+  }
+
+  private def rewrite(p: LogicalPlan): LogicalPlan =
+    fixedPoint((p: LogicalPlan) => p.endoRewrite(useTop))(p)
+}

community/cypher/cypher/src/test/scala/org/neo4j/cypher/LdbcQueries.scala

@@ -19,7 +19,7 @@
  */
 package org.neo4j.cypher
 
-import org.joda.time.{DateTimeZone, DateTime}
+import org.joda.time.{DateTime, DateTimeZone}
 
 /**
  * These are the 14 LDBC stream that runs in the LDBC projects. The stream are (semi-)generated so the idea is rather
@@ -895,7 +895,7 @@ object LdbcQueries {
       Map("personId" -> 3, "commentContent" -> "C01", "commentId" -> 10, "personLastName" -> "three-ᚠさ丵פش", "commentCreationDate" -> 1, "personFirstName" -> "friend"),
       Map("personId" -> 3, "commentContent" -> "C11", "commentId" -> 11, "personLastName" -> "three-ᚠさ丵פش", "commentCreationDate" -> 1, "personFirstName" -> "friend"))
 
-    override def supportedInCompiledRuntime: Boolean = true
+    override def supportedInCompiledRuntime = true
   }
 
   object Query9 extends LdbcQuery {

enterprise/cypher/acceptance-spec-suite/src/test/resources/blacklists/compatibility-23.txt

@@ -55,3 +55,6 @@ Get node degree via length of pattern expression that specifies a relationship t
 //OrderByAcceptance.feature - Unsupported orderability
 ORDER BY nodes should return null results last in ascending order
 ORDER BY relationships should return null results last in ascending order
+
+//SkipLimitAcceptance.feature - Unsupported limit syntax
+Negative parameter for LIMIT should not generate errors

enterprise/cypher/acceptance-spec-suite/src/test/resources/cypher/features/OrderByAcceptance.feature

@@ -106,3 +106,29 @@ Feature: OrderByAcceptance
       |  4  | 'd' |
 
     And no side effects
+
+  Scenario: ORDER BY two node properties with LIMIT
+    Given an empty graph
+    And having executed:
+      """
+      CREATE (:L {a: 3, b: "a"}),
+             (:L {a: 1, b: "b"}),
+             (:L {a: 3, b: "c"}),
+             (:L {a: 4, b: "d"}),
+             (:L {a: 2, b: "e"})
+      """
+    When executing query:
+      """
+      MATCH (n:L)
+      WITH n.a AS a, n.b AS b
+      ORDER BY a, b DESC
+      LIMIT 3
+      RETURN a, b
+      """
+    Then the result should be, in order:
+      |  a  |  b  |
+      |  1  | 'b' |
+      |  2  | 'e' |
+      |  3  | 'c' |
+
+    And no side effects

enterprise/cypher/acceptance-spec-suite/src/test/resources/cypher/features/SkipLimitAcceptance.feature

@@ -0,0 +1,56 @@
+#
+# Copyright (c) 2002-2017 "Neo Technology,"
+# Network Engine for Objects in Lund AB [http://neotechnology.com]
+#
+# This file is part of Neo4j.
+#
+# Neo4j is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+Feature: SkipLimitAcceptance.feature
+
+  Background:
+    Given an empty graph
+
+  Scenario: Negative parameter for LIMIT should not generate errors
+    And having executed:
+      """
+      CREATE (s:Person {name: 'Steven'}),
+             (c:Person {name: 'Craig'})
+      """
+    And parameters are:
+      | limit | -1 |
+    When executing query:
+      """
+      MATCH (p:Person)
+      RETURN p.name AS name
+      LIMIT $limit
+      """
+    Then the result should be, in order:
+      | name |
+    And no side effects
+
+  Scenario: Negative LIMIT should fail with a syntax exception
+    And having executed:
+      """
+      CREATE (s:Person {name: 'Steven'}),
+             (c:Person {name: 'Craig'})
+      """
+    When executing query:
+      """
+      MATCH (p:Person)
+      RETURN p.name AS name
+      LIMIT -1
+      """
+    Then a SyntaxError should be raised at compile time: NegativeIntegerArgument