Skip to content

Commit

Permalink
fix: Proper conjunct (nested) filters on relationship queries.
Browse files Browse the repository at this point in the history
Fixes #1218.

Signed-off-by: Michael Simons <michael.simons@neo4j.com>
  • Loading branch information
michael-simons committed Jan 21, 2025
1 parent ba08f84 commit c675e8b
Show file tree
Hide file tree
Showing 5 changed files with 217 additions and 62 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2002-2022 "Neo4j,"
* Copyright (c) 2002-2025 "Neo4j,"
* Neo4j Sweden AB [http://neo4j.com]
*
* This file is part of Neo4j.
Expand Down Expand Up @@ -185,7 +185,7 @@ private static StringBuilder constructRelationshipQuery(String type, Iterable<Fi
addFilterToList(target, filter);
}
} else {
if (relationshipFilters.size() == 0) {
if (relationshipFilters.isEmpty()) {
filter.setBooleanOperator(BooleanOperator.NONE);
} else {
if (filter.getBooleanOperator().equals(BooleanOperator.NONE)) {
Expand Down Expand Up @@ -216,9 +216,8 @@ private static StringBuilder constructRelationshipQuery(String type, Iterable<Fi
}
properties.putAll(filteredQuery.parameters());
}
createNodeMatchSubquery(properties, outgoingFilters, query, "n");
createNodeMatchSubquery(properties, incomingFilters, query, "m");
createRelationSubquery(type, properties, relationshipFilters, query, initialDirection);
createRelationSubquery(type, properties, outgoingFilters, incomingFilters, relationshipFilters, query,
initialDirection);
return query;
}

Expand Down Expand Up @@ -247,29 +246,37 @@ private static void addFilterToList(FiltersAtStartNode target, Filter filter) {
}

private static void createRelationSubquery(String type, Map<String, Object> properties,
List<Filter> relationshipFilters, StringBuilder query, String initialDirection) {
FiltersAtStartNode filtersAtStartNode, FiltersAtStartNode filtersAtEndNode, List<Filter> relationshipFilters,
StringBuilder query, String initialDirection) {

String startLabel = filtersAtStartNode.startNodeLabel == null ? "" : String.format(":`%s`", filtersAtStartNode.startNodeLabel);
String endLabel = filtersAtEndNode.startNodeLabel == null ? "" : String.format(":`%s`", filtersAtEndNode.startNodeLabel);
if (initialDirection == null || initialDirection.equals(Relationship.OUTGOING)) {
query.append(String.format("MATCH (n)-[r0:`%s`]->(m) ", type));
query.append(String.format("MATCH (n%s)-[r0:`%s`]->(m%s) ", startLabel, type, endLabel));
} else {
query.append(String.format("MATCH (n)<-[r0:`%s`]-(m) ", type));
query.append(String.format("MATCH (n%s)<-[r0:`%s`]-(m%s) ", startLabel, type, endLabel));
}
if (relationshipFilters.size() > 0) {

boolean hasFiltersAtStart = !filtersAtStartNode.content.isEmpty();
boolean hasFiltersAtEnd = !filtersAtEndNode.content.isEmpty();
if (!relationshipFilters.isEmpty() || hasFiltersAtStart || hasFiltersAtEnd) {
query.append("WHERE ");
if (!filtersAtStartNode.content.isEmpty() || !filtersAtEndNode.content.isEmpty()) {
query.append("( ");
appendFilters(filtersAtStartNode.content, "n", query, properties);
if (hasFiltersAtStart && hasFiltersAtEnd && filtersAtEndNode.content.get(0).getBooleanOperator() == BooleanOperator.NONE) {
query.append("AND ");
}
appendFilters(filtersAtEndNode.content, "m", query, properties);
query.append(")");
if (!relationshipFilters.isEmpty()) {
query.append(" AND ");
}
}
appendFilters(relationshipFilters, "r0", query, properties);
}
}

private static void createNodeMatchSubquery(Map<String, Object> properties, FiltersAtStartNode filtersAtStartNode,
StringBuilder query, String nodeIdentifier) {
String nodeLabel = filtersAtStartNode.startNodeLabel;
List<Filter> nodeFilters = filtersAtStartNode.content;
if (!(nodeLabel == null || nodeFilters.isEmpty())) {
query.append(String.format("MATCH (%s:`%s`) WHERE ", nodeIdentifier, nodeLabel));
appendFilters(nodeFilters, nodeIdentifier, query, properties);
}
}

private static void appendFilters(List<Filter> filters, String nodeIdentifier, StringBuilder query,
Map<String, Object> properties) {
for (Filter filter : filters) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@
import static org.assertj.core.api.Assertions.*;

import java.io.IOException;
import java.util.Collections;

import org.assertj.core.api.Assertions;
import org.junit.After;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.neo4j.ogm.domain.music.Node;
import org.neo4j.ogm.domain.music.RelBetweenNodes;
import org.neo4j.ogm.domain.music.Studio;
import org.neo4j.ogm.session.Session;
import org.neo4j.ogm.session.SessionFactory;
Expand Down Expand Up @@ -100,4 +104,23 @@ public void ignoreCaseShouldBeApplicableToEndingWith() {
.extracting(Studio::getName)
.containsExactly(emi);
}

@Test // GH-1218
public void nestedFiltersOnRelationshipMustBeProperlyConjucted() {

Filters filters = new Filters(Collections.emptyList());
Filter filter1 = new Filter("id", ComparisonOperator.EQUALS, "my_id");
filter1.setNestedPropertyName("start");
filter1.setNestedPropertyType(Node.class);
Filter filter2 = new Filter("id", ComparisonOperator.EQUALS, "my_id");
filter2.setNestedPropertyName("end");
filter2.setNestedPropertyType(Node.class);
filters.add(filter1);
filters.or(filter2);
try {
session.loadAll(RelBetweenNodes.class, filters);
} catch (Exception e) {
Assertions.fail("Filtered query did not work.");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright (c) 2002-2025 "Neo4j,"
* Neo4j Sweden AB [http://neo4j.com]
*
* This file is part of Neo4j.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.neo4j.ogm.domain.music;

import java.util.HashSet;
import java.util.Set;

import org.neo4j.ogm.annotation.Id;
import org.neo4j.ogm.annotation.NodeEntity;
import org.neo4j.ogm.annotation.Relationship;

/**
* GH-1218
*/
@NodeEntity
public class Node {

@Id
String id;

@Relationship(type = "relationship")
public Set<RelBetweenNodes> relationships = new HashSet<>();

public String getId() {
return id;
}

public void setId(String id) {
this.id = id;
}

public Set<RelBetweenNodes> getRelationships() {
return relationships;
}

public void setRelationships(Set<RelBetweenNodes> relationships) {
this.relationships = relationships;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright (c) 2002-2025 "Neo4j,"
* Neo4j Sweden AB [http://neo4j.com]
*
* This file is part of Neo4j.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.neo4j.ogm.domain.music;

import org.neo4j.ogm.annotation.EndNode;
import org.neo4j.ogm.annotation.GeneratedValue;
import org.neo4j.ogm.annotation.Id;
import org.neo4j.ogm.annotation.NodeEntity;
import org.neo4j.ogm.annotation.Relationship;
import org.neo4j.ogm.annotation.RelationshipEntity;
import org.neo4j.ogm.annotation.StartNode;
import org.neo4j.ogm.domain.drink.Manufacturer;
import org.neo4j.ogm.id.UuidStrategy;

/**
* GH-1218
*/
@RelationshipEntity(type = "relationship")
public class RelBetweenNodes {

@Id @GeneratedValue
private Long id;

@StartNode
private Node start;

@EndNode
private Node end;

public Node getEnd() {
return end;
}

public void setEnd(Node end) {
this.end = end;
}

public Long getId() {
return id;
}

public void setId(Long id) {
this.id = id;
}

public Node getStart() {
return start;
}

public void setStart(Node start) {
this.start = start;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@
import static org.neo4j.ogm.cypher.ComparisonOperator.*;

import java.util.Arrays;
import java.util.List;

import org.junit.Test;
import org.neo4j.ogm.cypher.BooleanOperator;
import org.neo4j.ogm.cypher.ComparisonOperator;
import org.neo4j.ogm.cypher.Filter;
import org.neo4j.ogm.cypher.Filters;
import org.neo4j.ogm.cypher.query.PagingAndSortingQuery;
import org.neo4j.ogm.domain.music.Node;
import org.neo4j.ogm.domain.music.RelBetweenNodes;
import org.neo4j.ogm.exception.core.InvalidDepthException;
import org.neo4j.ogm.exception.core.MissingOperatorException;
import org.neo4j.ogm.session.request.strategy.QueryStatements;
Expand Down Expand Up @@ -173,12 +176,12 @@ public void testFindByNestedPropertyOutgoing() {
planetFilter.setRelationshipType("ORBITS");
planetFilter.setRelationshipDirection("OUTGOING");
assertThat(query.findByType("ORBITS", new Filters().add(planetFilter), 4).getStatement())
.isEqualTo("MATCH (n:`Planet`) WHERE n.`name` = $`world_name_0` " +
"MATCH (n)-[r0:`ORBITS`]->(m) WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m " +
"MATCH p1 = (n)-[*0..4]-() WITH r0, COLLECT(DISTINCT p1) AS startPaths, m " +
"MATCH p2 = (m)-[*0..4]-() WITH r0, startPaths, COLLECT(DISTINCT p2) AS endPaths " +
"WITH r0,startPaths + endPaths AS paths " +
"UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
.isEqualTo(
"MATCH (n:`Planet`)-[r0:`ORBITS`]->(m) WHERE ( n.`name` = $`world_name_0` ) "
+ "WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m MATCH p1 = (n)-[*0..4]-() "
+ "WITH r0, COLLECT(DISTINCT p1) AS startPaths, m "
+ "MATCH p2 = (m)-[*0..4]-() WITH r0, startPaths, COLLECT(DISTINCT p2) AS endPaths "
+ "WITH r0,startPaths + endPaths AS paths UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
}

@Test // DATAGRAPH-632
Expand All @@ -189,11 +192,12 @@ public void testFindByNestedPropertyIncoming() {
planetFilter.setRelationshipType("ORBITS");
planetFilter.setRelationshipDirection("INCOMING");
assertThat(query.findByType("ORBITS", new Filters().add(planetFilter), 4).getStatement())
.isEqualTo("MATCH (m:`Planet`) WHERE m.`name` = $`world_name_0` MATCH (n)<-[r0:`ORBITS`]-(m) " +
"WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m MATCH p1 = (n)-[*0..4]-() " +
"WITH r0, COLLECT(DISTINCT p1) AS startPaths, m " +
"MATCH p2 = (m)-[*0..4]-() WITH r0, startPaths, COLLECT(DISTINCT p2) AS endPaths " +
"WITH r0,startPaths + endPaths AS paths UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
.isEqualTo(
"MATCH (n)<-[r0:`ORBITS`]-(m:`Planet`) WHERE ( m.`name` = $`world_name_0` ) "
+ "WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m MATCH p1 = (n)-[*0..4]-() "
+ "WITH r0, COLLECT(DISTINCT p1) AS startPaths, m "
+ "MATCH p2 = (m)-[*0..4]-() WITH r0, startPaths, COLLECT(DISTINCT p2) AS endPaths "
+ "WITH r0,startPaths + endPaths AS paths UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
}

@Test // DATAGRAPH-632
Expand All @@ -212,12 +216,12 @@ public void testFindByMultipleNestedProperties() {
planetMoonsFilter.setBooleanOperator(BooleanOperator.AND);

assertThat(query.findByType("ORBITS", new Filters().add(planetNameFilter, planetMoonsFilter), 4).getStatement())
.isEqualTo("MATCH (n:`Planet`) WHERE n.`name` = $`world_name_0` AND n.`moons` = $`moons_moons_1` " +
"MATCH (n)-[r0:`ORBITS`]->(m) WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m " +
"MATCH p1 = (n)-[*0..4]-() WITH r0, COLLECT(DISTINCT p1) AS startPaths, m " +
"MATCH p2 = (m)-[*0..4]-() WITH r0, startPaths, COLLECT(DISTINCT p2) AS endPaths " +
"WITH r0,startPaths + endPaths AS paths " +
"UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
.isEqualTo(
"MATCH (n:`Planet`)-[r0:`ORBITS`]->(m) WHERE ( n.`name` = $`world_name_0` AND n.`moons` = $`moons_moons_1` ) "
+ "WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m "
+ "MATCH p1 = (n)-[*0..4]-() WITH r0, COLLECT(DISTINCT p1) AS startPaths, m "
+ "MATCH p2 = (m)-[*0..4]-() WITH r0, startPaths, COLLECT(DISTINCT p2) AS endPaths "
+ "WITH r0,startPaths + endPaths AS paths UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
}

@Test // DATAGRAPH-632
Expand All @@ -236,12 +240,11 @@ public void testFindByMultipleNestedPropertiesOnBothEnds() {
planetFilter.setRelationshipDirection("INCOMING");

assertThat(query.findByType("ORBITS", new Filters().add(moonFilter, planetFilter), 4).getStatement()).isEqualTo(
"MATCH (n:`Moon`) WHERE n.`name` = $`world_name_0` MATCH (m:`Planet`) WHERE m.`colour` = $`colour_colour_1` "
+
"MATCH (n)-[r0:`ORBITS`]->(m) WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m " +
"MATCH p1 = (n)-[*0..4]-() WITH r0, COLLECT(DISTINCT p1) AS startPaths, m " +
"MATCH p2 = (m)-[*0..4]-() WITH r0, startPaths, COLLECT(DISTINCT p2) AS endPaths " +
"WITH r0,startPaths + endPaths AS paths UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
"MATCH (n:`Moon`)-[r0:`ORBITS`]->(m:`Planet`) WHERE ( n.`name` = $`world_name_0` AND m.`colour` = $`colour_colour_1` ) "
+ "WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m "
+ "MATCH p1 = (n)-[*0..4]-() WITH r0, COLLECT(DISTINCT p1) AS startPaths, m "
+ "MATCH p2 = (m)-[*0..4]-() WITH r0, startPaths, COLLECT(DISTINCT p2) AS endPaths "
+ "WITH r0,startPaths + endPaths AS paths UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
}

@Test // DATAGRAPH-632
Expand Down Expand Up @@ -306,12 +309,12 @@ public void testFindByBaseAndNestedPropertyOutgoing() {
Filter time = new Filter("time", ComparisonOperator.EQUALS, 3600);
time.setBooleanOperator(BooleanOperator.AND);
assertThat(query.findByType("ORBITS", new Filters().add(planetFilter, time), 4).getStatement())
.isEqualTo("MATCH (n:`Planet`) WHERE n.`name` = $`world_name_0` " +
"MATCH (n)-[r0:`ORBITS`]->(m) WHERE r0.`time` = $`time_1` " +
"WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m MATCH p1 = (n)-[*0..4]-() " +
"WITH r0, COLLECT(DISTINCT p1) AS startPaths, m MATCH p2 = (m)-[*0..4]-() " +
"WITH r0, startPaths, COLLECT(DISTINCT p2) AS endPaths " +
"WITH r0,startPaths + endPaths AS paths UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
.isEqualTo(
"MATCH (n:`Planet`)-[r0:`ORBITS`]->(m) WHERE ( n.`name` = $`world_name_0` ) AND r0.`time` = $`time_1` "
+ "WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m "
+ "MATCH p1 = (n)-[*0..4]-() WITH r0, COLLECT(DISTINCT p1) AS startPaths, m "
+ "MATCH p2 = (m)-[*0..4]-() WITH r0, startPaths, COLLECT(DISTINCT p2) AS endPaths "
+ "WITH r0,startPaths + endPaths AS paths UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
}

@Test // DATAGRAPH-632
Expand All @@ -323,12 +326,13 @@ public void testFindByBaseAndNestedPropertyIncoming() {
planetFilter.setRelationshipDirection("INCOMING");
Filter time = new Filter("time", ComparisonOperator.EQUALS, 3600);
assertThat(query.findByType("ORBITS", new Filters().add(planetFilter, time), 4).getStatement())
.isEqualTo("MATCH (m:`Planet`) WHERE m.`name` = $`world_name_0` " +
"MATCH (n)<-[r0:`ORBITS`]-(m) WHERE r0.`time` = $`time_1` " +
"WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m MATCH p1 = (n)-[*0..4]-() " +
"WITH r0, COLLECT(DISTINCT p1) AS startPaths, m MATCH p2 = (m)-[*0..4]-() " +
"WITH r0, startPaths, COLLECT(DISTINCT p2) AS endPaths " +
"WITH r0,startPaths + endPaths AS paths UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
.isEqualTo(
"MATCH (n)<-[r0:`ORBITS`]-(m:`Planet`) WHERE ( m.`name` = $`world_name_0` ) AND r0.`time` = $`time_1` "
+ "WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m "
+ "MATCH p1 = (n)-[*0..4]-() WITH r0, COLLECT(DISTINCT p1) AS startPaths, m "
+ "MATCH p2 = (m)-[*0..4]-() WITH r0, startPaths, "
+ "COLLECT(DISTINCT p2) AS endPaths "
+ "WITH r0,startPaths + endPaths AS paths UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
}

@Test // DATAGRAPH-632
Expand All @@ -350,14 +354,11 @@ public void testFindByBaseAndMultipleNestedPropertiesOnBothEnds() {

assertThat(query.findByType("ORBITS", new Filters().add(moonFilter, planetFilter, time), 4).getStatement())
.isEqualTo(
"MATCH (n:`Moon`) WHERE n.`name` = $`world_name_0` MATCH (m:`Planet`) WHERE m.`colour` = $`colour_colour_1` "
+
"MATCH (n)-[r0:`ORBITS`]->(m) WHERE r0.`time` = $`time_2` " +
"WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m MATCH p1 = (n)-[*0..4]-() " +
"WITH r0, COLLECT(DISTINCT p1) AS startPaths, m MATCH p2 = (m)-[*0..4]-() " +
"WITH r0, startPaths, COLLECT(DISTINCT p2) AS endPaths WITH r0,startPaths + endPaths AS paths "
+
"UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
"MATCH (n:`Moon`)-[r0:`ORBITS`]->(m:`Planet`) WHERE ( n.`name` = $`world_name_0` AND m.`colour` = $`colour_colour_1` ) AND r0.`time` = $`time_2` "
+ "WITH DISTINCT(r0) as r0,startnode(r0) AS n, endnode(r0) AS m "
+ "MATCH p1 = (n)-[*0..4]-() WITH r0, COLLECT(DISTINCT p1) AS startPaths, m "
+ "MATCH p2 = (m)-[*0..4]-() WITH r0, startPaths, COLLECT(DISTINCT p2) AS endPaths "
+ "WITH r0,startPaths + endPaths AS paths UNWIND paths AS p RETURN DISTINCT p, ID(r0)");
}

@Test(expected = MissingOperatorException.class) // GH-73
Expand Down

0 comments on commit c675e8b

Please sign in to comment.