Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove indirect inheritance from Named to Node and Relationship. #550

Open
michael-simons opened this issue Jan 9, 2023 · 0 comments
Open

Comments

@michael-simons
Copy link
Collaborator

Entities such as Node and Relationship are not named by default. The indirect inheritance is plain wrong.

michael-simons added a commit that referenced this issue Jan 9, 2023
This addresses and fixes #547 to some extend. The goal was to allow compilation of

```java
var cypher = Cypher.match(person)
    .returning(
        person,
        Expressions.count(person.relationshipTo(person, "ACTED_IN")).as("acted_in_rel_count")
    ).build();
```

without aliasing the `person` node with `as`.

This could be achieved by allowing `IdentifiableElement` on the `returning` methods the same way this PR does with the `with` methods. However, in contrast to `with`, `return` does allow non-aliased expressions, so the overload taking in `Expression` must continue to exists. That will rule out to add another overload with a `Collection<SomethingElse>`, as they will have the same type erasure as existing methods. Adding a varg of `IdentifiableElement` will break a gazillion lines of code, even in our tests, as the method will then be ambiguous. 

I think the original sin was having `Node` and `Relationship` indirectly implement `Named` through `PropertyContainer`. If that wasn't the case, `Named` could become an expression and things will resolve nicely without additional overloads.

That change however would take several engineering weeks and turn the DSL inside out and can't be part of this change.

We created #550 for the next major release after `2023.0.0` to track this issue.

This change here will require some changes to calling code of `with`, but getting rid of about 250 LoC is worth the change, in addition, `with` will actually be nicer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant