Merge remote-tracking branch 'neo/3.2' into 3.3

neo4j · Aug 21, 2018 · 9a922aa · 9a922aa
2 parents 25473ad + 26b8bb4
commit 9a922aa
Show file tree

Hide file tree

Showing 3 changed files with 114 additions and 3 deletions.
diff --git a/...n/scala/org/neo4j/cypher/internal/compiler/v3_3/planner/logical/steps/OrLeafPlanner.scala b/...n/scala/org/neo4j/cypher/internal/compiler/v3_3/planner/logical/steps/OrLeafPlanner.scala
@@ -55,12 +55,14 @@ case class OrLeafPlanner(inner: Seq[LeafPlanFromExpressions]) extends LeafPlanne
               None
             case plans =>
               // We need to collect the predicates to be able to update solved correctly. After finishing to build the
-              // OR plan, we will report solving the OR predicate, but also other predicates solved.
+              // OR plan, we will report solving the OR predicate, but also other predicates which are covered by ALL
+              // underlying plans are solved.
               val predicates = collection.mutable.HashSet[Expression]()
+              predicates ++= coveringPredicates(plans.head)
+
               val singlePlan = plans.reduce[LogicalPlan] {
                 case (p1, p2) =>
-                  predicates ++= coveringPredicates(p1)
-                  predicates ++= coveringPredicates(p2)
+                  predicates --= (predicates diff coveringPredicates(p2).toSet)
                   producer.planUnion(p1, p2)
               }
               val orPlan = context.logicalPlanProducer.planDistinctStar(singlePlan)

diff --git a/...org/neo4j/cypher/internal/compiler/v3_3/planner/logical/LeafPlanningIntegrationTest.scala b/...org/neo4j/cypher/internal/compiler/v3_3/planner/logical/LeafPlanningIntegrationTest.scala
@@ -722,6 +722,59 @@ class LeafPlanningIntegrationTest extends CypherFunSuite with LogicalPlanningTes
     plan should equal(distinct)
   }
 
+  test("should be able to OR together two index seeks with different labels")
+  {
+    val plan = (new given {
+      indexOn("Label1", "prop1")
+      indexOn("Label2", "prop2")
+    } getLogicalPlanFor "MATCH (n:Label1:Label2) WHERE n.prop1 = 'val' OR n.prop2 = 'val' RETURN n")._2
+
+    val propPredicate = SingleQueryExpression(StringLiteral("val")(pos))
+    val prop1 = PropertyKeyToken("prop1", PropertyKeyId(0))
+    val prop2 = PropertyKeyToken("prop2", PropertyKeyId(1))
+    val labelPredicate1 = HasLabels(Variable("n")(pos), Seq(LabelName("Label1")(pos)))(pos)
+    val labelPredicate2 = HasLabels(Variable("n")(pos), Seq(LabelName("Label2")(pos)))(pos)
+    val labelToken1 = LabelToken("Label1", LabelId(0))
+    val labelToken2 = LabelToken("Label2", LabelId(1))
+
+    val seek1: NodeIndexSeek = NodeIndexSeek("n", labelToken1, Seq(prop1), propPredicate, Set.empty)(solved)
+    val seek2: NodeIndexSeek = NodeIndexSeek("n", labelToken2, Seq(prop2), propPredicate, Set.empty)(solved)
+    val union: Union = Union(seek2, seek1)(solved)
+    val distinct = Distinct(union, Map("n" -> varFor("n")))(solved)
+    val filter = Selection(Seq(labelPredicate1, labelPredicate2), distinct)(solved)
+
+    plan should equal(filter)
+  }
+
+  test("should be able to OR together four index seeks")
+  {
+    val plan = (new given {
+      indexOn("Label1", "prop1")
+      indexOn("Label1", "prop2")
+      indexOn("Label2", "prop1")
+      indexOn("Label2", "prop2")
+    } getLogicalPlanFor "MATCH (n:Label1:Label2) WHERE n.prop1 = 'val' OR n.prop2 = 'val' RETURN n")._2
+
+    val propPredicate = SingleQueryExpression(StringLiteral("val")(pos))
+    val prop1 = PropertyKeyToken("prop1", PropertyKeyId(0))
+    val prop2 = PropertyKeyToken("prop2", PropertyKeyId(1))
+    val labelPredicate1 = HasLabels(Variable("n")(pos), Seq(LabelName("Label1")(pos)))(pos)
+    val labelPredicate2 = HasLabels(Variable("n")(pos), Seq(LabelName("Label2")(pos)))(pos)
+    val labelToken1 = LabelToken("Label1", LabelId(0))
+    val labelToken2 = LabelToken("Label2", LabelId(1))
+
+    val seek1: NodeIndexSeek = NodeIndexSeek("n", labelToken1, Seq(prop1), propPredicate, Set.empty)(solved)
+    val seek2: NodeIndexSeek = NodeIndexSeek("n", labelToken1, Seq(prop2), propPredicate, Set.empty)(solved)
+    val seek3: NodeIndexSeek = NodeIndexSeek("n", labelToken2, Seq(prop1), propPredicate, Set.empty)(solved)
+    val seek4: NodeIndexSeek = NodeIndexSeek("n", labelToken2, Seq(prop2), propPredicate, Set.empty)(solved)
+
+    val union: Union = Union( Union( Union(seek2, seek4)(solved), seek1)(solved), seek3)(solved)
+    val distinct = Distinct(union, Map("n" -> varFor("n")))(solved)
+    val filter = Selection(Seq(labelPredicate1, labelPredicate2), distinct)(solved)
+
+    plan should equal(filter)
+  }
+
   //---------------------------------------------------------------------------
   // Test expand order with multiple configurations and
   // unsupported.cypher.plan_with_minimum_cardinality_estimates setting

