Permalink
Browse files

Fixed problems with node predicates being evaluated at the wrong point

  • Loading branch information...
1 parent c13774f commit 5529472eff148b9758fc34647adcde3ceb68abb4 @systay committed Oct 25, 2012
@@ -22,9 +22,15 @@ package org.neo4j.cypher.internal.executionplan.builders
import org.neo4j.cypher.internal.commands.{VarLengthRelatedTo, RelatedTo, Pattern, Predicate}
import org.neo4j.graphdb.Direction
import org.neo4j.cypher.internal.commands.expressions.{Identifier, Property}
-import org.neo4j.cypher.internal.pipes.matching.{EndPoint, VariableLengthStepTrail, SingleStepTrail, Trail}
+import org.neo4j.cypher.internal.pipes.matching._
import org.neo4j.helpers.ThisShouldNotHappenError
import annotation.tailrec
+import org.neo4j.cypher.internal.pipes.matching.VariableLengthStepTrail
+import scala.Some
+import org.neo4j.cypher.internal.executionplan.builders.LongestTrail
+import org.neo4j.cypher.internal.pipes.matching.EndPoint
+import org.neo4j.cypher.internal.pipes.matching.SingleStepTrail
+import org.neo4j.cypher.internal.commands.expressions.Property
object TrailBuilder {
def findLongestTrail(patterns: Seq[Pattern], boundPoints: Seq[String], predicates: Seq[Predicate] = Seq.empty) =
@@ -45,14 +51,24 @@ final class TrailBuilder(patterns: Seq[Pattern], boundPoints: Seq[String], predi
}
def transformToTrail(p: Pattern, done: Trail, patternsToDo: Seq[Pattern]): (Trail, Seq[Pattern]) = {
- def predicateRewriter(from: String, to: String)(pred: Predicate) = pred.rewrite {
- case Identifier(name) if name == from => Identifier(to)
- case Property(name, prop) if name == from => Property(to, prop)
- case e => e
+
+ def nodePredicateRewriter(originalName: String)(pred: Predicate) = pred.rewrite {
+ case Identifier(name) if name == originalName => NodeIdentifier(name)
+ case Property(name, prop) if name == originalName => MiniMapNodeProperty(name, prop)
+ case e => e
+ }
+
+ def relPredicateRewriter(originalName: String)(pred: Predicate) = pred.rewrite {
+ case Identifier(name) if name == originalName => RelationshipIdentifier(name)
+ case Property(name, prop) if name == originalName => MiniMapRelProperty(name, prop)
+ case e => e
+ }
+
+ def relPred(k: String) = predicates.find(createFinder(k)).map(relPredicateRewriter(k))
+ def nodePred(k: String) = predicates.find(createFinder(k)).map(nodePredicateRewriter(k))
+ def singleStep(rel: RelatedTo, end: String, dir: Direction) = {
+ done.add(start => SingleStepTrail(EndPoint(end), dir, rel.relName, rel.relTypes, start, relPred(rel.relName), nodePred(end), rel))
}
- def relPred(k: String) = predicates.find(createFinder(k)).map(predicateRewriter(k, "r"))
- def nodePred(k: String) = predicates.find(createFinder(k)).map(predicateRewriter(k, "n"))
- def singleStep(rel: RelatedTo, end: String, dir: Direction) = done.add(start => SingleStepTrail(EndPoint(end), dir, rel.relName, rel.relTypes, start, relPred(rel.relName), nodePred(start), rel))
def multiStep(rel: VarLengthRelatedTo, end: String, dir: Direction) = done.add(start => VariableLengthStepTrail(EndPoint(end), dir, rel.relTypes, rel.minHops.getOrElse(1), rel.maxHops, rel.pathName, rel.relIterator, start, rel))
val patternsLeft = patternsToDo.filterNot(_ == p)
@@ -42,7 +42,7 @@ class BidirectionalTraversalMatcher(steps: ExpanderStep,
val initialEndStep = new InitialStateFactory[Option[ExpanderStep]] {
def initialState(path: Path): Option[ExpanderStep] = Some(reversedSteps)
}
- val baseTraversal: TraversalDescription = Traversal.traversal(Uniqueness.NODE_PATH)
+ val baseTraversal: TraversalDescription = Traversal.traversal(Uniqueness.RELATIONSHIP_PATH)
val collisionDetector = new StepCollisionDetector
def findMatchingPaths(state: QueryState, context: ExecutionContext): Iterable[Path] = {
@@ -92,11 +92,11 @@ class BidirectionalTraversalMatcher(steps: ExpanderStep,
case (Some(x), None) =>
val result = startPath.length() == 0
- (result, result)
+ (result, true)
case (None, Some(x)) =>
val result = endPath.length() == 0
- (result, result)
+ (result, true)
case _ => throw new ThisShouldNotHappenError("Andres", "Unexpected traversal state encountered")
}
@@ -19,23 +19,39 @@
*/
package org.neo4j.cypher.internal.pipes.matching
-import org.neo4j.graphdb.{Node, Relationship, Direction, RelationshipType}
+import org.neo4j.graphdb._
import org.neo4j.cypher.internal.commands.{True, Predicate}
import collection.mutable
import org.neo4j.cypher.internal.pipes.ExecutionContext
+import org.neo4j.cypher.internal.commands.expressions.Expression
+import org.neo4j.cypher.internal.symbols.SymbolTable
+import org.neo4j.cypher.internal.pipes.matching.MiniMap
+import scala.Some
+import org.neo4j.cypher.internal.commands.True
+import org.neo4j.cypher.EntityNotFoundException
+import org.neo4j.helpers.ThisShouldNotHappenError
trait ExpanderStep {
def next: Option[ExpanderStep]
+
def typ: Seq[RelationshipType]
+
def direction: Direction
+
def id: Int
+
def relPredicate: Predicate
+
def nodePredicate: Predicate
- def createCopy(next:Option[ExpanderStep], direction:Direction, nodePredicate:Predicate):ExpanderStep
- def size:Option[Int]
+
+ def createCopy(next: Option[ExpanderStep], direction: Direction, nodePredicate: Predicate): ExpanderStep
+
+ def size: Option[Int]
+
def expand(node: Node, parameters: ExecutionContext): (Iterable[Relationship], Option[ExpanderStep])
- def shouldInclude():Boolean
+
+ def shouldInclude(): Boolean
/*
The way we reverse the steps is by first creating a Seq out of the steps. In this Seq, the first element points to
@@ -73,16 +89,73 @@ trait ExpanderStep {
}
}
+abstract class MiniMapProperty(originalName: String, prop: String) extends Expression {
+ protected def calculateType(symbols: SymbolTable) = fail()
+
+ def filter(f: (Expression) => Boolean) = fail()
+
+ def rewrite(f: (Expression) => Expression) = fail()
+
+ def symbolTableDependencies = fail()
+
+ def apply(ctx: ExecutionContext) = {
+ ctx match {
+ case m: MiniMap => {
+ val pc = extract(m)
+ try {
+ pc.getProperty(prop)
+ } catch {
+ case x: NotFoundException =>
+ throw new EntityNotFoundException("The property '%s' does not exist on %s, which was found with the identifier: %s".format(prop, pc, originalName), x)
+ }
+ }
+ case _ => fail()
+ }
+ }
+
+
+ protected def fail() = throw new ThisShouldNotHappenError("Andres", "This predicate should never be used outside of the traversal matcher")
+
+ protected def extract(m: MiniMap): PropertyContainer
+}
+
+abstract class MiniMapIdentifier(originalName:String) extends Expression {
+ protected def calculateType(symbols: SymbolTable) = fail()
+
+ def filter(f: (Expression) => Boolean) = fail()
+
+ def rewrite(f: (Expression) => Expression) = fail()
+
+ def symbolTableDependencies = fail()
+
+ def apply(ctx: ExecutionContext) = ctx match {
+ case m: MiniMap => extract(m)
+ case _ => fail()
+ }
+
+ protected def extract(m: MiniMap): PropertyContainer
+
+ def fail() = throw new ThisShouldNotHappenError("Andres", "This predicate should never be used outside of the traversal matcher")
+}
+
+case class MiniMapRelProperty(originalName: String, prop: String) extends MiniMapProperty(originalName, prop) {
+ protected def extract(m: MiniMap) = m.relationship
+}
+
+case class MiniMapNodeProperty(originalName: String, prop: String) extends MiniMapProperty(originalName, prop) {
+ protected def extract(m: MiniMap) = m.node
+}
+
+case class NodeIdentifier(name:String) extends MiniMapIdentifier(name) {
+ protected def extract(m: MiniMap) = m.node
+}
+
+case class RelationshipIdentifier(name:String) extends MiniMapIdentifier(name) {
+ protected def extract(m: MiniMap) = m.relationship
+}
-class MiniMap(r: Relationship, n: Node, parameters: ExecutionContext)
+case class MiniMap(relationship: Relationship, node: Node, parameters: ExecutionContext)
extends ExecutionContext(params = parameters.params) {
- override def get(key: String): Option[Any] =
- if (key == "r")
- Some(r)
- else if (key == "n")
- Some(n)
- else
- parameters.get(key)
override def iterator = throw new RuntimeException
@@ -2256,4 +2256,18 @@ RETURN x0.name?
assert(result.toList === List(Map("tail([])" -> List())))
}
+
+ @Test
+ def nodes_named_r_should_not_pose_a_problem() {
+ val a = createNode()
+ val r = createNode("foo"->"bar")
+ val b = createNode()
+
+ relate(a,r)
+ relate(r,b)
+
+ val result = parseAndExecute("START a=node(1) MATCH a-->r-->b WHERE r.foo = 'bar' RETURN b")
+
+ assert(result.toList === List(Map("b" -> b)))
+ }
}
@@ -97,8 +97,8 @@ class TrailBuilderTest extends GraphDatabaseTestBase with Assertions with Builde
val r2Pred = Equals(Property("pr2", "prop"), Literal("FOO"))
val predicates = Seq(r1Pred, r2Pred)
- val rewrittenR1 = Equals(Property("r", "prop"), Literal(42))
- val rewrittenR2 = Equals(Property("r", "prop"), Literal("FOO"))
+ val rewrittenR1 = Equals(MiniMapRelProperty("pr1", "prop"), Literal(42))
+ val rewrittenR2 = Equals(MiniMapRelProperty("pr2", "prop"), Literal("FOO"))
val boundPoint = EndPoint("c")
val second = SingleStepTrail(boundPoint, Direction.OUTGOING, "pr2", Seq("B"), "b", relPred = Some(rewrittenR2), nodePred = None, pattern = BtoC)
@@ -116,11 +116,11 @@ class TrailBuilderTest extends GraphDatabaseTestBase with Assertions with Builde
val nodePred = Equals(Property("b", "prop"), Literal(42))
val predicates = Seq(nodePred)
- val rewrittenPredicate = Equals(Property("n", "prop"), Literal(42))
+ val rewrittenPredicate = Equals(MiniMapNodeProperty("b", "prop"), Literal(42))
val boundPoint = EndPoint("c")
- val second = SingleStepTrail(boundPoint, Direction.OUTGOING, "pr2", Seq("B"), "b", None, Some(rewrittenPredicate), BtoC)
- val first = SingleStepTrail(second, Direction.OUTGOING, "pr1", Seq("A"), "a", None, None, AtoB)
+ val second = SingleStepTrail(boundPoint, Direction.OUTGOING, "pr2", Seq("B"), "b", None, None, BtoC)
+ val first = SingleStepTrail(second, Direction.OUTGOING, "pr1", Seq("A"), "a", None, Some(rewrittenPredicate), AtoB)
val expectedTrail = Some(LongestTrail("a", Some("c"), first))
val foundTrail = TrailBuilder.findLongestTrail(Seq(AtoB, BtoC, BtoB2), Seq("a", "c"), predicates)

0 comments on commit 5529472

Please sign in to comment.