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

Using path patterns in WHERE #206

Closed romain-rossi closed 4 years ago

romain-rossi commented 4 years ago

Allows to add path pattern to clause WHERE (https://neo4j.com/docs/cypher-manual/current/clauses/where/#query-where-patterns).

For now, it's possible to create where conditions from RelationshipPattern using functions size(relationship)... or exists(relationship).... But this workaround has performance issue compare to use directly path pattern in WHERE.

Pull Request: I will be happy to provide a PR for this improvement, could you please point me in the right direction to do it? Should I generalize the method where(Condition) parameter by using Expression instead of Condition? or make RelationshipPattern implements Condition? or...

michael-simons commented 4 years ago

Going to have a look, thank you!

michael-simons commented 4 years ago

I prefer "or": Adding an overload to where which accepts a RelationshipPattern, which would fit the docs. The builder can wrap it in a condition… I'm trying this out.

It's a bit sad that the OpenCypher spec uses the notion of RelationshipPattern and not PathPattern like the docs do, but as I modelled everything after the spec, I stick with RelationshipPattern.

romain-rossi commented 4 years ago

Thank you @michael-simons! I don't know if it's related but it could be great if Relationship and RelationshipChain share the same "features" to easily add relations without lot of casting:

        final Node a = node("A").named("a");
        final Node b = node("B").named("b");
        final Node c = node("C").named("c");
        ...
        // Relationship
        RelationshipPattern relation = a.relationshipTo(b);
        ...
        // RelationshipChain
        chain = (RelationshipPattern) ((ExposesRelationships<?>) releation).relationshipTo(c);
        ....

Idealy:

        final Node a = node("A").named("a");
        final Node b = node("B").named("b");
        final Node c = node("C").named("c");
        ...
        [Relationship] relation = a.relationshipTo(b);
        ...
        (relation = )relation.relationshipTo(c);
        ....
michael-simons commented 4 years ago

@romain-rossi I think this is solvable by moving ExposesRelationships to the RelationshipPattern. I would favour this in contrast to make a chain of relationships (aka RelationshipChain) a relationship itself.

final Node a = Cypher.node("A").named("a");
final Node b = Cypher.node("B").named("b");
final Node c = Cypher.node("C").named("c");

RelationshipPattern relation = a.relationshipTo(b);
relation = relation.relationshipTo©;
romain-rossi commented 4 years ago

I tried that on my fork :-):

public interface RelationshipPattern<T> extends PatternElement, ExposesRelationships<T> {
}

It is nicer (less casts) but still a cast (generics):

final Node a = node("A").named("a");
final Node b = node("B").named("b");
final Node c = node("C").named("c");

RelationshipPattern<?> r = a.relationshipTo(b);
r = (RelationshipPattern<?>) r.relationshipFrom(c);

Statement s = match(r)
    .returning(a)
    .build();

assertThat(cypherRenderer.render(s))
    .isEqualTo("MATCH (a:`A`)-->(b:`B`)<--(c:`C`)

I can quickly create a PR if it's fine with you...

michael-simons commented 4 years ago

Already have it here… It works without casts as shown above.