Skip to content

Commit a315202

Browse files
michael-simonsmichael-simons
authored
GH-96 - Make sure aliased expressions are not rendered multiple times.
This became appearent when trying to formulate ``` String query = "" + "MATCH (n:Order) - [r:HAS] -> (l:LineItem) " + "WHERE n.id = $id " + "WITH n, sum(l.price * l.quantity) AS netAmount, collect(r) as hasRel, collect(l) as lineItems " + "RETURN {" + " netAmount: netAmount," + " taxAmount: netAmount * $taxRate," + " totalAmount: netAmount * (1+$taxRate)," + " hasRel: hasRel, lineItems: lineItems" + "}"; ``` for the spring data neo4j native project.
1 parent 3fe8a67 commit a315202

File tree

4 files changed

+109
-17
lines changed

4 files changed

+109
-17
lines changed

‎neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/AliasedExpression.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.neo4j.cypherdsl.core;
2020

2121
import static org.apiguardian.api.API.Status.EXPERIMENTAL;
22+
import static org.apiguardian.api.API.Status.INTERNAL;
2223

2324
import org.apiguardian.api.API;
2425
import org.neo4j.cypherdsl.core.support.Visitor;
@@ -47,7 +48,8 @@ public String getAlias() {
4748
return alias;
4849
}
4950

50-
Expression getDelegate() {
51+
@API(status = INTERNAL)
52+
public Expression getDelegate() {
5153
return delegate;
5254
}
5355

‎neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/renderer/RenderingVisitor.java‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
*/
1919
package org.neo4j.cypherdsl.core.renderer;
2020

21+
import java.util.Deque;
2122
import java.util.HashSet;
23+
import java.util.LinkedList;
2224
import java.util.Locale;
2325
import java.util.Map;
2426
import java.util.Optional;
@@ -111,6 +113,11 @@ class RenderingVisitor extends ReflectiveVisitor {
111113
*/
112114
private final java.util.Set<Named> visitedNamed = new HashSet<>();
113115

116+
/**
117+
* Keeps track if currently in an aliased expression so that the content can be skipped when already visited.
118+
*/
119+
private final Deque<AliasedExpression> currentAliasedElements = new LinkedList<>();
120+
114121
/**
115122
* Keeps track of unresolved symbolic names.
116123
*/
@@ -150,15 +157,18 @@ private String resolve(SymbolicName symbolicName) {
150157
});
151158
}
152159

