Skip to content

Commit

Permalink
fix: Make sure local scope is cleared after leaving subquery expressi…
Browse files Browse the repository at this point in the history
…ons. (#837)

Fixes #832.
  • Loading branch information
michael-simons authored Oct 30, 2023
1 parent 8316cce commit 9e455e9
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,12 @@ public void doLeave(Visitable visitable) {
}
}

/**
* Anything that might import variables from the outside, without using an explicit {@code WITH} clause.
*
* @param visitable the element to be checked whether it implicitly imports named elements.
* @return {@literal true} if named elements are imported
*/
private static boolean hasImplicitScope(Visitable visitable) {
return visitable instanceof SubqueryExpression || visitable instanceof Statement.UnionQuery;
}
Expand Down Expand Up @@ -317,6 +323,7 @@ private Predicate<String> identifiedBy(Named needle) {
private static boolean hasLocalScope(Visitable visitable) {
return visitable instanceof PatternComprehension ||
visitable instanceof Subquery ||
visitable instanceof SubqueryExpression ||
visitable instanceof Foreach;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -691,16 +691,17 @@ void returningRawShouldWork() {
.build();

String cypher = Renderer.getRenderer(Configuration.prettyPrinting()).render(statement);
assertThat(cypher).isEqualTo("MATCH (node:Node)\n"
+ "CALL {\n"
+ " WITH node\n"
+ " WITH node AS this\n"
+ " MATCH (this)-[:LINK]-(o:Other) RETURN o AS result\n"
+ "}\n"
+ "RETURN result {\n"
+ " .foo,\n"
+ " .bar\n"
+ "}");
assertThat(cypher).isEqualTo("""
MATCH (node:Node)
CALL {
WITH node
WITH node AS this
MATCH (this)-[:LINK]-(o:Other) RETURN o AS result
}
RETURN result {
.foo,
.bar
}""");
}

@Test // GH-190
Expand Down Expand Up @@ -910,13 +911,13 @@ void symbolicNamesInNotConditionsMustNotBeResolvedWhenConditionIsARelationshipPa
"MATCH (node:Division) WITH DISTINCT node WHERE NOT (node)-[:IN]->(:Department)-[:INSIDE {rel_property: true}]->(:Department)-[:EMPLOYS]->(:Employee) RETURN *");

assertThat(Renderer.getRenderer(Configuration.prettyPrinting()).render(q))
.isEqualTo(
"MATCH (node:Division)\n" +
"WITH DISTINCT node\n" +
"WHERE NOT (node)-[:IN]->(:Department)-[:INSIDE {\n" +
" rel_property: true\n" +
"}]->(:Department)-[:EMPLOYS]->(:Employee)\n" +
"RETURN *"
.isEqualTo("""
MATCH (node:Division)
WITH DISTINCT node
WHERE NOT (node)-[:IN]->(:Department)-[:INSIDE {
rel_property: true
}]->(:Department)-[:EMPLOYS]->(:Employee)
RETURN *"""
);
}

Expand All @@ -940,13 +941,13 @@ void symbolicNamesInNotConditionsMustNotBeResolvedWhenConditionIsARelationshipPa
"MATCH (node:Division) WITH DISTINCT node WHERE NOT (node)-[:IN]->(:Department)-[:INSIDE {rel_property: true}]->(:Department)-[:EMPLOYS]->(:Employee) RETURN *");

assertThat(Renderer.getRenderer(Configuration.prettyPrinting()).render(q))
.isEqualTo(
"MATCH (node:Division)\n" +
"WITH DISTINCT node\n" +
"WHERE NOT (node)-[:IN]->(:Department)-[:INSIDE {\n" +
" rel_property: true\n" +
"}]->(:Department)-[:EMPLOYS]->(:Employee)\n" +
"RETURN *"
.isEqualTo("""
MATCH (node:Division)
WITH DISTINCT node
WHERE NOT (node)-[:IN]->(:Department)-[:INSIDE {
rel_property: true
}]->(:Department)-[:EMPLOYS]->(:Employee)
RETURN *"""
);
}

