Commit 6faf33ab authored by Davide Grohmann's avatar Davide Grohmann Committed by Andres Taylor
Browse files

Fix problem about not using indexes when multiple labels are present on a node

o fix conflicts between Ronja rewriters and legacy planner
o added more index acceptance tests
o fix the legacy planner builder to make sure indexes can use values generated by unwind
parent 57506bda
Showing with 64 additions and 30 deletions
+64 -30
......@@ -41,7 +41,6 @@ class ASTRewriter(rewritingMonitor: AstRewritingMonitor, shouldExtractParameters
rewriters += normalizeEqualsArgumentOrder
rewriters += reattachAliasedExpressions
rewriters += addUniquenessPredicates
rewriters += CNFNormalizer // <- do not add any new predicates after this rewriter!
rewriters += expandStar
rewriters += isolateAggregation
rewriters += aliasReturnItems
......
......@@ -124,6 +124,7 @@ The Neo4j Team""")
*/
private def prepare = new Phase {
def myBuilders: Seq[PlanBuilder] = Seq(
new UnwindBuilder,
new LoadCSVBuilder,
new PredicateRewriter,
new KeyTokenResolver,
......@@ -136,7 +137,6 @@ The Neo4j Team""")
private def matching = new Phase {
def myBuilders: Seq[PlanBuilder] = Seq(
new UnwindBuilder,
new TraversalMatcherBuilder,
new FilterBuilder,
new NamedPathBuilder,
......
......@@ -45,9 +45,7 @@ case class Planner(monitors: Monitors,
producePlan(inputQuery.statement, inputQuery.semanticTable, inputQuery.queryText)(planContext)
private def producePlan(statement: Statement, semanticTable: SemanticTable, query: String)(planContext: PlanContext): PipeInfo = {
// TODO: When Ronja is the only planner around, move this to ASTRewriter
val rewrittenStatement = rewriteStatement(statement)
rewrittenStatement match {
Planner.rewriteStatement(statement) match {
case ast: Query =>
monitor.startedPlanning(query)
val logicalPlan = produceQueryPlan(ast, semanticTable)(planContext).plan
......@@ -61,17 +59,6 @@ case class Planner(monitors: Monitors,
}
}
def rewriteStatement(statement: Statement) = {
val namedStatement = statement.rewrite(bottomUp(
inSequence(nameVarLengthRelationships, namePatternPredicates)
)).asInstanceOf[Statement]
val statementWithInlinedProjections = inlineProjections(namedStatement)
val statementWithAliasedSortSkipAndLimit = statementWithInlinedProjections.rewrite(bottomUp(useAliasesInSortSkipAndLimit))
statementWithAliasedSortSkipAndLimit
}
def produceQueryPlan(ast: Query, semanticTable: SemanticTable)(planContext: PlanContext): QueryPlan = {
tokenResolver.resolve(ast)(semanticTable, planContext)
val (plannerQuery, subQueriesLookupTable) = plannerQueryBuilder.produce(ast)
......@@ -84,6 +71,20 @@ case class Planner(monitors: Monitors,
}
}
object Planner {
def rewriteStatement(statement: Statement): Statement = {
val cnfStatement = statement.typedRewrite[Statement](CNFNormalizer)
val namedStatement = cnfStatement.typedRewrite[Statement](bottomUp(
inSequence(nameVarLengthRelationships, namePatternPredicates)
))
val statementWithInlinedProjections = inlineProjections(namedStatement)
val statementWithAliasedSortSkipAndLimit = statementWithInlinedProjections.typedRewrite[Statement](bottomUp(useAliasesInSortSkipAndLimit))
statementWithAliasedSortSkipAndLimit
}
}
trait PlanningMonitor {
def startedPlanning(q: String)
def foundPlan(q: String, p: LogicalPlan)
......
......@@ -25,6 +25,11 @@ import org.neo4j.cypher.internal.compiler.v2_1.Rewriter
class CNFNormalizerTest extends CypherFunSuite with PredicateTestSupport {
def rewriter: Rewriter = CNFNormalizer
test("should flatten multiple ANDs in a ANDS") {
and(P, and(R, S)) <=> ands(P, R, S)
and(and(R, S), P) <=> ands(R, S, P)
}
test("should be able to convert a dnf to cnf") {
or(and(P, Q), and(R, S)) <=> ands(ors(P, R), ors(Q, R), ors(P, S), ors(Q, S))
}
......
......@@ -28,7 +28,7 @@ import org.neo4j.cypher.internal.compiler.v2_1.spi.{GraphStatistics, PlanContext
import org.neo4j.cypher.internal.compiler.v2_1.parser.{ParserMonitor, CypherParser}
import org.neo4j.cypher.internal.compiler.v2_1.planner.logical._
import org.neo4j.cypher.internal.compiler.v2_1.planner.logical.plans._
import org.neo4j.cypher.internal.compiler.v2_1.ast.{PatternExpression, AstConstructionTestSupport, RelTypeName, Query}
import org.neo4j.cypher.internal.compiler.v2_1.ast._
import org.neo4j.cypher.internal.compiler.v2_1.planner.logical.Metrics._
import org.mockito.Mockito._
import org.mockito.Matchers._
......@@ -152,9 +152,9 @@ trait LogicalPlanningTestSupport
val parsedStatement = parser.parse(queryText)
semanticChecker.check(queryText, parsedStatement)
val (rewrittenStatement, _) = astRewriter.rewrite(queryText, parsedStatement)
planner.rewriteStatement(rewrittenStatement) match {
Planner.rewriteStatement(rewrittenStatement) match {
case ast: Query =>
val semanticTable = semanticChecker.check(queryText, rewrittenStatement)
val semanticTable = semanticChecker.check(queryText, ast)
tokenResolver.resolve(ast)(semanticTable, planContext)
planner.produceQueryPlan(ast, semanticTable)(planContext)
case _ =>
......
......@@ -249,7 +249,7 @@ trait LogicalPlanningTestSupport2 extends CypherTestSupport with AstConstruction
semanticChecker.check(queryString, parsedStatement)
val (rewrittenStatement, _) = astRewriter.rewrite(queryString, parsedStatement)
val semanticTable = semanticChecker.check(queryString, rewrittenStatement)
SemanticPlan(planner.rewriteStatement(rewrittenStatement) match {
SemanticPlan(Planner.rewriteStatement(rewrittenStatement) match {
case ast: Query =>
tokenResolver.resolve(ast)(semanticTable, planContext)
planner.produceQueryPlan(ast, semanticTable)(planContext).plan
......
......@@ -20,10 +20,9 @@
import org.neo4j.graphdb.Direction
import org.neo4j.cypher.internal.commons.CypherFunSuite
import org.neo4j.cypher.internal.compiler.v2_1.{inSequence, bottomUp}
import org.neo4j.cypher.internal.compiler.v2_1.planner._
import org.neo4j.cypher.internal.compiler.v2_1.ast._
import org.neo4j.cypher.internal.compiler.v2_1.ast.rewriters.{inlineProjections, namePatternPredicates, nameVarLengthRelationships}
import org.neo4j.cypher.internal.compiler.v2_1.ast.rewriters.inlineProjections
import org.neo4j.cypher.internal.compiler.v2_1.planner.logical.plans._
class SimplePlannerQueryBuilderTest extends CypherFunSuite with LogicalPlanningTestSupport {
......@@ -45,9 +44,8 @@ class SimplePlannerQueryBuilderTest extends CypherFunSuite with LogicalPlanningT
val rewrittenAst: Statement = if (normalize) {
val step1 = astRewriter.rewrite(query, ast)._1
val step2 = step1.rewrite(inSequence(nameVarLengthRelationships, namePatternPredicates)).asInstanceOf[Statement]
val step3 = inlineProjections(step2)
step3
val step2 = Planner.rewriteStatement(step1)
inlineProjections(step2)
} else {
ast
}
......
2.1.2
-----
o Allow Cypher to use indexes to solve IN queries
o Fix problems where indexes where not picked up if multiple labels
where used on a pattern node
o Fix problem where indexes where not picked up when a query uses UNWIND
2.1.0-M02
---------
......
......@@ -19,7 +19,21 @@
*/
package org.neo4j.cypher
class IndexUsageAcceptanceTest extends ExecutionEngineFunSuite {
test("should be able to use indexes") {
// Given
execute("CREATE (_0:Matrix { name:'The Architect' }),(_1:Matrix { name:'Agent Smith' }),(_2:Matrix:Crew { name:'Cypher' }),(_3:Crew { name:'Trinity' }),(_4:Crew { name:'Morpheus' }),(_5:Crew { name:'Neo' }), _1-[:CODED_BY]->_0, _2-[:KNOWS]->_1, _4-[:KNOWS]->_3, _4-[:KNOWS]->_2, _5-[:KNOWS]->_4, _5-[:LOVES]->_3")
graph.createIndex("Crew", "name")
// When
val result = execute("MATCH (n:Crew) WHERE n.name = 'Neo' RETURN n")
// Then
result.executionPlanDescription.toString should include("SchemaIndex")
}
test("should not forget predicates") {
// Given
execute("CREATE (_0:Matrix { name:'The Architect' }),(_1:Matrix { name:'Agent Smith' }),(_2:Matrix:Crew { name:'Cypher' }),(_3:Crew { name:'Trinity' }),(_4:Crew { name:'Morpheus' }),(_5:Crew { name:'Neo' }), _1-[:CODED_BY]->_0, _2-[:KNOWS]->_1, _4-[:KNOWS]->_3, _4-[:KNOWS]->_2, _5-[:KNOWS]->_4, _5-[:LOVES]->_3")
......@@ -27,20 +41,34 @@ class IndexUsageAcceptanceTest extends ExecutionEngineFunSuite {
// When
val result = execute("MATCH (n:Crew) WHERE n.name = 'Neo' AND n.name= 'Morpheus' RETURN n")
val result = execute("cypher 2.1 MATCH (n:Crew) WHERE n.name = 'Neo' AND n.name = 'Morpheus' RETURN n")
// Then
result shouldBe empty
result.executionPlanDescription.toString should include("SchemaIndex")
}
test("should use index when there are multiple labels on the node") {
// Given
execute("CREATE (_0:Matrix { name:'The Architect' }),(_1:Matrix { name:'Agent Smith' }),(_2:Matrix:Crew { name:'Cypher' }),(_3:Crew { name:'Trinity' }),(_4:Crew { name:'Morpheus' }),(_5:Crew { name:'Neo' }), _1-[:CODED_BY]->_0, _2-[:KNOWS]->_1, _4-[:KNOWS]->_3, _4-[:KNOWS]->_2, _5-[:KNOWS]->_4, _5-[:LOVES]->_3")
graph.createIndex("Crew", "name")
// When
val result = execute("MATCH (n:Matrix:Crew) WHERE n.name = 'Neo' RETURN n")
// Then
assert(result.isEmpty, "Should not find anything here")
result.executionPlanDescription.toString should include("SchemaIndex")
}
test("should handle dependencies coming from UNWIND properly") {
test("should be able to use value coming from UNWIND for index seek") {
// Given
graph.createIndex("Prop", "id")
// When
val result = execute("unwind [1,2,3] as x match (n:Prop) using index n:Prop(id) where n.id = x return *;")
val result = execute("unwind [1,2,3] as x match (n:Prop) where n.id = x return *;")
// Then
assert(result.isEmpty, "Should not find anything here")
result shouldBe empty
result.executionPlanDescription.toString should include("SchemaIndex")
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment