Merge pull request #10359 from systay/3.2-pruning-varexpand-bug

Fixes bug in pattern matching around pruning var expand

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

@@ -96,7 +96,7 @@ case object pruningVarExpander extends Rewriter {
         val distinctSet = findDistinctSet(plan)
 
         val innerRewriter = topDown(Rewriter.lift {
-          case expand@VarExpand(lhs, fromId, dir, _, relTypes, toId, _, length, _, predicates) if distinctSet(expand) =>
+          case expand@VarExpand(lhs, fromId, dir, _, relTypes, toId, _, length, ExpandAll, predicates) if distinctSet(expand) =>
             if (length.min >= 4 && length.max.get >= 5)
               // These constants were selected by benchmarking on randomized graphs, with different
               // degrees of interconnection.

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

@@ -239,7 +239,7 @@ class PruningVarExpanderTest extends CypherFunSuite with LogicalPlanningTestSupp
     assertNotRewritten(input)
   }
 
-  test("do not use pruning for pathExpressions when path is needed"){
+  test("do not use pruning for pathExpressions when path is needed") {
     // Simplest query:
     // match p=(from)-[r*0..1]->(to) with nodes(p) as d return distinct d
 
@@ -260,6 +260,21 @@ class PruningVarExpanderTest extends CypherFunSuite with LogicalPlanningTestSupp
     assertNotRewritten(distinct)
   }
 
+  test("do not use pruning-varexpand when both sides of the var-length-relationship are already known") {
+    val fromId = IdName("from")
+    val toId = IdName("to")
+    val fromPlan = AllNodesScan(fromId, Set.empty)(solved)
+    val toPlan = AllNodesScan(toId, Set.empty)(solved)
+    val xJoin = CartesianProduct(fromPlan, toPlan)(solved)
+    val dir = SemanticDirection.BOTH
+    val length = VarPatternLength(1, Some(3))
+    val relId = IdName("r")
+    val originalExpand = VarExpand(xJoin, fromId, dir, dir, Seq.empty, toId, relId, length, ExpandInto)(solved)
+    val input = Aggregation(originalExpand, Map("to" -> Variable("to")(pos)), Map.empty)(solved)
+
+    assertNotRewritten(input)
+  }
+
   test("should handle insanely long logical plans without running out of stack") {
     val leafPlan: LogicalPlan = Argument(Set(IdName("x")))(solved)()
     var plan = leafPlan
@@ -273,6 +288,7 @@ class PruningVarExpanderTest extends CypherFunSuite with LogicalPlanningTestSupp
   private def assertNotRewritten(p: LogicalPlan): Unit = {
     rewrite(p) should equal(p)
   }
+
   private def rewrite(p: LogicalPlan): LogicalPlan =
     p.endoRewrite(pruningVarExpander)
 }

enterprise/cypher/acceptance-spec-suite/src/test/resources/blacklists/cost-compiled.txt

@@ -163,6 +163,7 @@ difficult to plan query number 1
 difficult to plan query number 2
 difficult to plan query number 3
 Match on multiple labels with OR
+Variable length path with both sides already bound
 
 //AggregationAcceptance.feature
 Multiple aggregations should work

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

@@ -167,3 +167,22 @@ Feature: MatchAcceptance
       | r.bar |
       | 2     |
     And no side effects
+
+  Scenario: Variable length path with both sides already bound
+    Given an empty graph
+    And having executed:
+      """
+      CREATE (a:A {id: 1})-[:T]->(b:B {id: 2})
+      CREATE (a)-[:S]->(b)
+      CREATE (a)-[:T]->(:B)
+      CREATE (a)-[:T]->()-[:T]->(:B)
+      """
+    When executing query:
+      """
+      MATCH (a:A {id: 1})-[:S]->(b:B {id: 2}), (a)-[:T*1..2]->(b)
+      RETURN DISTINCT a.id, b.id
+      """
+    Then the result should be:
+      | a.id | b.id |
+      | 1    | 2    |
+    And no side effects