Skip to content

Commit 9964547

Browse files
authored
Refactor add methods in spatial index classes for improved efficiency and clarity (#441)
This commit refactors the `add` methods across spatial index classes to use batch operations, deduplicate inputs, and centralize single-node insertion via a default interface method. It also fixes exceptions when deleting nodes attached to multiple layers by properly handling incoming relationships. - Introduced `add(Transaction, List<Node>)` as primary insertion endpoint and removed redundant single-node overrides. - Enhanced bulk insertion in `RTreeIndex` with deduplication via `LinkedHashSet` and updated rebuild logic. - Fixed relationship cleanup in `mergeTwoSubtrees` and `onIndexReference` to avoid orphaned or missing relationships.
1 parent e73f56b commit 9964547

File tree

3 files changed

+36
-31
lines changed

3 files changed

+36
-31
lines changed

server-plugin/src/main/java/org/neo4j/gis/spatial/index/ExplicitIndexBackedPointIndex.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,17 +79,12 @@ public SearchRecords search(Transaction tx, SearchFilter filter) {
7979
return new SearchRecords(layer, searchIndex(tx, filter));
8080
}
8181

82-
@Override
83-
public void add(Transaction tx, Node geomNode) {
84-
index.add(geomNode, getIndexValueFor(tx, geomNode));
85-
}
86-
8782
protected abstract E getIndexValueFor(Transaction tx, Node geomNode);
8883

8984
@Override
9085
public void add(Transaction tx, List<Node> geomNodes) {
9186
for (Node node : geomNodes) {
92-
add(tx, node);
87+
index.add(node, getIndexValueFor(tx, node));
9388
}
9489
}
9590

server-plugin/src/main/java/org/neo4j/gis/spatial/index/SpatialIndexWriter.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@
2727

2828
public interface SpatialIndexWriter extends SpatialIndexReader {
2929

30-
void add(Transaction tx, Node geomNode);
30+
default void add(Transaction tx, Node geomNode) {
31+
add(tx, List.of(geomNode));
32+
}
3133

3234
void add(Transaction tx, List<Node> geomNodes);
3335

server-plugin/src/main/java/org/neo4j/gis/spatial/rtree/RTreeIndex.java

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@
2525
import java.util.HashMap;
2626
import java.util.HashSet;
2727
import java.util.Iterator;
28+
import java.util.LinkedHashSet;
2829
import java.util.List;
2930
import java.util.Map;
31+
import java.util.Set;
3032
import java.util.stream.Collectors;
3133
import java.util.stream.StreamSupport;
3234
import javax.annotation.Nonnull;
@@ -170,15 +172,6 @@ public void configure(Map<String, Object> config) {
170172
});
171173
}
172174

173-
@Override
174-
public void add(Transaction tx, Node geomNode) {
175-
// initialize the search with root
176-
Node parent = getIndexRoot(tx);
177-
addBelow(tx, parent, geomNode);
178-
countSaved = false;
179-
totalGeometryCount++;
180-
}
181-
182175
/**
183176
* This method will add the node somewhere below the parent.
184177
*/
@@ -229,29 +222,34 @@ private void insertIndexNodeOnParent(Transaction tx, Node parent, Node child) {
229222
*/
230223
@Override
231224
public void add(Transaction tx, List<Node> geomNodes) {
225+
Node indexRoot = getIndexRoot(tx);
232226

233227
//If the insertion is large relative to the size of the tree, simply rebuild the whole tree.
234-
if (geomNodes.size() > totalGeometryCount * 0.4) {
235-
List<Node> nodesToAdd = new ArrayList<>(geomNodes.size() + totalGeometryCount);
236-
for (Node n : getAllIndexedNodes(tx)) {
237-
nodesToAdd.add(n);
238-
}
239-
nodesToAdd.addAll(geomNodes);
240-
detachGeometryNodes(tx, false, getIndexRoot(tx), new NullListener());
241-
deleteTreeBelow(getIndexRoot(tx));
242-
buildRtreeFromScratch(tx, getIndexRoot(tx), decodeGeometryNodeEnvelopes(nodesToAdd), 0.7);
228+
if (totalGeometryCount > 0
229+
&& geomNodes.size() > 1
230+
&& geomNodes.size() > totalGeometryCount * 0.4
231+
) {
232+
Set<Node> uniqueNodes
233+
= new LinkedHashSet<>(geomNodes.size() + totalGeometryCount);
234+
getAllIndexedNodes(tx).forEach(uniqueNodes::add);
235+
uniqueNodes.addAll(geomNodes);
236+
237+
List<Node> nodesToAdd = new ArrayList<>(uniqueNodes);
238+
detachGeometryNodes(tx, false, indexRoot, new NullListener());
239+
deleteTreeBelow(indexRoot);
240+
buildRtreeFromScratch(tx, indexRoot, decodeGeometryNodeEnvelopes(nodesToAdd), 0.7);
243241
countSaved = false;
244242
totalGeometryCount = nodesToAdd.size();
245243
monitor.addNbrRebuilt(this, tx);
246244
} else {
247245

248-
List<NodeWithEnvelope> outliers = bulkInsertion(tx, getIndexRoot(tx), getHeight(getIndexRoot(tx), 0),
246+
List<NodeWithEnvelope> outliers = bulkInsertion(tx, indexRoot, getHeight(indexRoot, 0),
249247
decodeGeometryNodeEnvelopes(geomNodes), 0.7);
250-
countSaved = false;
251-
totalGeometryCount = totalGeometryCount + (geomNodes.size() - outliers.size());
252248
for (NodeWithEnvelope n : outliers) {
253-
add(tx, n.node);
249+
addBelow(tx, indexRoot, n.node);
254250
}
251+
totalGeometryCount += geomNodes.size();
252+
countSaved = false;
255253
}
256254
}
257255

@@ -499,7 +497,11 @@ protected void mergeTwoSubtrees(Transaction tx, NodeWithEnvelope parent, List<No
499497
disconnectedChildren.forEach(t -> t.node.delete());
500498

501499
for (NodeWithEnvelope n : right) {
502-
n.node.getSingleRelationship(RTreeRelationshipTypes.RTREE_CHILD, Direction.INCOMING);
500+
Relationship previousParent = n.node.getSingleRelationship(RTreeRelationshipTypes.RTREE_CHILD,
501+
Direction.INCOMING);
502+
if (previousParent != null) {
503+
previousParent.delete();
504+
}
503505
parent.node.createRelationshipTo(n.node, RTreeRelationshipTypes.RTREE_CHILD);
504506
parent.envelope.expandToInclude(n.envelope);
505507
}
@@ -681,7 +683,13 @@ public boolean needsToVisit(Envelope indexNodeEnvelope) {
681683

682684
@Override
683685
public void onIndexReference(Node geomNode) {
684-
geomNode.getSingleRelationship(referenceRelationshipType, Direction.INCOMING).delete();
686+
try (var relationships = geomNode.getRelationships(Direction.INCOMING, referenceRelationshipType)) {
687+
for (Relationship rel : relationships) {
688+
if (rel.getStartNode().equals(indexRoot)) {
689+
rel.delete();
690+
}
691+
}
692+
}
685693
if (deleteGeomNodes) {
686694
deleteNode(geomNode);
687695
}

0 commit comments

Comments
 (0)