Skip to content

Commit

Permalink
Return correct results when paging and filtering on relationship prop…
Browse files Browse the repository at this point in the history 
…erty

Replace "with n" for "with distinct n" when filtering on a relationship property.
Fixes neo4j#384
  • Loading branch information
@frant-hartm
Eric Spiegelberg authored and frant-hartm committed Sep 5, 2017
1 parent 6829f9c commit 17c895b
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 14 deletions.
5 changes: 3 additions & 2 deletions core/src/main/java/org/neo4j/ogm/cypher/query/PagingAndSortingQuery.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
*/
package org.neo4j.ogm.cypher.query;

import org.neo4j.ogm.session.request.NodeQueryBuilder;

import java.util.Map;

import org.apache.commons.lang3.StringUtils;

/**
* Extends {@link CypherQuery} with additional functionality for Paging and Sorting.
* Only used by queries that return actual nodes and/or relationships from the graph. Other queries
Expand Down Expand Up @@ -67,6 +67,7 @@ public String getStatement() {
StringBuilder sb = new StringBuilder();

String returnClause = this.returnClause;

sb.append(matchClause);
if (!sorting.isEmpty()) {
sb.append(sorting.replace("$", "n"));
Expand Down
8 changes: 8 additions & 0 deletions core/src/main/java/org/neo4j/ogm/session/request/NodeQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public class NodeQueryBuilder {
private Map<String, Object> parameters;
private int matchClauseId;
private boolean built = false;
private boolean hasRelationshipMatch = false;

public NodeQueryBuilder(String principalLabel, Iterable<Filter> filters) {
this.principalClause = new PrincipalNodeMatchClause(principalLabel);
Expand All @@ -56,6 +57,7 @@ public FilteredQuery build() {
}
if (filter.isNested()) {
appendNestedFilter(filter);
hasRelationshipMatch = true;
} else {
//If the filter is not nested, it belongs to the node we're returning
principalClause().append(filter);
Expand Down Expand Up @@ -132,6 +134,12 @@ private StringBuilder toCypher() {
stringBuilder.append(matchClause.toCypher());
}

if (hasRelationshipMatch) {
stringBuilder.append("WITH DISTINCT n");
} else {
stringBuilder.append("WITH n");
}

return stringBuilder;
}
}
2 changes: 1 addition & 1 deletion core/src/main/java/org/neo4j/ogm/session/request/strategy/impl/NodeQueryStatements.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public PagingAndSortingQuery findByType(String label, int depth) {
@Override
public PagingAndSortingQuery findByType(String label, Filters parameters, int depth) {
FilteredQuery filteredQuery = FilteredQueryBuilder.buildNodeQuery(label, parameters);
String matchClause = filteredQuery.statement()+ "WITH n";
String matchClause = filteredQuery.statement();
String returnClause = loadClauseBuilder.build(label, depth);
return new PagingAndSortingQuery(matchClause, returnClause, filteredQuery.parameters(), depth != 0, true);
}
Expand Down
22 changes: 11 additions & 11 deletions test/src/test/java/org/neo4j/ogm/session/request/strategy/impl/NodeQueryStatementsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ public void testFindByNestedPropertyOutgoing() {
planetParam.setNestedEntityTypeLabel("Planet");
planetParam.setRelationshipType("COLLIDES");
planetParam.setRelationshipDirection("OUTGOING");
assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)-[:`COLLIDES`]->(m0) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)-[:`COLLIDES`]->(m0) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
}

/**
Expand All @@ -391,7 +391,7 @@ public void testFindByNestedPropertySameEntityType() {
filter.setNestedEntityTypeLabel("Asteroid");
filter.setRelationshipType("COLLIDES");
filter.setRelationshipDirection("OUTGOING");
assertThat(queryStatements.findByType("Asteroid", new Filters().add(filter), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Asteroid`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)-[:`COLLIDES`]->(m0) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
assertThat(queryStatements.findByType("Asteroid", new Filters().add(filter), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Asteroid`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)-[:`COLLIDES`]->(m0) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
}

/**
Expand All @@ -404,7 +404,7 @@ public void testFindByNestedPropertyIncoming() {
planetParam.setNestedEntityTypeLabel("Planet");
planetParam.setRelationshipType("COLLIDES");
planetParam.setRelationshipDirection("INCOMING");
assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)<-[:`COLLIDES`]-(m0) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)<-[:`COLLIDES`]-(m0) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
}

/**
Expand All @@ -417,7 +417,7 @@ public void testFindByNestedPropertyUndirected() {
planetParam.setNestedEntityTypeLabel("Planet");
planetParam.setRelationshipType("COLLIDES");
planetParam.setRelationshipDirection("UNDIRECTED");
assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)-[:`COLLIDES`]-(m0) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)-[:`COLLIDES`]-(m0) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
}

/**
Expand All @@ -433,7 +433,7 @@ public void testFindByMultipleNestedProperties() {
planetParam.setNestedEntityTypeLabel("Planet");
planetParam.setRelationshipType("COLLIDES");
planetParam.setRelationshipDirection("OUTGOING");
assertThat(queryStatements.findByType("Asteroid", new Filters().add(diameterParam).add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) WHERE n.`diameter` > { `diameter_0` } MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_1` } MATCH (n)-[:`COLLIDES`]->(m0) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
assertThat(queryStatements.findByType("Asteroid", new Filters().add(diameterParam).add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) WHERE n.`diameter` > { `diameter_0` } MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_1` } MATCH (n)-[:`COLLIDES`]->(m0) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
}

/**
Expand All @@ -449,7 +449,7 @@ public void testFindByMultipleNestedPropertiesInfiniteDepth() {
planetParam.setNestedEntityTypeLabel("Planet");
planetParam.setRelationshipType("COLLIDES");
planetParam.setRelationshipDirection("OUTGOING");
assertThat(queryStatements.findByType("Asteroid", new Filters().add(diameterParam).add(planetParam), -1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) WHERE n.`diameter` > { `diameter_0` } MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_1` } MATCH (n)-[:`COLLIDES`]->(m0) WITH n MATCH p=(n)-[*0..]-(m) RETURN p, ID(n)");
assertThat(queryStatements.findByType("Asteroid", new Filters().add(diameterParam).add(planetParam), -1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) WHERE n.`diameter` > { `diameter_0` } MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_1` } MATCH (n)-[:`COLLIDES`]->(m0) WITH DISTINCT n MATCH p=(n)-[*0..]-(m) RETURN p, ID(n)");
}

/**
Expand Down Expand Up @@ -502,7 +502,7 @@ public void testFindByNestedREProperty() {

assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (n)-[r0:`COLLIDES`]->(m0) " +
"WHERE r0.`totalDestructionProbability` = { `collision_totalDestructionProbability_0` } " +
"WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
"WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
}

/**
Expand All @@ -528,7 +528,7 @@ public void testFindByMultipleNestedREProperty() {
satelliteParam.setNestedRelationshipEntity(true);

assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam).add(satelliteParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (n)-[r0:`COLLIDES`]->(m0) WHERE r0.`totalDestructionProbability` = { `collision_totalDestructionProbability_0` } " +
"MATCH (n)<-[r1:`MONITORED_BY`]-(m1) WHERE r1.`signalStrength` >= { `monitoringSatellites_signalStrength_1` } WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
"MATCH (n)<-[r1:`MONITORED_BY`]-(m1) WHERE r1.`signalStrength` >= { `monitoringSatellites_signalStrength_1` } WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
}


Expand All @@ -554,7 +554,7 @@ public void testFindByNestedBaseAndREProperty() {

assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam, moonParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (n)-[r0:`COLLIDES`]->(m0) " +
"WHERE r0.`totalDestructionProbability` = { `collision_totalDestructionProbability_0` } " +
"MATCH (m1:`Moon`) WHERE m1.`name` = { `moon_name_1` } MATCH (n)<-[:`ORBITS`]-(m1) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
"MATCH (m1:`Moon`) WHERE m1.`name` = { `moon_name_1` } MATCH (n)<-[:`ORBITS`]-(m1) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
}


Expand All @@ -578,7 +578,7 @@ public void testFindByDifferentNestedPropertiesAnded() {
assertThat(queryStatements.findByType("Asteroid",
new Filters().add(planetParam).add(moonParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } " +
"MATCH (m1:`Moon`) WHERE m1.`name` = { `moon_name_1` } MATCH (n)-[:`COLLIDES`]->(m0) " +
"MATCH (n)<-[:`ORBITS`]-(m1) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
"MATCH (n)<-[:`ORBITS`]-(m1) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
}

/**
Expand Down Expand Up @@ -620,7 +620,7 @@ public void testFindByMultipleNestedPropertiesAnded() {
moonParam.setRelationshipType("COLLIDES");
moonParam.setRelationshipDirection("OUTGOING");
moonParam.setBooleanOperator(BooleanOperator.AND);
assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam).add(moonParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } AND m0.`size` = { `collidesWith_size_1` } MATCH (n)-[:`COLLIDES`]->(m0) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam).add(moonParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } AND m0.`size` = { `collidesWith_size_1` } MATCH (n)-[:`COLLIDES`]->(m0) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)");
}

/**
Expand Down

0 comments on commit 17c895b

Please sign in to comment.