Merge remote-tracking branch 'upstream/3.0' into 3.1

neo4j · Mar 6, 2017 · 3e8c046 · 3e8c046
2 parents 760a2bb + bf1e789
commit 3e8c046
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 8 deletions.
diff --git a/.../neo4j/internal/cypher/acceptance/MatchAggregationsBackedByCountStoreAcceptanceTest.scala b/.../neo4j/internal/cypher/acceptance/MatchAggregationsBackedByCountStoreAcceptanceTest.scala
@@ -26,6 +26,24 @@ import org.scalatest.matchers.{MatchResult, Matcher}
 
 class MatchAggregationsBackedByCountStoreAcceptanceTest extends ExecutionEngineFunSuite with QueryStatisticsTestSupport with NewPlannerTestSupport {
 
+  test("do not plan counts store lookup for loop matches") {
+    val n = createNode()
+    // two loops
+    relate(n, n)
+    relate(n, n)
+    // one non-loop
+    relate(n, createNode())
+
+    val resultStar = executeWithAllPlannersAndCompatibilityMode("MATCH (a)-->(a) RETURN count(*)")
+    val resultVar = executeWithAllPlannersAndCompatibilityMode("MATCH (a)-[r]->(a) RETURN count(r)")
+
+    resultStar.toList should equal(List(Map("count(*)" -> 2)))
+    resultVar.toList should equal(List(Map("count(r)" -> 2)))
+
+    resultStar.executionPlanDescription() shouldNot includeOperation("RelationshipCountFromCountStore")
+    resultVar.executionPlanDescription() shouldNot includeOperation("RelationshipCountFromCountStore")
+  }
+
   test("counts nodes using count store") {
     // Given
     withModel(
@@ -34,7 +52,7 @@ class MatchAggregationsBackedByCountStoreAcceptanceTest extends ExecutionEngineF
       query = "MATCH (n) RETURN count(n)", f = { result =>
 
         // Then
-        result.columnAs("count(n)").toSet[Int] should equal(Set(2))
+        result.columnAs("count(n)").toSet[Int] should equal(Set(3))
 
     })
   }
@@ -47,7 +65,7 @@ class MatchAggregationsBackedByCountStoreAcceptanceTest extends ExecutionEngineF
       query = "MATCH (n) RETURN COUNT(n)", f = { result =>
 
         // Then
-        result.columnAs("COUNT(n)").toSet[Int] should equal(Set(2))
+        result.columnAs("COUNT(n)").toSet[Int] should equal(Set(3))
 
       })
   }
@@ -60,7 +78,7 @@ class MatchAggregationsBackedByCountStoreAcceptanceTest extends ExecutionEngineF
       query = "MATCH (n) RETURN count(*)", f = { result =>
 
         // Then
-        result.columnAs("count(*)").toSet[Int] should equal(Set(2))
+        result.columnAs("count(*)").toSet[Int] should equal(Set(3))
 
     })
   }
@@ -96,10 +114,10 @@ class MatchAggregationsBackedByCountStoreAcceptanceTest extends ExecutionEngineF
     withModel(
 
       // When
-      query = "MATCH (n) RETURN count(n)/2*5 as someNum", f = { result =>
+      query = "MATCH (n) RETURN count(n)/2.0*5 as someNum", f = { result =>
 
         // Then
-        result.columnAs("someNum").toSet[Int] should equal(Set(5))
+        result.columnAs("someNum").toSet[Int] should equal(Set(7.5))
 
       })
   }
@@ -112,7 +130,7 @@ class MatchAggregationsBackedByCountStoreAcceptanceTest extends ExecutionEngineF
       query = "MATCH ()-[r]->() RETURN count(r)", f = { result =>
 
         // Then
-        result.columnAs("count(r)").toSet[Int] should equal(Set(1))
+        result.columnAs("count(r)").toSet[Int] should equal(Set(2))
 
       })
   }
@@ -125,7 +143,7 @@ class MatchAggregationsBackedByCountStoreAcceptanceTest extends ExecutionEngineF
       query = "MATCH ()-->() RETURN count(*)", f = { result =>
 
         // Then
-        result.columnAs("count(*)").toSet[Int] should equal(Set(1))
+        result.columnAs("count(*)").toSet[Int] should equal(Set(2))
 
       })
   }
@@ -608,6 +626,7 @@ class MatchAggregationsBackedByCountStoreAcceptanceTest extends ExecutionEngineF
          |CREATE (p:$label1 {name: 'Petra'})
          |CREATE (s:$label2 {name: 'Steve'})
          |CREATE (p)-[:$type1]->(s)
+         |CREATE (a)-[:LOOP]->(a)
       """.stripMargin)
 
     val result: InternalExecutionResult = executeWithAllPlannersAndCompatibilityMode(query)

diff --git a/...ala/org/neo4j/cypher/internal/compiler/v3_1/planner/logical/steps/countStorePlanner.scala b/...ala/org/neo4j/cypher/internal/compiler/v3_1/planner/logical/steps/countStorePlanner.scala
@@ -88,13 +88,16 @@ case object countStorePlanner {
       val lpp = context.logicalPlanProducer
       val aggregation1 = lpp.planCountStoreNodeAggregation(query, IdName(columnName), label, argumentIds)(context)
       labelCheck(label)(Some(aggregation1))
-    } else if (patternRelationships.size == 1) { // MATCH ()-[r]->(), MATCH ()-[r:X]->(), MATCH ()-[r:X|Y]->()
+    } else if (patternRelationships.size == 1 && notLoop(patternRelationships.head)) { // MATCH ()-[r]->(), MATCH ()-[r:X]->(), MATCH ()-[r:X|Y]->()
       labelCheck(None)(
         trySolveRelationshipAggregation(query, columnName, variableName, patternRelationships, argumentIds, selections)
       )
     } else None
   }
 
+  // the counts store counts loops twice
+  private def notLoop(r: PatternRelationship) = r.nodes._1 != r.nodes._2
+
   private def trySolveRelationshipAggregation(query: PlannerQuery, columnName: String, variableName: Option[String],
                                               patternRelationships: Set[PatternRelationship], argumentIds: Set[IdName],
                                               selections: Selections)(implicit context: LogicalPlanningContext): Option[RelationshipCountFromCountStore] = {