Expand Down Expand Up @@ -1012,17 +1013,17 @@ void communitySite20220304() {

Renderer renderer = Renderer.getRenderer(Configuration.prettyPrinting());
String cypher = renderer.render(completeStatement);
String expected = ""
+ "MATCH p = (:lookingType)<-[:specifiedRelation]-()\n"
+ "WITH nodes(p) AS nodes, relationships(p) AS relations\n"
+ "CALL {\n"
+ " WITH nodes\n"
+ " UNWIND nodes AS n\n"
+ " WITH n\n"
+ " MATCH second_p = (n)-[second_relations:otherRelation]->(second_nodes)\n"
+ " RETURN second_nodes, second_relations\n"
+ "}\n"
+ "RETURN nodes, relations, collect(second_nodes), collect(second_relations)";
String expected = """
MATCH p = (:lookingType)<-[:specifiedRelation]-()
WITH nodes(p) AS nodes, relationships(p) AS relations
CALL {
WITH nodes
UNWIND nodes AS n
WITH n
MATCH second_p = (n)-[second_relations:otherRelation]->(second_nodes)
RETURN second_nodes, second_relations
}
RETURN nodes, relations, collect(second_nodes), collect(second_relations)""";
assertThat(cypher).isEqualTo(expected);
}

Expand All @@ -1047,15 +1048,15 @@ void subqueryWithRename() {

Renderer renderer = Renderer.getRenderer(Configuration.prettyPrinting());
String cypher = renderer.render(completeStatement);
String expected = ""
+ "MATCH p = (:Target)<-[:REL]-()\n"
+ "WITH nodes(p) AS nodes, relationships(p) AS relations\n"
+ "CALL {\n"
+ " WITH nodes\n"
+ " WITH nodes AS x\n"
+ " RETURN x\n"
+ "}\n"
+ "RETURN *";
String expected = """
MATCH p = (:Target)<-[:REL]-()
WITH nodes(p) AS nodes, relationships(p) AS relations
CALL {
WITH nodes
WITH nodes AS x
RETURN x
}
RETURN *""";
assertThat(cypher).isEqualTo(expected);
}

Expand All @@ -1080,13 +1081,13 @@ void subqueryWithoutImport() {

Renderer renderer = Renderer.getRenderer(Configuration.prettyPrinting());
String cypher = renderer.render(completeStatement);
String expected = ""
+ "MATCH p = (:Target)<-[:REL]-()\n"
+ "WITH nodes(p) AS nodes, relationships(p) AS relations\n"
+ "CALL {\n"
+ " RETURN 1\n"
+ "}\n"
+ "RETURN true";
String expected = """
MATCH p = (:Target)<-[:REL]-()
WITH nodes(p) AS nodes, relationships(p) AS relations
CALL {
RETURN 1
}
RETURN true""";
assertThat(cypher).isEqualTo(expected);
}

Expand Down Expand Up @@ -1245,17 +1246,17 @@ void shouldRenderSetOpOnNodeWithMap() {
.build();

assertThat(Renderer.getRenderer(Configuration.prettyPrinting()).render(statement))
.isEqualTo(""
+ "MERGE (existingNode:CordraObject {\n"
+ " _id: 'test/55de0539eb1e14f26a04'\n"
+ "})\n"
+ "SET existingNode = {\n"
+ " _id: 'test/55de0539eb1e14f26a04',\n"
+ " _type: 'Movie',\n"
+ " title: 'Top Gun',\n"
+ " released: 1986\n"
+ "}\n"
+ "RETURN existingNode");
.isEqualTo("""
MERGE (existingNode:CordraObject {
_id: 'test/55de0539eb1e14f26a04'
})
SET existingNode = {
_id: 'test/55de0539eb1e14f26a04',
_type: 'Movie',
title: 'Top Gun',
released: 1986
}
RETURN existingNode""");
}

