Skip to content

Commit

Permalink
Fixes #834: MATCH with properties should throw an exception
Browse files Browse the repository at this point in the history
  • Loading branch information
systay authored and peterneubauer committed Aug 30, 2012
1 parent 3f88c29 commit ae0c296
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 14 deletions.
5 changes: 3 additions & 2 deletions cypher/CHANGES.txt
Expand Up @@ -9,8 +9,9 @@ o Fixed #795: so that WITH keeps parameters also for empty aggregation results
o Fixed #772: Creating nodes/relationships goes before SET, so SET can run on already created elements
o Added error when using != instead of <>
o Fixed #787: Issue when comparing array properties with literal collections
o Fixes #751: Better error message for some type errors
o Fixed #818: problem where properties could only have scalar values (they can be arrays as well)
o Fixed #751: Better error message for some type errors
o Fixed #818: Problem where properties could only have scalar values (they can be arrays as well)
o Fixed #834: Gives relevant syntax exception when properties are entered in MATCH

1.8.M07 (2012-08-08)
--------------------
Expand Down
Expand Up @@ -38,20 +38,34 @@ trait MatchClause extends Base with ParserPattern {
}

def matchTranslator(abstractPattern: AbstractPattern): Maybe[Any] = abstractPattern match {
case ParsedNamedPath(name, patterns) =>
val namedPathPatterns = patterns.map(matchTranslator)
val result = namedPathPatterns.reduce(_ ++ _)
result.seqMap(p => Seq(NamedPath(name, p.map(_.asInstanceOf[Pattern]):_*)))
case ParsedNamedPath(name, patterns) => parsedPath(name, patterns)

case ParsedRelation(name, props, ParsedEntity(left, startProps, True()), ParsedEntity(right, endProps, True()), relType, dir, optional, predicate) =>
successIfEntities(left, right)((l, r) => RelatedTo(left = l, right = r, relName = name, relTypes = relType, direction = dir, optional = optional, predicate = True()))
if (props.isEmpty && startProps.isEmpty && endProps.isEmpty)
successIfEntities(left, right)((l, r) => RelatedTo(left = l, right = r, relName = name, relTypes = relType, direction = dir, optional = optional, predicate = True()))
else
No(Seq("Properties on pattern elements are not allowed in MATCH"))

case ParsedVarLengthRelation(name, props, ParsedEntity(left, startProps, True()), ParsedEntity(right, endProps, True()), relType, dir, optional, predicate, min, max, relIterator) =>
if (props.isEmpty && startProps.isEmpty && endProps.isEmpty)
successIfEntities(left, right)((l, r) => RelatedTo(left = l, right = r, relName = name, relTypes = relType, direction = dir, optional = optional, predicate = True()))
else
No(Seq("Properties on pattern elements are not allowed in MATCH"))
successIfEntities(left, right)((l, r) => VarLengthRelatedTo(pathName = name, start = l, end = r, minHops = min, maxHops = max, relTypes = relType, direction = dir, relIterator = relIterator, optional = optional, predicate = predicate))

case ParsedShortestPath(name, props, ParsedEntity(left, startProps, True()), ParsedEntity(right, endProps, True()), relType, dir, optional, predicate, max, single, relIterator) =>
if (props.isEmpty && startProps.isEmpty && endProps.isEmpty)
successIfEntities(left, right)((l, r) => RelatedTo(left = l, right = r, relName = name, relTypes = relType, direction = dir, optional = optional, predicate = True()))
else
No(Seq("Properties on pattern elements are not allowed in MATCH"))
successIfEntities(left, right)((l, r) => ShortestPath(pathName = name, start = l, end = r, relTypes = relType, dir = dir, maxDepth = max, optional = optional, single = single, relIterator = relIterator, predicate = predicate))

case x => No(Seq("failed to parse MATCH pattern"))
}

