neo4j / sdn-rx

Nextgen Spring Data module for Neo4j supporting (not only) reactive data access and immutable support
https://neo4j.github.io/sdn-rx
Apache License 2.0
65 stars 23 forks source link

WITH and WHERE clauses are missing using Cypher DSL #167

Closed romain-rossi closed 4 years ago

romain-rossi commented 4 years ago

Environment

Description

I'm using org.neo4j.springframework.data.core.cypher.Cypher to create the following (partial) cypher:

MATCH
  (app:Location{uuid:$app_uuid})<-[:PART_OF*0..3]-(loc_start:Location),
  (loc_start)<-[:IN|:IN_ANALYTICS]-(r:Resume)
WITH DISTINCT r, loc_start, app
MATCH (r:Resume)-[:IN_COHORT_OF]->(o:Offer {is_valid:true})-[:IN]->(app)
WITH DISTINCT r, o, loc_start, app
MATCH (o)-[:FOR]->(start_n:ResumeNode)
WHERE (ID(start_n) IN $start_ids)
WITH DISTINCT r, start_n, app, loc_start, o
...

Here is a link on a dummy JUnit test with the code I'm using : DummyTest.java

The test output below shows than the "WITH" and "WHERE" clauses are missing:

MATCH 
  (app:`Location` {uuid: $app_uuid})<-[:`PART_OF*0..3`]-(loc_start:`Location`), 
  (loc_start)<-[:`IN`]-(r:`Resume`)
WITH DISTINCT r, loc_start, app 
MATCH (r)-[:`IN_COHORT_OF`]->(o:`Offer` {is_valid: true})-[:`IN`]->(app) 
MATCH (o:`Offer`)-[:`FOR`]->(start_n:`ResumeNode`) 
...
michael-simons commented 4 years ago

Hello Romain,

thanks for reporting this issue.

As you might have noticed, I extracted a couple of things: We couldn't render relationship cardinalities and we didn't deal with with multiple relationship types correctly. This is implemented / fixed now.

The reason your dummy test is not working as expect is that the builder are immutable. If you split the building process into multiple parts, you have to assign the returned steps to a variable and use that. I.e. using the returned value from with and work with that.

You don't even have to care about the types (I never do this with builders like that, see a sql example here) when on JDK 11+ and just declare the variables as var.

So, with the other fixes in place, you query can look like this with JDK 11+:

@Test
void gh167() {
    var app = node("Location").named("app").properties("uuid", parameter("app_uuid"));
    var locStart = node("Location").named("loc_start");
    var resume = node("Resume").named("r");
    var offer = node("Offer").named("o");
    var startN = node("ResumeNode").named("start_n");

    var aFl = app
        .relationshipFrom(locStart, "PART_OF").length(0, 3)
        .relationshipFrom(resume, "IN", "IN_ANALYTICS");

    var statement = match(aFl)
        .withDistinct(resume, locStart, app)
        .match(resume
            .relationshipTo(offer.properties("is_valid", literalTrue()), "IN_COHORT_OF")
            .relationshipTo(anyNode("app"), "IN")
        )
        .withDistinct(resume, locStart, app, offer)
        .match(offer.relationshipTo(startN, "FOR"))
        .where(Functions.id(name("start_n")).in(parameter("start_ids")))
        .returningDistinct(resume, locStart, app, offer, startN).build();

    assertThat(cypherRenderer.render(statement))
        .isEqualTo(
            "MATCH (app:`Location` {uuid: $app_uuid})<-[:`PART_OF`*0..3]-(loc_start:`Location`)<-[:`IN`|`IN_ANALYTICS`]-(r:`Resume`) WITH DISTINCT r, loc_start, app MATCH (r)-[:`IN_COHORT_OF`]->(o:`Offer` {is_valid: true})-[:`IN`]->(app) WITH DISTINCT r, loc_start, app, o MATCH (o:`Offer`)-[:`FOR`]->(start_n:`ResumeNode`) WHERE id(start_n) IN $start_ids RETURN DISTINCT r, loc_start, app, o, start_n");
}

In our test for it I have used of course type, as we must be compatible with JDK 8.

romain-rossi commented 4 years ago

Thanks a lot Michaël! Should I work on the https://github.com/neo4j/sdn-rx/tree/issue/167 branch to get the fix?

romain-rossi commented 4 years ago

Thanks a lot Michaël! Should I work on the https://github.com/neo4j/sdn-rx/tree/issue/167 branch to get the fix?

All good using the artifacts from the latest master build: https://github.com/neo4j/sdn-rx/actions/runs/66340906