Skip to content

Commit 7c04896

Browse files
meistermeiermeistermeier
committed
GH-2750 - Avoid toString in relationship creation.
Without the string conversion Neo4j will either to an (element)id or label seek instead of a full node scan. Also: In some corner case adding labels to the generic cyclic query can improve the performance of the query. On the other hand it cannot make it worse. Closes #2750
1 parent b1c7b07 commit 7c04896

File tree

5 files changed

+62
-40
lines changed

5 files changed

+62
-40
lines changed

‎src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,12 @@ private <T> T saveImpl(T instance, @Nullable Collection<PropertyFilter.Projected
409409
throw new IllegalStateException("Could not retrieve an internal id while saving");
410410
}
411411

412-
String elementId = newOrUpdatedNode.map(IdentitySupport::getElementId).get();
412+
Object elementId = newOrUpdatedNode.map(node -> {
413+
if (!entityMetaData.isUsingDeprecatedInternalId() && entityMetaData.isUsingInternalIds()) {
414+
return IdentitySupport.getElementId(node);
415+
}
416+
return node.id();
417+
}).get();
413418

414419
PersistentPropertyAccessor<T> propertyAccessor = entityMetaData.getPropertyAccessor(entityToBeSaved);
415420
TemplateSupport.setGeneratedIdIfNecessary(entityMetaData, propertyAccessor, elementId, newOrUpdatedNode);
@@ -807,14 +812,14 @@ private <T> T processNestedRelations(
807812
? stateMachine.getProcessedAs(relatedObjectBeforeCallbacksApplied)
808813
: eventSupport.maybeCallBeforeBind(relatedObjectBeforeCallbacksApplied);
809814

810-
String relatedInternalId;
815+
Object relatedInternalId;
811816
Entity savedEntity = null;
812817
// No need to save values if processed
813818
if (stateMachine.hasProcessedValue(relatedValueToStore)) {
814819
relatedInternalId = stateMachine.getObjectId(relatedValueToStore);
815820
} else {
816821
savedEntity = saveRelatedNode(newRelatedObject, targetEntity, includeProperty, currentPropertyPath);
817-
relatedInternalId = TemplateSupport.rendererCanUseElementIdIfPresent(renderer) ? savedEntity.elementId() : Long.toString(savedEntity.id());
822+
relatedInternalId = TemplateSupport.rendererCanUseElementIdIfPresent(renderer, targetEntity) ? savedEntity.elementId() : savedEntity.id();
818823
stateMachine.markEntityAsProcessed(relatedValueToStore, relatedInternalId);
819824
if (relatedValueToStore instanceof MappingSupport.RelationshipPropertiesWithEntityHolder) {
820825
Object entity = ((MappingSupport.RelationshipPropertiesWithEntityHolder) relatedValueToStore).getRelatedEntity();

‎src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,9 @@ private <T> Mono<T> saveImpl(T instance, @Nullable Collection<PropertyFilter.Pro
447447

448448
PersistentPropertyAccessor<T> propertyAccessor = entityMetaData.getPropertyAccessor(entityToBeSaved);
449449
return idMono.doOnNext(newOrUpdatedNode -> {
450-
var elementId = IdentitySupport.getElementId(newOrUpdatedNode);
450+
var elementId = !entityMetaData.isUsingDeprecatedInternalId() && entityMetaData.isUsingInternalIds()
451+
? IdentitySupport.getElementId(newOrUpdatedNode)
452+
: newOrUpdatedNode.id();
451453
TemplateSupport.setGeneratedIdIfNecessary(entityMetaData, propertyAccessor, elementId, Optional.of(newOrUpdatedNode));
452454
TemplateSupport.updateVersionPropertyIfPossible(entityMetaData, propertyAccessor, newOrUpdatedNode);
453455
finalStateMachine.markEntityAsProcessed(instance, elementId);
@@ -942,17 +944,17 @@ private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity
942944
.flatMap(newRelatedObject -> {
943945
Neo4jPersistentEntity<?> targetEntity = neo4jMappingContext.getRequiredPersistentEntity(relatedObjectBeforeCallbacksApplied.getClass());
944946

945-
Mono<Tuple2<AtomicReference<String>, AtomicReference<Entity>>> queryOrSave;
947+
Mono<Tuple2<AtomicReference<Object>, AtomicReference<Entity>>> queryOrSave;
946948
if (stateMachine.hasProcessedValue(relatedValueToStore)) {
947-
AtomicReference<String> relatedInternalId = new AtomicReference<>();
948-
String possibleValue = stateMachine.getObjectId(relatedValueToStore);
949+
AtomicReference<Object> relatedInternalId = new AtomicReference<>();
950+
Object possibleValue = stateMachine.getObjectId(relatedValueToStore);
949951
if (possibleValue != null) {
950952
relatedInternalId.set(possibleValue);
951953
}
952954
queryOrSave = Mono.just(Tuples.of(relatedInternalId, new AtomicReference<>()));
953955
} else {
954956
queryOrSave = saveRelatedNode(newRelatedObject, targetEntity, includeProperty, currentPropertyPath)
955-
.map(entity -> Tuples.of(new AtomicReference<>(TemplateSupport.rendererCanUseElementIdIfPresent(renderer) ? entity.elementId() : Long.toString(entity.id())), new AtomicReference<>(entity)))
957+
.map(entity -> Tuples.of(new AtomicReference<>((Object) (TemplateSupport.rendererCanUseElementIdIfPresent(renderer, targetEntity) ? entity.elementId() : entity.id())), new AtomicReference<>(entity)))
956958
.doOnNext(t -> {
957959
var relatedInternalId = t.getT1().get();
958960
stateMachine.markEntityAsProcessed(relatedValueToStore, relatedInternalId);
@@ -963,7 +965,7 @@ private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity
963965
});
964966
}
965967
return queryOrSave.flatMap(idAndEntity -> {
966-
String relatedInternalId = idAndEntity.getT1().get();
968+
Object relatedInternalId = idAndEntity.getT1().get();
967969
Entity savedEntity = idAndEntity.getT2().get();
968970
Neo4jPersistentProperty requiredIdProperty = targetEntity.getRequiredIdProperty();
969971
PersistentPropertyAccessor<?> targetPropertyAccessor = targetEntity.getPropertyAccessor(newRelatedObject);

‎src/main/java/org/springframework/data/neo4j/core/TemplateSupport.java‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -204,28 +204,26 @@ boolean hasRootNodeIds() {
204204

205205
Statement toStatement(NodeDescription<?> nodeDescription) {
206206

207-
String rootNodeIds = "rootNodeIds";
208-
String relationshipIds = "relationshipIds";
209-
String relatedNodeIds = "relatedNodeIds";
210-
Node rootNodes = Cypher.anyNode(rootNodeIds);
211-
Node relatedNodes = Cypher.anyNode(relatedNodeIds);
212-
Relationship relationships = Cypher.anyNode().relationshipBetween(Cypher.anyNode()).named(relationshipIds);
207+
String primaryLabel = nodeDescription.getPrimaryLabel();
208+
Node rootNodes = Cypher.node(primaryLabel).named(ROOT_NODE_IDS);
209+
Node relatedNodes = Cypher.anyNode(RELATED_NODE_IDS);
210+
Relationship relationships = Cypher.anyNode().relationshipBetween(Cypher.anyNode()).named(RELATIONSHIP_IDS);
213211
return Cypher.match(rootNodes)
214-
.where(Functions.elementId(rootNodes).in(Cypher.parameter(rootNodeIds)))
212+
.where(Functions.elementId(rootNodes).in(Cypher.parameter(ROOT_NODE_IDS)))
215213
.with(Functions.collect(rootNodes).as(Constants.NAME_OF_ROOT_NODE))
216214
.optionalMatch(relationships)
217-
.where(Functions.elementId(relationships).in(Cypher.parameter(relationshipIds)))
215+
.where(Functions.elementId(relationships).in(Cypher.parameter(RELATIONSHIP_IDS)))
218216
.with(Constants.NAME_OF_ROOT_NODE, Functions.collectDistinct(relationships).as(Constants.NAME_OF_SYNTHESIZED_RELATIONS))
219217
.optionalMatch(relatedNodes)
220-
.where(Functions.elementId(relatedNodes).in(Cypher.parameter(relatedNodeIds)))
218+
.where(Functions.elementId(relatedNodes).in(Cypher.parameter(RELATED_NODE_IDS)))
221219
.with(
222220
Constants.NAME_OF_ROOT_NODE,
223221
Cypher.name(Constants.NAME_OF_SYNTHESIZED_RELATIONS).as(Constants.NAME_OF_SYNTHESIZED_RELATIONS),
224222
Functions.collectDistinct(relatedNodes).as(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES)
225223
)
226-
.unwind(Constants.NAME_OF_ROOT_NODE).as(rootNodeIds)
224+
.unwind(Constants.NAME_OF_ROOT_NODE).as(ROOT_NODE_IDS)
227225
.with(
228-
Cypher.name(rootNodeIds).as(Constants.NAME_OF_TYPED_ROOT_NODE.apply(nodeDescription).getValue()),
226+
Cypher.name(ROOT_NODE_IDS).as(Constants.NAME_OF_TYPED_ROOT_NODE.apply(nodeDescription).getValue()),
229227
Cypher.name(Constants.NAME_OF_SYNTHESIZED_RELATIONS),
230228
Cypher.name(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES))
231229
.orderBy(queryFragments.getOrderBy())
@@ -353,7 +351,7 @@ public Map<String, Object> apply(T t) {
353351
static <T> void setGeneratedIdIfNecessary(
354352
Neo4jPersistentEntity<?> entityMetaData,
355353
PersistentPropertyAccessor<T> propertyAccessor,
356-
String elementId,
354+
Object elementId,
357355
Optional<Entity> databaseEntity
358356
) {
359357
if (!entityMetaData.isUsingInternalIds()) {
@@ -381,11 +379,11 @@ static <T> void setGeneratedIdIfNecessary(
381379
* @param <T> The type of the entity
382380
* @return The actual related internal id being used.
383381
*/
384-
static <T> String retrieveOrSetRelatedId(
382+
static <T> Object retrieveOrSetRelatedId(
385383
Neo4jPersistentEntity<?> entityMetadata,
386384
PersistentPropertyAccessor<T> propertyAccessor,
387385
Optional<Entity> databaseEntity,
388-
@Nullable String relatedInternalId
386+
@Nullable Object relatedInternalId
389387
) {
390388
if (!entityMetadata.isUsingInternalIds()) {
391389
return Objects.requireNonNull(relatedInternalId);
@@ -403,7 +401,7 @@ static <T> String retrieveOrSetRelatedId(
403401
}
404402
} else {
405403
if (relatedInternalId == null && current != null) {
406-
relatedInternalId = (String) current;
404+
relatedInternalId = current;
407405
} else if (current == null) {
408406
propertyAccessor.setProperty(requiredIdProperty, relatedInternalId);
409407
}
@@ -415,8 +413,8 @@ static <T> String retrieveOrSetRelatedId(
415413
* Checks if the renderer is configured in such a way that it will use element id or apply toString(id(n)) workaround.
416414
* @return {@literal true} if renderer will use elementId
417415
*/
418-
static boolean rendererCanUseElementIdIfPresent(Renderer renderer) {
419-
return renderer.render(Cypher.returning(Functions.elementId(Cypher.anyNode("n"))).build())
416+
static boolean rendererCanUseElementIdIfPresent(Renderer renderer, Neo4jPersistentEntity<?> targetEntity) {
417+
return !targetEntity.isUsingDeprecatedInternalId() && targetEntity.isUsingInternalIds() && renderer.render(Cypher.returning(Functions.elementId(Cypher.anyNode("n"))).build())
420418
.equals("RETURN elementId(n)");
421419
}
422420

‎src/main/java/org/springframework/data/neo4j/core/mapping/CypherGenerator.java‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -430,13 +430,14 @@ public Statement prepareSaveOfRelationship(Neo4jPersistentEntity<?> neo4jPersist
430430
var startNodeIdFunction = getNodeIdFunction(neo4jPersistentEntity);
431431
return match(startNode)
432432
.where(startNodeIdFunction.apply(startNode).isEqualTo(idParameter))
433-
.match(endNode).where(Functions.elementId(endNode).isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)))
433+
.match(endNode)
434+
.where(getEndNodeIdFunction((Neo4jPersistentEntity<?>) relationship.getTarget()).apply(endNode).isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)))
434435
.merge(relationshipFragment)
435436
.returning(getReturnedIdExpressionsForRelationship(relationship, relationshipFragment))
436437
.build();
437438
}
438439

439-
private static Function<Node, Expression> getNodeIdFunction(Neo4jPersistentEntity<?> entity) {
440+
private static Function<Node, Expression> getNodeIdFunction(@Nullable Neo4jPersistentEntity<?> entity) {
440441

441442
Function<Node, Expression> startNodeIdFunction;
442443
var idProperty = entity.getRequiredIdProperty();
@@ -452,6 +453,20 @@ private static Function<Node, Expression> getNodeIdFunction(Neo4jPersistentEntit
452453
return startNodeIdFunction;
453454
}
454455

456+
private static Function<Node, Expression> getEndNodeIdFunction(@Nullable Neo4jPersistentEntity<?> entity) {
457+
458+
Function<Node, Expression> startNodeIdFunction;
459+
if (entity == null) {
460+
return Functions::elementId;
461+
}
462+
if (!entity.isUsingDeprecatedInternalId() && entity.isUsingInternalIds()) {
463+
startNodeIdFunction = Functions::elementId;
464+
} else {
465+
startNodeIdFunction = Functions::id;
466+
}
467+
return startNodeIdFunction;
468+
}
469+
455470
private static Function<Relationship, Expression> getRelationshipIdFunction(RelationshipDescription relationshipDescription) {
456471

457472
Function<Relationship, Expression> result = Functions::elementId;
@@ -488,7 +503,8 @@ public Statement prepareSaveOfRelationships(Neo4jPersistentEntity<?> neo4jPersis
488503
.with(row)
489504
.match(startNode)
490505
.where(getNodeIdFunction(neo4jPersistentEntity).apply(startNode).isEqualTo(idProperty))
491-
.match(endNode).where(Functions.elementId(endNode).isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME)))
506+
.match(endNode)
507+
.where(getEndNodeIdFunction((Neo4jPersistentEntity<?>) relationship.getTarget()).apply(endNode).isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME)))
492508
.merge(relationshipFragment)
493509
.returning(getReturnedIdExpressionsForRelationship(relationship, relationshipFragment))
494510
.build();
@@ -521,7 +537,8 @@ public Statement prepareSaveOfRelationshipWithProperties(Neo4jPersistentEntity<?
521537

522538
StatementBuilder.OngoingReadingWithWhere startAndEndNodeMatch = match(startNode)
523539
.where(nodeIdFunction.apply(startNode).isEqualTo(idParameter))
524-
.match(endNode).where(Functions.elementId(endNode).isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)));
540+
.match(endNode)
541+
.where(getEndNodeIdFunction((Neo4jPersistentEntity<?>) relationship.getTarget()).apply(endNode).isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)));
525542

526543
StatementBuilder.ExposesSet createOrMatch = isNew
527544
? startAndEndNodeMatch.create(relationshipFragment)
@@ -567,7 +584,7 @@ public Statement prepareUpdateOfRelationshipsWithProperties(Neo4jPersistentEntit
567584
.match(startNode)
568585
.where(nodeIdFunction.apply(startNode).isEqualTo(idProperty))
569586
.match(endNode)
570-
.where(endNode.elementId().isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME)))
587+
.where(getEndNodeIdFunction((Neo4jPersistentEntity<?>) relationship.getTarget()).apply(endNode).isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME)))
571588
.create(relationshipFragment)
572589
.mutate(RELATIONSHIP_NAME, relationshipProperties)
573590
.returning(getReturnedIdExpressionsForRelationship(relationship, relationshipFragment))

‎src/main/java/org/springframework/data/neo4j/core/mapping/NestedRelationshipProcessingStateMachine.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public enum ProcessState {
6464
* A map pointing from a processed object to the internal id.
6565
* This will be useful during the persistence to avoid another DB network round-trip.
6666
*/
67-
private final Map<Integer, String> processedObjectsIds = new HashMap<>();
67+
private final Map<Integer, Object> processedObjectsIds = new HashMap<>();
6868

6969
public NestedRelationshipProcessingStateMachine(final Neo4jMappingContext mappingContext) {
7070

@@ -73,7 +73,7 @@ public NestedRelationshipProcessingStateMachine(final Neo4jMappingContext mappin
7373
this.mappingContext = mappingContext;
7474
}
7575

76-
public NestedRelationshipProcessingStateMachine(final Neo4jMappingContext mappingContext, Object initialObject, String elementId) {
76+
public NestedRelationshipProcessingStateMachine(final Neo4jMappingContext mappingContext, Object initialObject, Object elementId) {
7777
this(mappingContext);
7878

7979
Assert.notNull(initialObject, "Initial object must not be null");
@@ -143,7 +143,7 @@ public void markRelationshipAsProcessed(Object fromId, @Nullable RelationshipDes
143143
* @param valueToStore If not {@literal null}, all non-null values will be marked as processed
144144
* @param elementId The internal id of the value processed
145145
*/
146-
public void markEntityAsProcessed(Object valueToStore, String elementId) {
146+
public void markEntityAsProcessed(Object valueToStore, Object elementId) {
147147

148148
final long stamp = lock.writeLock();
149149
try {
@@ -154,7 +154,7 @@ public void markEntityAsProcessed(Object valueToStore, String elementId) {
154154
}
155155
}
156156

157-
private void doMarkValueAsProcessed(Object valueToStore, String elementId) {
157+
private void doMarkValueAsProcessed(Object valueToStore, Object elementId) {
158158

159159
Object value = extractRelatedValueFromRelationshipProperties(valueToStore);
160160
storeHashedVersionInProcessedObjectsIds(valueToStore, elementId);
@@ -191,7 +191,7 @@ public boolean hasProcessedValue(Object value) {
191191
.findAny();
192192
if (alreadyProcessedObject.isPresent()) { // Skip the show the next time around.
193193
processed = true;
194-
String internalId = getObjectId(alreadyProcessedObject.get());
194+
Object internalId = getObjectId(alreadyProcessedObject.get());
195195
if (internalId != null) {
196196
stamp = lock.tryConvertToWriteLock(stamp);
197197
doMarkValueAsProcessed(valueToCheck, internalId);
@@ -239,11 +239,11 @@ public void markAsAliased(Object aliasEntity, Object entityOrId) {
239239
* @return The objects id
240240
*/
241241
@Nullable
242-
public String getObjectId(Object object) {
242+
public Object getObjectId(Object object) {
243243
final long stamp = lock.readLock();
244244
try {
245245
Object valueToCheck = extractRelatedValueFromRelationshipProperties(object);
246-
String possibleId = getProcessedObjectIds(valueToCheck);
246+
Object possibleId = getProcessedObjectIds(valueToCheck);
247247
return possibleId != null ? possibleId : getProcessedObjectIds(getProcessedAs(valueToCheck));
248248
} finally {
249249
lock.unlock(stamp);
@@ -261,7 +261,7 @@ public Object getProcessedAs(Object entity) {
261261
}
262262

263263
@Nullable
264-
private String getProcessedObjectIds(@Nullable Object entity) {
264+
private Object getProcessedObjectIds(@Nullable Object entity) {
265265
if (entity == null) {
266266
return null;
267267
}
@@ -282,7 +282,7 @@ private Object extractRelatedValueFromRelationshipProperties(Object valueToStore
282282
/*
283283
* Convenience wrapper functions to avoid exposing the System.identityHashCode "everywhere" in this class.
284284
*/
285-
private void storeHashedVersionInProcessedObjectsIds(Object initialObject, String elementId) {
285+
private void storeHashedVersionInProcessedObjectsIds(Object initialObject, Object elementId) {
286286
processedObjectsIds.put(System.identityHashCode(initialObject), elementId);
287287
}
288288

0 commit comments

Comments
 (0)