From 6c8c5763843ef95d9cad8ee618284e703f03e064 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Tue, 9 May 2017 10:47:50 +0200 Subject: [PATCH] Make sure range queries are only used against single column indexes --- .../compiler/v3_2/commands/indexQuery.scala | 2 +- .../steps/AbstractIndexSeekLeafPlanner.scala | 14 ++++++++-- .../AggregationAcceptanceTest.scala | 6 ----- .../CompositeIndexAcceptanceTest.scala | 27 ++++++++++++++++++- 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/community/cypher/cypher-compiler-3.2/src/main/scala/org/neo4j/cypher/internal/compiler/v3_2/commands/indexQuery.scala b/community/cypher/cypher-compiler-3.2/src/main/scala/org/neo4j/cypher/internal/compiler/v3_2/commands/indexQuery.scala index aca145605757..d533a453e4e5 100644 --- a/community/cypher/cypher-compiler-3.2/src/main/scala/org/neo4j/cypher/internal/compiler/v3_2/commands/indexQuery.scala +++ b/community/cypher/cypher-compiler-3.2/src/main/scala/org/neo4j/cypher/internal/compiler/v3_2/commands/indexQuery.scala @@ -107,7 +107,7 @@ object indexQuery extends GraphElementPropertyFunctions { throw new InternalException("A CompositeQueryExpression can't be nested in a CompositeQueryExpression") case RangeQueryExpression(rangeWrapper) => - throw new InternalException("Range queries on composite indexea not yet supported") + throw new InternalException("Range queries on composite indexes not yet supported") } } } diff --git a/community/cypher/cypher-compiler-3.2/src/main/scala/org/neo4j/cypher/internal/compiler/v3_2/planner/logical/steps/AbstractIndexSeekLeafPlanner.scala b/community/cypher/cypher-compiler-3.2/src/main/scala/org/neo4j/cypher/internal/compiler/v3_2/planner/logical/steps/AbstractIndexSeekLeafPlanner.scala index 9bfe3ccf1b00..a745ac798458 100644 --- a/community/cypher/cypher-compiler-3.2/src/main/scala/org/neo4j/cypher/internal/compiler/v3_2/planner/logical/steps/AbstractIndexSeekLeafPlanner.scala +++ b/community/cypher/cypher-compiler-3.2/src/main/scala/org/neo4j/cypher/internal/compiler/v3_2/planner/logical/steps/AbstractIndexSeekLeafPlanner.scala @@ -20,7 +20,7 @@ package org.neo4j.cypher.internal.compiler.v3_2.planner.logical.steps import org.neo4j.cypher.internal.compiler.v3_2.IndexDescriptor -import org.neo4j.cypher.internal.compiler.v3_2.commands.{CompositeQueryExpression, QueryExpression, SingleQueryExpression} +import org.neo4j.cypher.internal.compiler.v3_2.commands.{CompositeQueryExpression, ManyQueryExpression, QueryExpression, SingleQueryExpression} import org.neo4j.cypher.internal.compiler.v3_2.planner.logical.LeafPlansForVariable.maybeLeafPlans import org.neo4j.cypher.internal.compiler.v3_2.planner.logical.plans._ import org.neo4j.cypher.internal.compiler.v3_2.planner.logical.{LeafPlanFromExpressions, LeafPlanner, LeafPlansForVariable, LogicalPlanningContext} @@ -177,12 +177,22 @@ abstract class AbstractIndexSeekLeafPlanner extends LeafPlanner with LeafPlanFro } // Currently we only support using the composite index if ALL properties are specified, but this could be generalized - if (foundPredicates.length == indexDescriptor.properties.length) + if (foundPredicates.length == indexDescriptor.properties.length && isSupportedByCurrentIndexes(foundPredicates)) Some(foundPredicates) else None } + private def isSupportedByCurrentIndexes(foundPredicates: Seq[IndexPlannableExpression]) = { + // We currently only support range queries against single prop indexes + foundPredicates.length == 1 || + foundPredicates.forall(_.queryExpression match { + case _: SingleQueryExpression[_] => true + case _: ManyQueryExpression[_] => true + case _ => false + }) + } + case class IndexPlannableExpression(name: String, propertyKeyName: PropertyKeyName, propertyPredicate: Expression, queryExpression: QueryExpression[Expression], hints: Set[Hint], argumentIds: Set[IdName]) diff --git a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/AggregationAcceptanceTest.scala b/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/AggregationAcceptanceTest.scala index 6727ff9578c6..c54518f3c425 100644 --- a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/AggregationAcceptanceTest.scala +++ b/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/AggregationAcceptanceTest.scala @@ -23,12 +23,6 @@ import org.neo4j.cypher.{ExecutionEngineFunSuite, NewPlannerTestSupport} class AggregationAcceptanceTest extends ExecutionEngineFunSuite with NewPlannerTestSupport { - test("apa") { - val r = graph.execute("cypher debug=generate_java_source debug=show_java_source match (n) return n") - while(r.hasNext) r.next() - println(r.getExecutionPlanDescription) - } - // Non-deterministic query -- needs TCK design test("should aggregate using as grouping key expressions using variables in scope and nothing else") { val userId = createLabeledNode(Map("userId" -> 11), "User") diff --git a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/CompositeIndexAcceptanceTest.scala b/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/CompositeIndexAcceptanceTest.scala index a36b7b935160..8a1e87146e67 100644 --- a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/CompositeIndexAcceptanceTest.scala +++ b/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/CompositeIndexAcceptanceTest.scala @@ -271,7 +271,7 @@ class CompositeIndexAcceptanceTest extends ExecutionEngineFunSuite with NewPlann graph should haveIndexes(":L(foo,bar,baz)") val result = executeWithAllPlanners("MATCH (n:L {foo: 42, bar: 1337, baz: 1980}) RETURN count(n)") result should evaluateTo(Seq(Map("count(n)" -> 1))) - result should useIndex(":L(foo,bar,baz") + result should useIndex(":L(foo,bar,baz)") } test("should not fail on multiple attempts to create a composite index") { @@ -280,6 +280,31 @@ class CompositeIndexAcceptanceTest extends ExecutionEngineFunSuite with NewPlann executeWithCostPlannerAndInterpretedRuntimeOnly("CREATE INDEX ON :Person(firstname, lastname)") } + test("should not use range queries against a composite index") { + // given + graph.createIndex("X", "p1", "p2") + val n = createLabeledNode(Map("p1" -> 1, "p2" -> 1), "X") + val result = executeWithAllPlanners("match (n:X) where n.p1 = 1 AND n.p2 > 0 return n;") + + result should evaluateTo(Seq(Map("n" -> n))) + result shouldNot useIndex(":X(p1,p2)") + } + + test("nested index join with composite indexes") { + // given + graph.createIndex("X", "p1", "p2") + (1 to 1000) foreach { _ => // Get the planner to do what we expect it to! + createLabeledNode("X") + } + val a = createNode("p1" -> 1, "p2" -> 1) + val b = createLabeledNode(Map("p1" -> 1, "p2" -> 1), "X") + val result = executeWithAllPlanners("match (a), (b:X) where id(a) = $id AND b.p1 = a.p1 AND b.p2 = 1 return b", + "id" -> a.getId) + + result should evaluateTo(Seq(Map("b" -> b))) + result should useIndex(":X(p1,p2)") + } + case class haveIndexes(expectedIndexes: String*) extends Matcher[GraphDatabaseQueryService] { def apply(graph: GraphDatabaseQueryService): MatchResult = { graph.inTx {