Exhibit issue and propose fix

neo4j · Oct 11, 2023 · e81b648 · e81b648
1 parent ddd841c
commit e81b648
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 8 deletions.
diff --git a/core/src/main/java/org/neo4j/gds/core/huge/CompositeAdjacencyCursor.java b/core/src/main/java/org/neo4j/gds/core/huge/CompositeAdjacencyCursor.java
@@ -51,9 +51,10 @@ public List<AdjacencyCursor> cursors() {
         return cursors;
     }
 
-    void updateCursorsQueue() {
+    void reinitializeCursors() {
         cursorQueue.clear();
-        cursorQueue.addAll(cursors);
+        initializeQueue();
+
     }
     @Override
     public int size() {

diff --git a/core/src/main/java/org/neo4j/gds/core/huge/CompositeAdjacencyList.java b/core/src/main/java/org/neo4j/gds/core/huge/CompositeAdjacencyList.java
@@ -126,19 +126,19 @@ public CompositeAdjacencyCursor adjacencyCursor(@Nullable AdjacencyCursor reuse,
         if (reuse instanceof CompositeAdjacencyCursor) {
             var compositeReuse = (CompositeAdjacencyCursor) reuse;
             var iter = compositeReuse.cursors().listIterator();
-            boolean cursorsAreUpdated = false;
+
             while (iter.hasNext()) {
                 var index = iter.nextIndex();
                 var cursor = iter.next();
                 var newCursor = adjacencyLists.get(index).adjacencyCursor(cursor, node, fallbackValue);
                 if (newCursor != cursor) {
                     iter.set(adjacencyCursorWrapperFactory.create(newCursor));
-                    cursorsAreUpdated = true;
                 }
             }
-            if (cursorsAreUpdated) {
-                compositeReuse.updateCursorsQueue();
-            }
+            //we must update all the cursors all the time because old cursor might be further ahead instead of start
+            //then, object equality can be then the same, but in reality not so much
+            compositeReuse.reinitializeCursors();
+
             return compositeReuse;
         }
         return adjacencyCursor(node, fallbackValue);

diff --git a/core/src/test/java/org/neo4j/gds/core/huge/CompositeAdjacencyListTest.java b/core/src/test/java/org/neo4j/gds/core/huge/CompositeAdjacencyListTest.java
@@ -20,11 +20,12 @@
 package org.neo4j.gds.core.huge;
 
 import org.junit.jupiter.api.Test;
+import org.neo4j.gds.api.Graph;
 import org.neo4j.gds.extension.GdlExtension;
 import org.neo4j.gds.extension.GdlGraph;
 import org.neo4j.gds.extension.Inject;
-import org.neo4j.gds.api.Graph;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 @GdlExtension
@@ -47,4 +48,26 @@ void shouldIgnoreInputDegreeForCursor() {
         var cursor = adjacencyList.adjacencyCursor(0);
         assertEquals(2, cursor.remaining());
     }
+
+    @Test
+    void shouldReuseCursor() {
+
+        var adjacencyList = ((UnionGraph) graph).relationshipTopology();
+        var cursor = adjacencyList.adjacencyCursor(null, 0);
+        assertThat(cursor.peekVLong()).isEqualTo(1L);
+        cursor.nextVLong();
+
+        cursor = adjacencyList.adjacencyCursor(cursor, 0);
+        assertThat(cursor.peekVLong()).isEqualTo(1L);
+
+
+        cursor.nextVLong();
+        cursor.nextVLong();
+
+        cursor = adjacencyList.adjacencyCursor(cursor, 0);
+        assertThat(cursor.hasNextVLong()).isTrue();
+
+
+    }
+
 }