Fix AIOOB edge case in SparseLongArray

Cherry-pick #4037

Co-authored-by: Martin Junghanns <[email protected]>
Co-authored-by: Florentin Dörre <[email protected]>
Co-authored-by: Adam Schill Collberg <[email protected]>

Author: 4 people

alpha/alpha-proc/src/main/java/org/neo4j/gds/traverse/TraverseProc.java

@@ -98,7 +98,7 @@ protected Traverse build(
                 // target node given; terminate if target is reached
                 if (!configuration.targetNodes().isEmpty()) {
                     List<Long> mappedTargets = configuration.targetNodes().stream()
-                        .map(graph::toMappedNodeId)
+                        .map(graph::safeToMappedNodeId)
                         .collect(Collectors.toList());
                     exitFunction = (s, t, w) -> mappedTargets.contains(t) ? Traverse.ExitPredicate.Result.BREAK : Traverse.ExitPredicate.Result.FOLLOW;
                     aggregatorFunction = (s, t, w) -> .0;
@@ -113,7 +113,7 @@ protected Traverse build(
                 }
 
                 validateStartNode(configuration.startNode(), graph);
-                configuration.targetNodes().stream().forEach(neoId -> validateEndNode(neoId, graph));
+                configuration.targetNodes().forEach(neoId -> validateEndNode(neoId, graph));
 
                 var mappedStartNodeId = graph.toMappedNodeId(configuration.startNode());
 

alpha/alpha-proc/src/main/java/org/neo4j/gds/utils/InputNodeValidator.java

@@ -20,21 +20,22 @@
 package org.neo4j.gds.utils;
 
 import org.neo4j.gds.api.Graph;
+import org.neo4j.gds.api.IdMapping;
 
 import static org.neo4j.gds.utils.StringFormatting.formatWithLocale;
 
 public final class InputNodeValidator {
 
     public static void validateStartNode(long nodeId, Graph graph) throws IllegalArgumentException {
-        validateNodeIsLoaded(nodeId, graph.toMappedNodeId(nodeId), "startNode");
+        validateNodeIsLoaded(nodeId, graph.safeToMappedNodeId(nodeId), "startNode");
     }
 
     public static void validateEndNode(long nodeId, Graph graph) throws IllegalArgumentException {
-        validateNodeIsLoaded(nodeId, graph.toMappedNodeId(nodeId), "endNode");
+        validateNodeIsLoaded(nodeId, graph.safeToMappedNodeId(nodeId), "endNode");
     }
 
     private static void validateNodeIsLoaded(long nodeId, long mappedId, String nodeDescription) throws IllegalArgumentException {
-        if (mappedId == -1) {
+        if (mappedId == IdMapping.NOT_FOUND) {
             throw new IllegalArgumentException(formatWithLocale(
                 "%s with id %d was not loaded",
                 nodeDescription,

alpha/alpha-proc/src/main/java/org/neo4j/gds/walking/RandomWalkProc.java

@@ -154,15 +154,15 @@ private IntStream idStream(Object start, Graph graph, int limit) {
                     return cursor.nodeReference();
                 });
             }
-            return ids.map(graph::toMappedNodeId).mapToInt(Math::toIntExact).onClose(cursor::close);
+            return ids.map(graph::safeToMappedNodeId).mapToInt(Math::toIntExact).onClose(cursor::close);
         } else if (start instanceof Collection) {
             return ((Collection<?>) start)
                 .stream()
                 .mapToLong(e -> ((Number) e).longValue())
-                .map(graph::toMappedNodeId)
+                .map(graph::safeToMappedNodeId)
                 .mapToInt(Math::toIntExact);
         } else if (start instanceof Number) {
-            return LongStream.of(((Number) start).longValue()).map(graph::toMappedNodeId).mapToInt(Math::toIntExact);
+            return LongStream.of(((Number) start).longValue()).map(graph::safeToMappedNodeId).mapToInt(Math::toIntExact);
         } else {
             if (nodeCount < limit) {
                 return IntStream.range(0, nodeCount).limit(limit);

core/src/main/java/org/neo4j/gds/api/IdMapping.java

@@ -30,11 +30,27 @@ public interface IdMapping {
      */
     long START_NODE_ID = 0;
 
+    /**
+     * Defines the value for unmapped ids
+     */
+    long NOT_FOUND = -1;
+
     /**
      * Map original nodeId to inner nodeId
+     *
+     * @param nodeId must be smaller or equal to the id returned by {@link IdMapping#highestNeoId}
      */
     long toMappedNodeId(long nodeId);
 
+    /**
+     * Map original nodeId to inner nodeId
+     *
+     * Returns org.neo4j.gds.api.IdMapping#NOT_FOUND if the nodeId is not mapped.
+     */
+    default long safeToMappedNodeId(long nodeId) {
+        return highestNeoId() < nodeId ? NOT_FOUND : toMappedNodeId(nodeId);
+    }
+
     /**
      * Map inner nodeId back to original nodeId
      */

core/src/main/java/org/neo4j/gds/api/NodeMapping.java

@@ -26,8 +26,6 @@
 
 public interface NodeMapping extends IdMapping, NodeIterator, BatchNodeIterable {
 
-    long NOT_FOUND = -1;
-
     Set<NodeLabel> nodeLabels(long nodeId);
 
     void forEachNodeLabel(long nodeId, NodeLabelConsumer consumer);

core/src/main/java/org/neo4j/gds/core/utils/paged/HugeSparseLongArray.java

@@ -19,7 +19,7 @@
  */
 package org.neo4j.gds.core.utils.paged;
 
-import org.neo4j.gds.api.NodeMapping;
+import org.neo4j.gds.api.IdMapping;
 import org.neo4j.gds.core.utils.mem.AllocationTracker;
 import org.neo4j.gds.core.utils.mem.MemoryRange;
 import org.neo4j.gds.core.utils.mem.MemoryUsage;
@@ -35,7 +35,7 @@
 
 public final class HugeSparseLongArray {
 
-    private static final long NOT_FOUND = NodeMapping.NOT_FOUND;
+    private static final long NOT_FOUND = IdMapping.NOT_FOUND;
 
     private static final int PAGE_SHIFT = 12;
     private static final int PAGE_SIZE = 1 << PAGE_SHIFT;

core/src/main/java/org/neo4j/gds/core/utils/paged/SparseLongArray.java

@@ -21,7 +21,7 @@
 
 import com.carrotsearch.hppc.sorting.IndirectSort;
 import org.jetbrains.annotations.TestOnly;
-import org.neo4j.gds.api.NodeMapping;
+import org.neo4j.gds.api.IdMapping;
 import org.neo4j.gds.core.utils.ArrayLayout;
 import org.neo4j.gds.core.utils.AscendingLongComparator;
 import org.neo4j.gds.core.utils.BitUtil;
@@ -37,7 +37,7 @@
 
 public final class SparseLongArray {
 
-    public static final long NOT_FOUND = NodeMapping.NOT_FOUND;
+    public static final long NOT_FOUND = IdMapping.NOT_FOUND;
 
     public static final int BLOCK_SIZE = 64;
     public static final int SUPER_BLOCK_SIZE = BLOCK_SIZE * Long.SIZE;
@@ -125,6 +125,10 @@ public long highestNeoId() {
         return highestNeoId;
     }
 
+    /**
+     *
+     * @param originalId must be smaller or equal to highestNeoId
+     */
     public long toMappedNodeId(long originalId) {
         var page = pageId(originalId);
         var indexInPage = indexInPage(originalId);

proc/common/src/main/java/org/neo4j/gds/GraphStoreValidation.java

@@ -20,7 +20,7 @@
 package org.neo4j.gds;
 
 import org.neo4j.gds.api.GraphStore;
-import org.neo4j.gds.api.NodeMapping;
+import org.neo4j.gds.api.IdMapping;
 import org.neo4j.gds.config.AlgoBaseConfig;
 import org.neo4j.gds.config.ConfigurableSeedConfig;
 import org.neo4j.gds.config.FeaturePropertiesConfig;
@@ -185,7 +185,7 @@ private static void validateFeaturesProperties(GraphStore graphStore, FeaturePro
     private static void validateSourceNode(GraphStore graphStore, SourceNodeConfig config) {
         var sourceNodeId = config.sourceNode();
 
-        if (graphStore.nodes().toMappedNodeId(sourceNodeId) == NodeMapping.NOT_FOUND) {
+        if (graphStore.nodes().safeToMappedNodeId(sourceNodeId) == IdMapping.NOT_FOUND) {
             throw new IllegalArgumentException(formatWithLocale(
                 "Source node does not exist in the in-memory graph: `%d`",
                 sourceNodeId
@@ -196,7 +196,7 @@ private static void validateSourceNode(GraphStore graphStore, SourceNodeConfig c
     private static void validateSourceNodes(GraphStore graphStore, SourceNodesConfig config) {
         var nodeMapping = graphStore.nodes();
         var missingNodes = config.sourceNodes().stream()
-            .filter(nodeId -> nodeMapping.toMappedNodeId(nodeId) == NodeMapping.NOT_FOUND)
+            .filter(nodeId -> nodeMapping.safeToMappedNodeId(nodeId) == IdMapping.NOT_FOUND)
             .map(Object::toString)
             .collect(Collectors.toList());
 
@@ -211,7 +211,7 @@ private static void validateSourceNodes(GraphStore graphStore, SourceNodesConfig
     private static void validateTargetNode(GraphStore graphStore, TargetNodeConfig config) {
         var targetNodeId = config.targetNode();
 
-        if (graphStore.nodes().toMappedNodeId(targetNodeId) == NodeMapping.NOT_FOUND) {
+        if (graphStore.nodes().safeToMappedNodeId(targetNodeId) == IdMapping.NOT_FOUND) {
             throw new IllegalArgumentException(formatWithLocale(
                 "Target node does not exist in the in-memory graph: `%d`",
                 targetNodeId

proc/common/src/main/java/org/neo4j/gds/functions/NodePropertyFunc.java

@@ -86,7 +86,7 @@ public Object nodeProperty(
             }
         }
 
-        long internalId = graphStore.nodes().toMappedNodeId(nodeId.longValue());
+        long internalId = graphStore.nodes().safeToMappedNodeId(nodeId.longValue());
 
         if (internalId == -1) {
             throw new IllegalArgumentException(formatWithLocale("Node id %d does not exist.", nodeId.longValue()));