Fixed shortestPath planning when preceded by unwind #8699
Conversation
| setupModel() | ||
| val result = executeUsingCostPlannerOnly( | ||
| """ | ||
| |//same idea but iterating over two collections of nodes |
There was a problem hiding this comment.
Same idea as what? The below test? I find this in-query comment a bit redundant wrt the test name.
| |match pathx = allShortestPaths((charlesThompson)-[*1..5]-(joeySantiago)) | ||
| |where none (n IN nodes(pathx) where id(n) = id(kimDeal)) | ||
| |with nodes(pathx) as nodes | ||
| |return nodes |
There was a problem hiding this comment.
Since this is a pretty complex query, I think it'd aid readability to format it more in line with Cypher code style.
| """.stripMargin) | ||
| val results = result.columnAs("nodes").toList | ||
| println(results) | ||
| println(result.executionPlanDescription()) |
| val results = result.columnAs("nodes").toList | ||
| println(results) | ||
| println(result.executionPlanDescription()) | ||
| val ids = results(0).asInstanceOf[Seq[_]].map(n => n.asInstanceOf[Node].getId) |
There was a problem hiding this comment.
If we are asserting on the ids only, isn't it easier to adapt the query to return only ids, instead of this mouthful?
E.g:
...
RETURN [n IN nodes | id(n)] AS ids
and then
result.toList should equal List(Map("ids" -> List(0, 4, 3, 2)))
That also makes sure that there isn't a second row returned (which is overlooked by only asserting on results(0)).
| } | ||
|
|
||
| test("shortestPath with complex LHS should be planned with exhaustive fallback and include predicate") { | ||
| System.setProperty("pickBestPlan.VERBOSE","true") |
| |match pathx = shortestPath((charlesThompson)-[*1..5]-(joeySantiago)) | ||
| |where none (n IN nodes(pathx) where id(n) = id(kimDeal)) | ||
| |unwind nodes(pathx) as nodes | ||
| |return nodes |
There was a problem hiding this comment.
Can I please request Cypher styling of this one too? :)
| """.stripMargin) | ||
| val results = result.columnAs("nodes").toList | ||
| println(results) | ||
| println(result.executionPlanDescription()) |
| val results = result.columnAs("nodes").toList | ||
| println(results) | ||
| println(result.executionPlanDescription()) | ||
| assertThat(results.length, equalTo(4)) |
There was a problem hiding this comment.
That's a pretty vague result assertion; why not assert on the exact result contents?
| |merge (p4:Pixie {fname:'David'}) | ||
| |merge (p5:Pixie {fname:'Paz'}) | ||
| |merge (p1)-[:KNOWS]->(p2)-[:KNOWS]->(p3)-[:KNOWS]->(p4)-[:KNOWS]->(p5)-[:KNOWS]->(p1) | ||
| |return p1,p2,p3,p4,p5 |
There was a problem hiding this comment.
I guess it doesn't matter a lot, but these could be CREATE instead, and RETURN (and the comment) may be omitted.
| |UNWIND joeyS AS joeySantiago | ||
| |MATCH pathx = allShortestPaths((charlesThompson)-[*1..5]-(joeySantiago)) | ||
| |WHERE none (n IN nodes(pathx) WHERE id(n) = id(kimDeal)) | ||
| |RETURN extract(node in nodes(pathx) | id(node)) as ids |
There was a problem hiding this comment.
Not saying you need to change this, but the fancy way of doing extract is through list comprehensions: [node IN nodes(pathx) | id(node)] AS ids
There was a problem hiding this comment.
I thought that was only in 3.1 and later.
There was a problem hiding this comment.
You are correct! I forgot that this was 3.0
No description provided.