def parsedPath(name: String, patterns: Seq[AbstractPattern]): Maybe[NamedPath] = {
val namedPathPatterns = patterns.map(matchTranslator)
val result = namedPathPatterns.reduce(_ ++ _)
result.seqMap(p => Seq(NamedPath(name, p.map(_.asInstanceOf[Pattern]): _*)))
}
}
Expand Up @@ -52,9 +52,9 @@ trait ParserPattern extends Base {
val concretePattern = abstractPattern.map(p => translator(p))

concretePattern.find(!_.success) match {
case Some(No(msg)) => Failure(msg.mkString("\n"), rest)
case None => Success(concretePattern.flatMap(_.values), rest)
case _ => throw new ThisShouldNotHappenError("Andres", "This is here to stop compiler warnings.")
case Some(No(msg)) => Failure(msg.mkString("\n"), rest.rest)
case None => Success(concretePattern.flatMap(_.values), rest)
case _ => throw new ThisShouldNotHappenError("Andres", "This is here to stop compiler warnings.")
}

case Failure(msg, rest) => Failure(msg, rest)
Expand Down
Expand Up @@ -21,6 +21,7 @@ package org.neo4j.cypher.internal.parser.v1_8

import org.neo4j.cypher.internal.commands._
import org.neo4j.graphdb.Direction
import org.neo4j.helpers.ThisShouldNotHappenError


trait StartClause extends Base with Expressions with CreateUnique {
Expand All @@ -42,9 +43,23 @@ trait StartClause extends Base with Expressions with CreateUnique {

case class NamedPathWStartItems(path:NamedPath, items:Seq[StartItem])

private def removeProperties(in:AbstractPattern):AbstractPattern = in match {
case ParsedNamedPath(name, patterns) => throw new ThisShouldNotHappenError("Andres", "We don't support paths in paths, and so should never see this")
case rel: ParsedRelation => rel.copy(
props = Map(),
start = rel.start.copy(props = Map()),
end = rel.end.copy(props = Map())
)
case n:ParsedEntity => n.copy(props = Map())
}

private def translate(abstractPattern: AbstractPattern): Maybe[Any] = abstractPattern match {
case ParsedNamedPath(name, patterns) =>
val namedPathPatterns: Maybe[Any] = patterns.map(matchTranslator).reduce(_ ++ _)
val namedPathPatterns: Maybe[Any] = patterns.
map(removeProperties).
map(matchTranslator).
reduce(_ ++ _)

val startItems = patterns.map(p => translate(p.makeOutgoing)).reduce(_ ++ _)

startItems match {
Expand Down
18 changes: 15 additions & 3 deletions cypher/src/test/scala/org/neo4j/cypher/ErrorMessagesTest.scala
Expand Up @@ -102,19 +102,19 @@ class ErrorMessagesTest extends ExecutionEngineHelper with Assertions with Strin
@Test def badMatch5() {
expectSyntaxError(
"start p=node(2) match p[:likes]->dude return dude.name",
"failed to parse MATCH pattern", 23)
"failed to parse MATCH pattern", 24)
}

@Test def badMatch7() {
expectSyntaxError(
"start p=node(2) match p->dude return dude.name",
"expected [ or -", 24)
"failed to parse MATCH pattern", 24)
}

@Test def badMatch8() {
expectSyntaxError(
"start p=node(2) match p->dude return dude.name",
"expected [ or -", 24)
"failed to parse MATCH pattern", 24)
}

@Ignore @Test def missingComaBetweenColumns() {
Expand Down Expand Up @@ -226,6 +226,18 @@ class ErrorMessagesTest extends ExecutionEngineHelper with Assertions with Strin
"Expected `r` to be a Map but it was a Collection")
}

@Test def error_when_using_properties_on_relationships_in_match() {
expectError(
"START p=node(0) MATCH p-[r {a:'foo'}]->() RETURN r",
"Properties on pattern elements are not allowed in MATCH")
}

@Test def error_when_using_properties_on_relationships_in_match2() {
expectError(
"START p=node(0) MATCH p-[r]->({a:'foo'}) RETURN r",
"Properties on pattern elements are not allowed in MATCH")
}

private def expectError[T <: CypherException](query: String, expectedError: String)(implicit manifest: Manifest[T]): T = {
val error = intercept[T](engine.execute(query).toList)

Expand Down

0 comments on commit ae0c296

Please sign in to comment.