Permalink
Browse files

Fixes #797: CREATE UNIQUE now makes sure used identifiers have the sa…

…me properties even if they are re-used without the properties
  • Loading branch information...
1 parent e78b6c7 commit 6824c5505134d29d65c094957aabd8850af3467b @systay committed Aug 21, 2012
View
@@ -1,6 +1,8 @@
1.9
--------------------
o Predicates can now be returned and used to set properties
+o Fixes #797: CREATE UNIQUE now makes sure used identifiers have the same properties even if
+ they are re-used without the properties
1.8
--------------------
@@ -27,7 +27,8 @@ import org.neo4j.cypher.UniquePathNotUniqueException
import org.neo4j.graphdb.{Lock, PropertyContainer}
import org.neo4j.cypher.internal.commands.expressions.Expression
-case class CreateUniqueAction(links: UniqueLink*) extends StartItem("noooes") with Mutator with UpdateAction {
+case class CreateUniqueAction(incomingLinks: UniqueLink*) extends StartItem("noooes") with Mutator with UpdateAction {
+
def exec(context: ExecutionContext, state: QueryState): Traversable[ExecutionContext] = {
var linksToDo: Seq[UniqueLink] = links
var ctx = context
@@ -58,6 +59,18 @@ case class CreateUniqueAction(links: UniqueLink*) extends StartItem("noooes") wi
Stream(ctx)
}
+ /**
+ * Here we take the incoming links and prepare them to be used, by making sure that
+ * no named expectations contradict each other
+ */
+ val links:Seq[UniqueLink] = {
+ val nodesWithProperties: Seq[NamedExpectation] = incomingLinks.flatMap(_.nodesWProps)
+
+ nodesWithProperties.foldLeft(links) {
+ case (bunchOfLinks, nodeExpectation) => bunchOfLinks.map(link => link.expect(nodeExpectation))
+ }
+ }
+
private def tryAgain(linksToDo: Seq[UniqueLink], context: ExecutionContext, state: QueryState): ExecutionContext = {
val results: Seq[(UniqueLink, CreateUniqueResult)] = executeAllRemainingPatterns(linksToDo, context, state)
val updateCommands = extractUpdateCommands(results)
@@ -0,0 +1,55 @@
+/**
+ * Copyright (c) 2002-2012 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.cypher.internal.mutation
+
+import org.neo4j.cypher.internal.commands.IterableSupport
+import org.neo4j.cypher.internal.commands.expressions.Expression
+import org.neo4j.cypher.internal.symbols.{SymbolTable, TypeSafe}
+import org.neo4j.graphdb.PropertyContainer
+import org.neo4j.cypher.internal.pipes.ExecutionContext
+import collection.Map
+
+case class NamedExpectation(name: String, properties: Map[String, Expression])
+ extends GraphElementPropertyFunctions
+ with IterableSupport
+ with TypeSafe {
+ def this(name: String) = this(name, Map.empty)
+
+ def compareWithExpectations(pc: PropertyContainer, ctx: ExecutionContext): Boolean = properties.forall {
+ case ("*", expression) => getMapFromExpression(expression(ctx)).forall {
+ case (k, value) => pc.hasProperty(k) && pc.getProperty(k) == value
+ }
+ case (k, exp) =>
+ if (!pc.hasProperty(k)) false
+ else {
+ val expectationValue = exp(ctx)
+ val elementValue = pc.getProperty(k)
+
+ if (expectationValue == elementValue) true
+ else isCollection(expectationValue) && isCollection(elementValue) && makeTraversable(expectationValue).toList == makeTraversable(elementValue).toList
+ }
+ }
+
+ def symbolTableDependencies = symbolTableDependencies(properties)
+
+ def assertTypes(symbols: SymbolTable) {
+ checkTypes(properties, symbols)
+ }
+}
@@ -23,43 +23,14 @@ import org.neo4j.cypher.internal.commands.expressions.Expression
import org.neo4j.cypher.internal.commands._
import expressions.Identifier
import expressions.Literal
-import org.neo4j.cypher.internal.symbols.{RelationshipType, NodeType, SymbolTable, TypeSafe}
-import org.neo4j.graphdb.{Node, DynamicRelationshipType, Direction, PropertyContainer}
+import org.neo4j.cypher.internal.symbols.{RelationshipType, NodeType, SymbolTable}
+import org.neo4j.graphdb.{Node, DynamicRelationshipType, Direction}
import org.neo4j.cypher.internal.pipes.{QueryState, ExecutionContext}
-import org.neo4j.cypher.{CypherTypeException, UniquePathNotUniqueException}
+import org.neo4j.cypher.{SyntaxException, CypherTypeException, UniquePathNotUniqueException}
import collection.JavaConverters._
import collection.Map
import org.neo4j.cypher.internal.commands.CreateRelationshipStartItem
import org.neo4j.cypher.internal.commands.CreateNodeStartItem
-import scala.Some
-
-case class NamedExpectation(name: String, properties: Map[String, Expression])
- extends GraphElementPropertyFunctions
- with IterableSupport
- with TypeSafe {
- def this(name: String) = this(name, Map.empty)
-
- def compareWithExpectations(pc: PropertyContainer, ctx: ExecutionContext): Boolean = properties.forall {
- case ("*", expression) => getMapFromExpression(expression(ctx)).forall {
- case (k, value) => pc.hasProperty(k) && pc.getProperty(k) == value
- }
- case (k, exp) =>
- if (!pc.hasProperty(k)) false
- else {
- val expectationValue = exp(ctx)
- val elementValue = pc.getProperty(k)
-
- if (expectationValue == elementValue) true
- else isCollection(expectationValue) && isCollection(elementValue) && makeTraversable(expectationValue).toList == makeTraversable(elementValue).toList
- }
- }
-
- def symbolTableDependencies = symbolTableDependencies(properties)
-
- def assertTypes(symbols: SymbolTable) {
- checkTypes(properties, symbols)
- }
-}
object UniqueLink {
def apply(start: String, end: String, relName: String, relType: String, dir: Direction): UniqueLink =
@@ -73,13 +44,10 @@ case class UniqueLink(start: NamedExpectation, end: NamedExpectation, rel: Named
def exec(context: ExecutionContext, state: QueryState): Option[(UniqueLink, CreateUniqueResult)] = {
// We haven't yet figured out if we already have both elements in the context
// so let's start by finding that first
-
val s = getNode(context, start.name)
val e = getNode(context, end.name)
(s, e) match {
- case (None, None) => Some(this->CanNotAdvance())
-
case (Some(startNode), None) => oneNode(startNode, context, dir, state, end)
case (None, Some(startNode)) => oneNode(startNode, context, dir.reverse(), state, start)
@@ -89,9 +57,15 @@ case class UniqueLink(start: NamedExpectation, end: NamedExpectation, rel: Named
else
twoNodes(startNode, endNode, context, state)
}
+
+ case _ => Some(this -> CanNotAdvance())
}
}
+ // These are the nodes that have properties defined. They should always go first,
+ // so any other links that use these nodes have to have them locked.
+ def nodesWProps:Seq[NamedExpectation] = Seq(start,end).filter(_.properties.nonEmpty)
+
// This method sees if a matching relationship already exists between two nodes
// If any matching rels are found, they are returned. Otherwise, a new one is
// created and returned.
@@ -173,8 +147,6 @@ case class UniqueLink(start: NamedExpectation, end: NamedExpectation, rel: Named
"[%s:`%s`]".format(relName, relType)
}
-
-
def filter(f: (Expression) => Boolean) = Seq.empty
def assertTypes(symbols: SymbolTable) {
@@ -197,4 +169,30 @@ case class UniqueLink(start: NamedExpectation, end: NamedExpectation, rel: Named
def nodes = Seq(start.name, end.name)
def rels = Seq(rel.name)
+
+ /**
+ * Either returns a unique link where the expectation is respected,
+ * or throws an exception if a contradiction is found.
+ * @param expectation The named expectation to follow
+ * @return
+ */
+ def expect(expectation: NamedExpectation): UniqueLink =
+ if ((expectation.name != start.name) && (expectation.name != end.name)) {
+ this
+ } else {
+ val s = compareAndMatch(expectation, start)
+ val e = compareAndMatch(expectation, end)
+ copy(start = s, end = e)
+ }
+
+ private def compareAndMatch(expectation: NamedExpectation, current: NamedExpectation): NamedExpectation = {
+ if (current.name == expectation.name) {
+ if (current.properties.nonEmpty && current.properties != expectation.properties)
+ throw new SyntaxException("`%s` can't have properties assigned to it more than once in the CREATE UNIQUE statement".format(current.name))
+ else
+ expectation
+ } else {
+ current
+ }
+ }
}
@@ -266,6 +266,39 @@ FOREACH(name in ['a','b','c'] :
assert(bookTags === Set("a", "b", "c"))
}
+ @Test def should_find_nodes_with_properties_first() {
+ createNode()
+ val b = createNode()
+ val wrongX = createNode("foo" -> "absolutely not bar")
+ relate(b, wrongX, "X")
+
+ val result = parseAndExecute("""
+START a = node(1), b = node(2)
+CREATE UNIQUE
+ a-[:X]->()-[:X]->(x {foo:'bar'}),
+ b-[:X]->x
+RETURN x""")
+
+ assertStats(result, nodesCreated = 2, relationshipsCreated = 3, propertiesSet = 1)
+ }
+
+ @Test def should_find_nodes_with_properties_first2() {
+ createNode()
+ val b = createNode()
+ val wrongX = createNode("foo" -> "absolutely not bar")
+ relate(b, wrongX, "X")
+
+ val result = parseAndExecute("""
+START a = node(1), b = node(2)
+CREATE UNIQUE
+ a-[:X]->z-[:X]->(x {foo:'bar'}),
+ b-[:X]->x-[:X]->(z {foo:'buzz'})
+RETURN x""")
+
+ assertStats(result, nodesCreated = 2, relationshipsCreated = 4, propertiesSet = 2)
+ }
+
+
@Test
def two_outgoing_parts() {
val a = createNode()

0 comments on commit 6824c55

Please sign in to comment.