diff --git a/...ite/src/test/scala/org/neo4j/internal/cypher/acceptance/NodeIndexSeekAcceptanceTest.scala b/...ite/src/test/scala/org/neo4j/internal/cypher/acceptance/NodeIndexSeekAcceptanceTest.scala
@@ -357,6 +357,62 @@ class NodeIndexSeekAcceptanceTest extends ExecutionEngineFunSuite with CypherCom
     result.toList should equal(List(Map("n" -> node1)))
   }
 
+  test("should not return any rows for OR predicates with different labels gh#12017") {
+    // Given
+    graph.createIndex("Label1", "prop1")
+    graph.createIndex("Label2", "prop2")
+    graph.execute("CREATE(:Label1 {prop1: 'val'})" )
+
+    // When
+    val result = executeWith(Configs.CommunityInterpreted, "MATCH (n:Label1:Label2) WHERE n.prop1 = 'val' OR n.prop2 = 'val' RETURN n",
+      planComparisonStrategy = ComparePlansWithAssertion(_ should useOperatorTimes("NodeIndexSeek", 2),
+        expectPlansToFail = Configs.Cost2_3 + Configs.Cost3_1 + Configs.AllRulePlanners),
+      //TODO: Remove when 3.2.13 is released
+      expectedDifferentResults = Configs.Version3_2)
+
+    // Then
+    result.toList should be (empty)
+  }
+
+  test("should be able to solve OR predicates with same label") {
+    // Given
+    graph.createIndex("Label1", "prop1")
+    graph.createIndex("Label1", "prop2")
+    val node1 = createLabeledNode(Map("prop1" -> "val"), "Label1")
+    val node2 = createLabeledNode(Map("prop2" -> "anotherVal"), "Label1")
+
+    // When
+    val result = executeWith(Configs.CommunityInterpreted, "MATCH (n:Label1) WHERE n.prop1 = 'val' OR n.prop2 = 'val' RETURN n",
+      planComparisonStrategy = ComparePlansWithAssertion(_ should useOperatorTimes("NodeIndexSeek", 2),
+        expectPlansToFail = Configs.Cost2_3 + Configs.Cost3_1 + Configs.AllRulePlanners))
+
+    // Then
+    result.toList should equal(List(Map("n" -> node1)))
+  }
+
+  test("should not return any rows for OR predicates with four indexes") {
+    // Given
+    graph.createIndex("Label1", "prop1")
+    graph.createIndex("Label1", "prop2")
+    graph.createIndex("Label2", "prop1")
+    graph.createIndex("Label2", "prop2")
+
+    for( i <- 1 to 10 ) {
+      graph.execute("CREATE(:Label1 {prop1: 'val', prop2: 'val'})" )
+      graph.execute("CREATE(:Label2 {prop1: 'val', prop2: 'val'})" )
+    }
+
+    // When
+    val result = executeWith(Configs.CommunityInterpreted, "MATCH (n:Label1:Label2) WHERE n.prop1 = 'val' OR n.prop2 = 'val' RETURN n",
+      planComparisonStrategy = ComparePlansWithAssertion(_ should useOperatorTimes("NodeIndexSeek", 4),
+        expectPlansToFail = Configs.Cost2_3 + Configs.Cost3_1 + Configs.AllRulePlanners),
+      //TODO: Remove when 3.2.13 is released
+      expectedDifferentResults = Configs.Version3_2)
+
+    // Then
+    result.toList should be (empty)
+  }
+
   private def setUpDatabaseForTests() {
     executeWith(Configs.All - Configs.Compiled - Configs.Cost2_3,
       """CREATE (architect:Matrix { name:'The Architect' }),