Merge pull request #8183 from knutwalker/cogds-off-by-1

Fix off-by-one for relationship count vs max id
neo4j · Sep 28, 2023 · 30e219c · 30e219c
2 parents 312758e + 28546cf
commit 30e219c
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 11 deletions.
diff --git a/...-engine-adapter/src/main/java/org/neo4j/gds/storageengine/InMemoryRelationshipCursor.java b/...-engine-adapter/src/main/java/org/neo4j/gds/storageengine/InMemoryRelationshipCursor.java
@@ -52,7 +52,7 @@ public abstract class InMemoryRelationshipCursor
     private final List<PropertyCursor[]> propertyCursorCache;
     private MutableDoubleList propertyValuesCache;
 
-    protected long maxRelationshipId;
+    protected long totalRelationshipCount;
 
     protected long sourceId;
     protected long targetId;
@@ -73,7 +73,7 @@ public InMemoryRelationshipCursor(CypherGraphStore graphStore, TokenHolders toke
         this.propertyCursorCache = new ArrayList<>();
         this.propertyValuesCache = new DoubleArrayList();
 
-        this.maxRelationshipId = 0;
+        this.totalRelationshipCount = 0;
         this.graphStore.relationshipIds().registerUpdateListener(this);
     }
 
@@ -138,7 +138,7 @@ public void onRelationshipIdsAdded(RelationshipIds.RelationshipIdContext relatio
         );
         var newSize = this.propertyCursorCache.stream().mapToInt(cursor -> cursor.length).max().orElse(0);
         this.propertyValuesCache = new DoubleArrayList(new double[newSize]);
-        this.maxRelationshipId += relationshipIdContext.relationshipCount();
+        this.totalRelationshipCount += relationshipIdContext.relationshipCount();
     }
 
     @Override

diff --git a/...rc/main/java/org/neo4j/internal/recordstorage/AbstractInMemoryRelationshipScanCursor.java b/...rc/main/java/org/neo4j/internal/recordstorage/AbstractInMemoryRelationshipScanCursor.java
@@ -30,7 +30,7 @@
 
 public abstract class AbstractInMemoryRelationshipScanCursor extends InMemoryRelationshipCursor implements StorageRelationshipScanCursor {
 
-    private long highMark;
+    private long highMarkExclusive;
 
     public AbstractInMemoryRelationshipScanCursor(CypherGraphStore graphStore, TokenHolders tokenHolders) {
         super(graphStore, tokenHolders);
@@ -41,14 +41,14 @@ public void scan() {
         reset();
         this.sourceId = 0;
         this.selection = RelationshipSelection.ALL_RELATIONSHIPS;
-        this.highMark = maxRelationshipId;
+        this.highMarkExclusive = totalRelationshipCount;
     }
 
     @Override
     public void single(long reference) {
         reset();
         setId(reference - 1);
-        this.highMark = reference;
+        this.highMarkExclusive = reference + 1;
         this.selection = RelationshipSelection.ALL_RELATIONSHIPS;
 
         initializeForRelationshipReference(reference);
@@ -57,7 +57,7 @@ public void single(long reference) {
     @Override
     public boolean next() {
         if (super.next()) {
-            return getId() <= highMark;
+            return getId() < highMarkExclusive;
         } else {
             this.sourceId++;
             if (this.sourceId >= graphStore.nodeCount()) {
@@ -75,16 +75,20 @@ public boolean scanBatch(AllRelationshipsScan scan, int sizeHint) {
             reset();
         }
 
-        highMark = maxRelationshipId;
+        highMarkExclusive = totalRelationshipCount;
         return ((BaseRecordScan<AbstractInMemoryRelationshipScanCursor>) scan).scanBatch(sizeHint, this);
     }
 
-    public boolean scanRange(long start, long stop) {
+    public boolean scanRange(long start, long stopInclusive) {
         reset();
         this.selection = RelationshipSelection.ALL_RELATIONSHIPS;
-        highMark = min(stop, maxRelationshipId);
+        if (stopInclusive < Long.MAX_VALUE) {
+            highMarkExclusive = min(stopInclusive + 1, totalRelationshipCount);
+        } else {
+            highMarkExclusive = totalRelationshipCount;
+        }
 
-        if (start > maxRelationshipId) {
+        if (start >= highMarkExclusive) {
             return false;
         }
         initializeForRelationshipReference(start);