Better error messages from CREATE UNIQUE problems

cypher/src/main/scala/org/neo4j/cypher/internal/commands/Pattern.scala

@@ -24,15 +24,15 @@ import org.neo4j.graphdb.Direction
 import collection.Seq
 import org.neo4j.cypher.internal.symbols._
 
-abstract class Pattern extends TypeSafe {
+trait Pattern extends TypeSafe {
   def optional: Boolean
   def predicate: Predicate
   def possibleStartPoints: Seq[(String,CypherType)]
   def relTypes:Seq[String]
 
   protected def node(name: String) = if (name.startsWith("  UNNAMED")) "()" else name
-  protected def left(dir: Direction) = if (dir == Direction.INCOMING) "<-" else "-"
-  protected def right(dir: Direction) = if (dir == Direction.OUTGOING) "->" else "-"
+  protected def leftArrow(dir: Direction) = if (dir == Direction.INCOMING) "<-" else "-"
+  protected def rightArrow(dir: Direction) = if (dir == Direction.OUTGOING) "->" else "-"
 
   def rewrite( f : Expression => Expression) : Pattern
   def equalOrUnnamed(name1: String, name2: String) = name1 == name2 || (name1.startsWith("  UNNAMED") && name2.startsWith("  UNNAMED"))
@@ -54,7 +54,7 @@ case class RelatedTo(left: String,
                      direction: Direction,
                      optional: Boolean,
                      predicate: Predicate) extends Pattern {
-  override def toString = node(left) + left(direction) + relInfo + right(direction) + node(right)
+  override def toString = node(left) + leftArrow(direction) + relInfo + rightArrow(direction) + node(right)
 
   private def relInfo: String = {
     var info = relName
@@ -119,7 +119,7 @@ case class VarLengthRelatedTo(pathName: String,
                               optional: Boolean,
                               predicate: Predicate) extends PathPattern {
 
-  override def toString: String = pathName + "=" + node(start) + left(direction) + relInfo + right(direction) + node(end)
+  override def toString: String = pathName + "=" + node(start) + leftArrow(direction) + relInfo + rightArrow(direction) + node(end)
 
   def symbolTableDependencies = predicate.symbolTableDependencies
 
@@ -179,7 +179,7 @@ case class ShortestPath(pathName: String,
                         relIterator: Option[String],
                         predicate: Predicate = True())
   extends PathPattern {
-  override def toString: String = pathName + "=" + algo + "(" + start + left(dir) + relInfo + right(dir) + end + ")"
+  override def toString: String = pathName + "=" + algo + "(" + start + leftArrow(dir) + relInfo + rightArrow(dir) + end + ")"
 
   private def algo = if (single) "singleShortestPath" else "allShortestPath"
 

cypher/src/main/scala/org/neo4j/cypher/internal/mutation/CreateUniqueAction.scala

@@ -76,18 +76,34 @@ case class CreateUniqueAction(links: UniqueLink*) extends StartItem("noooes") wi
     }
   }
 
-  private def traverseNextStep(nextSteps: Seq[(String, PropertyContainer)], oldContext: ExecutionContext): ExecutionContext = {
-    val uniqueKVPs = nextSteps.distinct
-    val uniqueKeys = nextSteps.toMap
+  case class TraverseResult(identifier: String, element: PropertyContainer, link: UniqueLink)
+
+  private def traverseNextStep(nextSteps: Seq[TraverseResult], oldContext: ExecutionContext): ExecutionContext = {
+    val uniqueKVPs = nextSteps.map(x => x.identifier -> x.element).distinct
+    val uniqueKeys = uniqueKVPs.toMap
 
     if (uniqueKeys.size != uniqueKVPs.size) {
-      //We can only go forward following a unique path. Fail.
-      throw new UniquePathNotUniqueException("The pattern " + this + " produced multiple possible paths, and that is not allowed")
+      fail(nextSteps)
     } else {
       oldContext.newWith(uniqueKeys)
     }
   }
 
+  private def fail(nextSteps: Seq[TraverseResult]): Nothing = {
+    //We can only go forward following a unique path. Fail.
+    val problemResultsByIdentifier: Map[String, Seq[TraverseResult]] = nextSteps.groupBy(_.identifier).
+      filter(_._2.size > 1)
+
+    val message = problemResultsByIdentifier.map {
+      case (identifier, links: Seq[TraverseResult]) =>
+        val hits = links.map(result => "%s found by : %s".format(result.element, result.link))
+
+        "Nodes for identifier: `%s` were found with differing values by these pattern relationships: %s".format(identifier, hits.mkString("\n  ", "\n  ", "\n"))
+    }
+
+    throw new UniquePathNotUniqueException(message.mkString("CREATE UNIQUE error\n", "\n", "\n"))
+  }
+
   private def runUpdateCommands(cmds: Seq[UpdateWrapper], oldContext: ExecutionContext, state: QueryState): ExecutionContext = {
     var context = oldContext
     var todo = cmds.distinct
@@ -122,10 +138,12 @@ case class CreateUniqueAction(links: UniqueLink*) extends StartItem("noooes") wi
       case _ => None
     }
 
-  private def extractTraversals(results: scala.Seq[(UniqueLink, CreateUniqueResult)]): Seq[(String, PropertyContainer)] =
+  private def extractTraversals(results: scala.Seq[(UniqueLink, CreateUniqueResult)]): Seq[TraverseResult] =
     results.flatMap {
-      case (_, Traverse(ctx@_*)) => ctx
-      case _ => None
+      case (link, Traverse(ctx@_*)) => ctx.map {
+        case (key, element) => TraverseResult(key, element, link)
+      }
+      case _                        => None
     }
 
   private def executeAllRemainingPatterns(linksToDo: Seq[UniqueLink], ctx: ExecutionContext, state: QueryState): Seq[(UniqueLink, CreateUniqueResult)] = linksToDo.flatMap(link => link.exec(ctx, state))

cypher/src/main/scala/org/neo4j/cypher/internal/mutation/UniqueLink.scala

@@ -19,14 +19,19 @@
  */
 package org.neo4j.cypher.internal.mutation
 
-import org.neo4j.cypher.internal.commands.expressions.{Identifier, Literal, Expression}
-import org.neo4j.cypher.internal.commands.{CreateNodeStartItem, CreateRelationshipStartItem, IterableSupport}
+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.pipes.{QueryState, ExecutionContext}
 import org.neo4j.cypher.{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
@@ -62,7 +67,7 @@ object UniqueLink {
 }
 
 case class UniqueLink(start: NamedExpectation, end: NamedExpectation, rel: NamedExpectation, relType: String, dir: Direction)
-  extends GraphElementPropertyFunctions {
+  extends GraphElementPropertyFunctions with Pattern {
   lazy val relationshipType = DynamicRelationshipType.withName(relType)
 
   def exec(context: ExecutionContext, state: QueryState): Option[(UniqueLink, CreateUniqueResult)] = {
@@ -127,7 +132,6 @@ case class UniqueLink(start: NamedExpectation, end: NamedExpectation, rel: Named
     }
   }
 
-
   private def createUpdateActions(dir: Direction, startNode: Node, end: NamedExpectation): Seq[UpdateWrapper] = {
     val createRel = if (dir == Direction.OUTGOING) {
       CreateRelationshipStartItem(rel.name, (Literal(startNode),Map()), (Identifier(end.name),Map()), relType, rel.properties)
@@ -161,11 +165,36 @@ case class UniqueLink(start: NamedExpectation, end: NamedExpectation, rel: Named
     UniqueLink(s, e, r, relType, dir)
   }
 
+  override def toString = node(start.name) + leftArrow(dir) + relInfo + rightArrow(dir) + node(end.name)
+
+  private def relInfo: String = {
+    val relName = if (rel.name.startsWith("  UNNAMED")) "" else "`" + rel.name + "`"
+
+    "[%s:`%s`]".format(relName, relType)
+  }
+
+
+
   def filter(f: (Expression) => Boolean) = Seq.empty
 
   def assertTypes(symbols: SymbolTable) {
     checkTypes(start.properties, symbols)
     checkTypes(end.properties, symbols)
     checkTypes(rel.properties, symbols)
   }
+
+  def optional: Boolean = false
+
+  def possibleStartPoints = Seq(
+    start.name -> NodeType(),
+    end.name -> NodeType(),
+    rel.name -> RelationshipType())
+
+  def predicate = True()
+
+  def relTypes = Seq(relType)
+
+  def nodes = Seq(start.name, end.name)
+
+  def rels = Seq(rel.name)
 }