Skip to content

Commit

Permalink
refactor: Improve returning and with. (#551)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
michael-simons authored Jan 9, 2023
1 parent 251838b commit 10080df
Show file tree
Hide file tree
Showing 16 changed files with 253 additions and 544 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,15 @@ private Asterisk() {
public String asString() {
return super.getContent();
}

enum IdentifiableAsterisk implements IdentifiableElement {

INSTANCE;

@NotNull
@Override
public Expression asExpression() {
return Asterisk.INSTANCE;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -456,21 +456,6 @@ public static StatementBuilder.OrderableOngoingReadingAndWithWithoutWhere with(S
return Statement.builder().with(variables);
}

/**
* Starts a statement with a leading {@code WITH}. Those are useful for passing on lists of various type that
* can be unwound later on etc. A leading {@code WITH} cannot be used with patterns obviously and needs its
* arguments to have an alias.
*
* @param variables One ore more variables.
* @return An ongoing with clause.
* @since 2021.2.2
*/
@NotNull @Contract(pure = true)
public static StatementBuilder.OrderableOngoingReadingAndWithWithoutWhere with(SymbolicName... variables) {

return Statement.builder().with(variables);
}

/**
* Starts a statement with a leading {@code WITH}. Those are useful for passing on lists of various type that
* can be unwound later on etc. A leading {@code WITH} cannot be used with patterns obviously and needs its
Expand All @@ -486,21 +471,6 @@ public static StatementBuilder.OrderableOngoingReadingAndWithWithoutWhere with(I
return Statement.builder().with(elements);
}

/**
* Starts a statement with a leading {@code WITH}. Those are useful for passing on lists of various type that
* can be unwound later on etc. A leading {@code WITH} cannot be used with patterns obviously and needs its
* arguments to have an alias.
*
* @param expressions One or more aliased expressions.
* @return An ongoing with clause.
* @since 2021.2.2
*/
@NotNull @Contract(pure = true)
public static StatementBuilder.OrderableOngoingReadingAndWithWithoutWhere with(AliasedExpression... expressions) {

return Statement.builder().with(expressions);
}

/**
* Starts a statement with a leading {@code WITH}. Those are useful for passing on lists of various type that
* can be unwound later on etc. A leading {@code WITH} cannot be used with patterns obviously and needs its
Expand All @@ -509,31 +479,14 @@ public static StatementBuilder.OrderableOngoingReadingAndWithWithoutWhere with(A
* This method takes both aliased and non-aliased expression. The later will produce only valid Cypher when used in
* combination with a correlated subquery via {@link Cypher#call(Statement)}.
*
* @param expressions One or more expressions.
* @return An ongoing with clause.
*/
@NotNull @Contract(pure = true)
public static StatementBuilder.OrderableOngoingReadingAndWithWithoutWhere with(Expression... expressions) {

return Statement.builder().with(expressions);
}

/**
* Starts a statement with a leading {@code WITH}. Those are useful for passing on lists of various type that
* can be unwound later on etc. A leading {@code WITH} cannot be used with patterns obviously and needs its
* arguments to have an alias.
* <p>
* This method takes both aliased and non-aliased expression. The later will produce only valid Cypher when used in
* combination with a correlated subquery via {@link Cypher#call(Statement)}.
*
* @param expressions One ore more expressions.
* @param elements One ore more expressions.
* @return An ongoing with clause.
* @since 2021.2.2
*/
@NotNull @Contract(pure = true)
public static StatementBuilder.OrderableOngoingReadingAndWithWithoutWhere with(Collection<Expression> expressions) {
public static StatementBuilder.OrderableOngoingReadingAndWithWithoutWhere with(Collection<IdentifiableElement> elements) {

return with(expressions.toArray(new Expression[] {}));
return Statement.builder().with(elements);
}

/**
Expand Down Expand Up @@ -810,7 +763,7 @@ public static Statement unionAll(Collection<Statement> statements) {
/**
* A {@literal RETURN} statement without a previous match.
*
* @param expressions The expressions to return
* @param expressions The elements to return
* @return A buildable statement
* @since 1.0.1
*/
Expand All @@ -828,7 +781,7 @@ public static StatementBuilder.OngoingReadingAndReturn returning(Expression... e
*/
@NotNull @Contract(pure = true)
public static StatementBuilder.OngoingReadingAndReturn returning(Collection<Expression> expressions) {
return returning(expressions.toArray(new Expression[] {}));
return Statement.builder().returning(expressions);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
@API(status = INTERNAL, since = "2021.2.1")
final class DefaultLoadCSVStatementBuilder extends DefaultStatementBuilder implements LoadCSVStatementBuilder {

static class PrepareLoadCSVStatementImpl implements ExposesLoadCSV, OngoingLoadCSV {
static final class PrepareLoadCSVStatementImpl implements ExposesLoadCSV, OngoingLoadCSV {

private final UsingPeriodicCommit usingPeriodicCommit;
private final DefaultStatementBuilder source;
Expand Down
Loading

0 comments on commit 10080df

Please sign in to comment.