153-
boolean inc;
154-
155160
@Override
156161
protected boolean preEnter(Visitable visitable) {
157162

158-
if (skipNodeContent) {
163+
Visitable lastAliased = currentAliasedElements.peek();
164+
if (skipNodeContent || visitableToAliased.containsValue(lastAliased)) {
159165
return false;
160166
}
161167

168+
if (visitable instanceof AliasedExpression) {
169+
currentAliasedElements.push((AliasedExpression) visitable);
170+
}
171+
162172
int nextLevel = ++currentLevel + 1;
163173
if (visitable instanceof TypedSubtree) {
164174
enableSeparator(nextLevel, true);
@@ -178,6 +188,10 @@ protected void postLeave(Visitable visitable) {
178188
enableSeparator(currentLevel + 1, false);
179189
}
180190

191+
if (currentAliasedElements.peek() == visitable) {
192+
currentAliasedElements.pop();
193+
}
194+
181195
--currentLevel;
182196
}
183197

@@ -241,8 +255,18 @@ void leave(Delete match) {
241255
builder.append(" ");
242256
}
243257

258+
void enter(AliasedExpression aliased) {
259+
260+
if (this.visitableToAliased.containsValue(aliased)) {
261+
builder.append(aliased.getAlias());
262+
}
263+
}
264+
244265
void leave(AliasedExpression aliased) {
245-
builder.append(" AS ").append(aliased.getAlias());
266+
267+
if (!this.visitableToAliased.containsValue(aliased)) {
268+
builder.append(" AS ").append(aliased.getAlias());
269+
}
246270
}
247271

248272
void enter(NestedExpression nested) {

‎neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/support/ReflectiveVisitor.java‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,16 @@
2121
import java.lang.reflect.Method;
2222
import java.util.ArrayList;
2323
import java.util.Deque;
24+
import java.util.HashMap;
2425
import java.util.LinkedList;
2526
import java.util.List;
2627
import java.util.Map;
2728
import java.util.Objects;
2829
import java.util.Optional;
2930
import java.util.concurrent.ConcurrentHashMap;
3031

32+
import org.neo4j.cypherdsl.core.AliasedExpression;
33+
3134
/**
3235
* This is a convenience class implementing a {@link Visitor} and it takes care of choosing the right methods
3336
* to dispatch the {@link Visitor#enter(Visitable)} and {@link Visitor#leave(Visitable)} calls to.
@@ -64,7 +67,9 @@ private enum Phase {
6467
*/
6568
private static final Map<TargetAndPhase, Optional<Method>> VISITING_METHODS_CACHE = new ConcurrentHashMap<>();
6669

67-
/** Keeps track of the ASTs current level. */
70+
/**
71+
* Keeps track of the ASTs current level.
72+
*/
6873
private Deque<Visitable> currentVisitedElements = new LinkedList<>();
6974

7075
/**
@@ -86,23 +91,36 @@ private enum Phase {
8691
*/
8792
protected abstract void postLeave(Visitable visitable);
8893

94+
protected final Map<Visitable, AliasedExpression> visitableToAliased = new HashMap<>();
95+
96+
private Visitable getAliasedIfSeen(Visitable visitable) {
97+
return visitableToAliased.containsKey(visitable) ? visitableToAliased.get(visitable) : visitable;
98+
}
99+
89100
@Override
90101
public final void enter(Visitable visitable) {
91102

92-
if (preEnter(visitable)) {
93-
currentVisitedElements.push(visitable);
94-
executeConcreteMethodIn(new TargetAndPhase(this, visitable.getClass(), Phase.ENTER), visitable);
103+
Visitable used = getAliasedIfSeen(visitable);
104+
if (preEnter(used)) {
105+
currentVisitedElements.push(used);
106+
executeConcreteMethodIn(new TargetAndPhase(this, used.getClass(), Phase.ENTER), used);
95107
}
96108
}
97109

98110
@Override
99111
public final void leave(Visitable visitable) {
100112

101-
if (currentVisitedElements.peek() == visitable) {
102-
executeConcreteMethodIn(new TargetAndPhase(this, visitable.getClass(), Phase.LEAVE), visitable);
103-
postLeave(visitable);
113+
Visitable used = getAliasedIfSeen(visitable);
114+
if (currentVisitedElements.peek() == used) {
115+
executeConcreteMethodIn(new TargetAndPhase(this, used.getClass(), Phase.LEAVE), used);
116+
postLeave(used);
104117
currentVisitedElements.pop();
105118
}
119+
120+
if (visitable instanceof AliasedExpression) {
121+
AliasedExpression aliasedExpression = (AliasedExpression) visitable;
122+
visitableToAliased.put((aliasedExpression).getDelegate(), aliasedExpression);
123+
}
106124
}
107125

108126
private void executeConcreteMethodIn(TargetAndPhase targetAndPhase, Visitable onVisitable) {

‎neo4j-cypher-dsl/src/test/java/org/neo4j/cypherdsl/core/CypherIT.java‎

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3341,11 +3341,13 @@ void shouldWorkWithMapProjections() {
33413341
.returning(
33423342
person.project(
33433343
"livesIn",
3344-
Cypher.valueAt(Cypher.listBasedOn(person.relationshipTo(location, "LIVES_IN")).returning(location.project("name")), 0)
3344+
Cypher.valueAt(Cypher.listBasedOn(person.relationshipTo(location, "LIVES_IN"))
3345+
.returning(location.project("name")), 0)
33453346
)
33463347
).build();
33473348
assertThat(cypherRenderer.render(statement))
3348-
.isEqualTo("MATCH (person:`Person`) RETURN person{livesIn: [(person)-[:`LIVES_IN`]->(personLivesIn:`Location`) | personLivesIn{.name}][0]}");
3349+
.isEqualTo(
3350+
"MATCH (person:`Person`) RETURN person{livesIn: [(person)-[:`LIVES_IN`]->(personLivesIn:`Location`) | personLivesIn{.name}][0]}");
33493351
}
33503352

33513353
@Test
@@ -3358,13 +3360,59 @@ void shouldSupportExpressions() {
33583360
person.project(
33593361
"livesIn",
33603362
Cypher.subList(
3361-
Cypher.listBasedOn(person.relationshipTo(location, "LIVES_IN")).returning(location.project("name")),
3362-
Cypher.parameter("personLivedInOffset"), Cypher.parameter("personLivedInOffset").add(Cypher.parameter("personLivedInFirst"))
3363+
Cypher.listBasedOn(person.relationshipTo(location, "LIVES_IN"))
3364+
.returning(location.project("name")),
3365+
Cypher.parameter("personLivedInOffset"),
3366+
Cypher.parameter("personLivedInOffset").add(Cypher.parameter("personLivedInFirst"))
33633367
)
33643368
)
33653369
).build();
33663370
assertThat(cypherRenderer.render(statement))
3367-
.isEqualTo("MATCH (person:`Person`) RETURN person{livesIn: [(person)-[:`LIVES_IN`]->(personLivesIn:`Location`) | personLivesIn{.name}][$personLivedInOffset..($personLivedInOffset + $personLivedInFirst)]}");
3371+
.isEqualTo(
3372+
"MATCH (person:`Person`) RETURN person{livesIn: [(person)-[:`LIVES_IN`]->(personLivesIn:`Location`) | personLivesIn{.name}][$personLivedInOffset..($personLivedInOffset + $personLivedInFirst)]}");
3373+
}
3374+
}
3375+
3376+
@Nested
3377+
class DoubleRendering {
3378+
3379+
@Test
3380+
void aliasedFunctionsShouldNotBeRenderedTwiceInProjection() {
3381+
3382+
Node o = Cypher.node("Order").named("o");
3383+
Node li = Cypher.node("LineItem").named("li");
3384+
Relationship hasLineItems = o.relationshipTo(li).named("h");
3385+
3386+
Expression netAmount = Functions.sum(li.property("price").multiply(li.property("quantity")))
3387+
.as("netAmount");
3388+
Expression totalAmount = netAmount.multiply(Cypher.literalOf(1).add(Cypher.parameter("taxRate")))
3389+
.as("totalAmount");
3390+
Statement statement = Cypher.match(hasLineItems)
3391+
.where(o.property("id").isEqualTo(Cypher.parameter("id")))
3392+
.with(o.getRequiredSymbolicName(), netAmount, totalAmount)
3393+
.returning(o.project(o.property("x"), netAmount, totalAmount, netAmount.multiply(Cypher.parameter("taxRate"))
3394+
.as("taxAmount"))).build();
3395+
assertThat(cypherRenderer.render(statement)).isEqualTo(
3396+
"MATCH (o:`Order`)-[h]->(li:`LineItem`) WHERE o.id = $id WITH o, sum((li.price * li.quantity)) AS netAmount, (netAmount * (1 + $taxRate)) AS totalAmount RETURN o{.x, netAmount: netAmount, totalAmount: totalAmount, taxAmount: (netAmount * $taxRate)}");
3397+
}
3398+
3399+
@Test
3400+
void aliasedFunctionsShouldNotBeRenderedTwiceInReturn() {
3401+
3402+
Node o = Cypher.node("Order").named("o");
3403+
Node li = Cypher.node("LineItem").named("li");
3404+
Relationship hasLineItems = o.relationshipTo(li).named("h");
3405+
3406+
Expression netAmount = Functions.sum(li.property("price").multiply(li.property("quantity")))
3407+
.as("netAmount");
3408+
Expression totalAmount = netAmount.multiply(Cypher.literalOf(1).add(Cypher.parameter("taxRate")))
3409+
.as("totalAmount");
3410+
Statement statement = Cypher.match(hasLineItems)
3411+
.where(o.property("id").isEqualTo(Cypher.parameter("id")))
3412+
.with(o.getRequiredSymbolicName(), netAmount, totalAmount)
3413+
.returning(netAmount, totalAmount).build();
3414+
assertThat(cypherRenderer.render(statement)).isEqualTo(
3415+
"MATCH (o:`Order`)-[h]->(li:`LineItem`) WHERE o.id = $id WITH o, sum((li.price * li.quantity)) AS netAmount, (netAmount * (1 + $taxRate)) AS totalAmount RETURN netAmount, totalAmount");
33683416
}
33693417
}
33703418
}

0 commit comments

Comments
 (0)