@Test // GH-388
Expand All @@ -1276,17 +1277,18 @@ void shouldProvideSetOperations() {
.build();

assertThat(Renderer.getRenderer(Configuration.prettyPrinting()).render(statement))
.isEqualTo(""
+ "MERGE (existingNode:CordraObject {\n"
+ " _id: 'test/55de0539eb1e14f26a04'\n"
+ "})\n"
+ "SET existingNode = {\n"
+ " _id: 'test/55de0539eb1e14f26a04',\n"
+ " _type: 'Movie',\n"
+ " title: 'Top Gun',\n"
+ " released: 1986\n"
+ "}\n"
+ "RETURN existingNode");
.isEqualTo("""
MERGE (existingNode:CordraObject {
_id: 'test/55de0539eb1e14f26a04'
})
SET existingNode = {
_id: 'test/55de0539eb1e14f26a04',
_type: 'Movie',
title: 'Top Gun',
released: 1986
}
RETURN existingNode"""
);
}

@Test // GH-388
Expand All @@ -1302,12 +1304,13 @@ void shouldProvideSetOperationsForParameter() {
.build();

assertThat(Renderer.getRenderer(Configuration.prettyPrinting()).render(statement))
.isEqualTo(""
+ "MERGE (existingNode:CordraObject {\n"
+ " _id: 'test/55de0539eb1e14f26a04'\n"
+ "})\n"
+ "SET existingNode = $aNewMap\n"
+ "RETURN existingNode");
.isEqualTo("""
MERGE (existingNode:CordraObject {
_id: 'test/55de0539eb1e14f26a04'
})
SET existingNode = $aNewMap
RETURN existingNode"""
);
}

@Test // GH-419
Expand All @@ -1333,18 +1336,20 @@ void aliasedElementsShouldBeCarriedForwardWithWithToo() {
String cypher = Renderer.getRenderer(
Configuration.newConfig().withPrettyPrint(true).alwaysEscapeNames(true).build()).render(statement);
assertThat(cypher)
.isEqualTo("MATCH (oi:`ObjectInstance`)\n"
+ "WITH {\n"
+ " oi: oi\n"
+ "} AS collection\n"
+ "UNWIND $attributes AS attributeTypeAndValue\n"
+ "WITH attributeTypeAndValue, collection\n"
+ "MATCH (att:`AttributeType` {\n"
+ " name: attributeTypeAndValue.name\n"
+ "})<-[:`OF_TYPE`]-(at:`Attribute`)\n"
+ "WITH at, collection.oi AS oi\n"
+ "MATCH (oi)-[:`IS_IDENTIFIED_BY`]->(at)\n"
+ "RETURN at");
.isEqualTo("""
MATCH (oi:`ObjectInstance`)
WITH {
oi: oi
} AS collection
UNWIND $attributes AS attributeTypeAndValue
WITH attributeTypeAndValue, collection
MATCH (att:`AttributeType` {
name: attributeTypeAndValue.name
})<-[:`OF_TYPE`]-(at:`Attribute`)
WITH at, collection.oi AS oi
MATCH (oi)-[:`IS_IDENTIFIED_BY`]->(at)
RETURN at"""
);
}

@Test
Expand Down Expand Up @@ -1757,4 +1762,30 @@ void identifiablesCreatedInSubqueriesMustBeRecognizedAsSeenToo() {
.isEqualTo(
"CREATE (n1:`Foo`) WITH n1 CALL {WITH n1 MERGE (n2:`Bar` {foo: 'x'}) CREATE (n1)-[:`NESTED`]->(n2) RETURN count(n2) AS foo_2} RETURN true");
}

@Test // GH-832
void sequentialExistingSubqueriesShouldNotHaveScopingIssues() {

var n1 = Cypher.node("Foo").named("n1");
var n2 = Cypher.node("Bar").named("n2");
var resultStatement = Cypher.match(n1)
.where(
Cypher.match(n1.relationshipTo(n2)).where(n2.property("bar").isFalse()).asCondition()
.or(Cypher.match(n1.relationshipTo(n2)).where(n2.property("foo").isTrue()).asCondition())
)
.returning(Cypher.literalTrue())
.build();

assertThat(resultStatement.getCypher())
.isEqualTo("MATCH (n1:`Foo`) "
+ "WHERE (EXISTS {"
+ " MATCH (n1)-->(n2:`Bar`)"
+ " WHERE n2.bar = false "
+ "} "
+ "OR EXISTS {"
+ " MATCH (n1)-->(n2:`Bar`)"
+ " WHERE n2.foo = true "
+ "}) "
+ "RETURN true");
}
}

0 comments on commit 9e455e9

Please sign in to comment.