Fix VertexIDAssigner bug when ids-flush is disabled

When ids-flush is disabled, ids are re-assigned even if elements already have ids, e.g. update of existing elements. This commit fixes this bug and does not re-assign ids. Signed-off-by: Boxuan Li <[email protected]>
li-boxuan · Nov 12, 2020 · b6575f2 · b6575f2
1 parent 39ac149
commit b6575f2
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 1 deletion.
diff --git a/...graph-core/src/main/java/org/janusgraph/graphdb/database/idassigner/VertexIDAssigner.java b/...graph-core/src/main/java/org/janusgraph/graphdb/database/idassigner/VertexIDAssigner.java
@@ -297,7 +297,7 @@ private long getPartitionID(final InternalVertex v) {
 
     private void assignID(final InternalElement element, final long partitionIDl, final IDManager.VertexIDType userVertexIDType) {
         Preconditions.checkNotNull(element);
-        Preconditions.checkArgument(!element.hasId());
+        if (element.hasId()) return;
         Preconditions.checkArgument((element instanceof JanusGraphRelation) ^ (userVertexIDType!=null));
         Preconditions.checkArgument(partitionIDl >= 0 && partitionIDl < partitionIdBound, partitionIDl);
         final int partitionID = (int) partitionIDl;

diff --git a/janusgraph-test/src/test/java/org/janusgraph/graphdb/idmanagement/VertexIDAssignerTest.java b/janusgraph-test/src/test/java/org/janusgraph/graphdb/idmanagement/VertexIDAssignerTest.java
@@ -16,6 +16,7 @@
 
 import com.carrotsearch.hppc.LongHashSet;
 import com.carrotsearch.hppc.LongSet;
+import org.apache.tinkerpop.gremlin.structure.Edge;
 import org.apache.tinkerpop.gremlin.structure.T;
 import org.janusgraph.core.JanusGraphFactory;
 import org.janusgraph.core.JanusGraph;
@@ -31,6 +32,7 @@
 import org.janusgraph.graphdb.internal.InternalRelation;
 import org.janusgraph.graphdb.internal.InternalVertex;
 
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -111,6 +113,27 @@ private JanusGraph getInMemoryGraph(boolean allowSettingVertexId, boolean idsFlu
         return JanusGraphFactory.open(config);
     }
 
+    @Test
+    public void testDisableIdsFlush() {
+        final JanusGraph graph = getInMemoryGraph(false, false, 2);
+        JanusGraphVertex v1 = graph.addVertex();
+        JanusGraphVertex v2 = graph.addVertex();
+        Edge e = v1.addEdge("knows", v2);
+        e.property("prop", "old");
+        graph.tx().commit();
+        assertEquals("old", graph.traversal().E().next().property("prop").value());
+        assertEquals(1, (long) graph.traversal().E().count().next());
+        Object id = graph.traversal().E().next().id();
+
+        // VertexIDAssigner shouldn't assign a new id if the edge is an existing one
+        e = graph.traversal().E().next();
+        e.property("prop", "new");
+        graph.tx().commit();
+        assertEquals("new", graph.traversal().E().next().property("prop").value());
+        assertEquals(1, (long) graph.traversal().E().count().next());
+        assertEquals(id, graph.traversal().E().next().id());
+    }
+
     @ParameterizedTest
     @MethodSource("configs")
     public void testIDAssignment(VertexIDAssigner idAssigner, long maxIDAssignments, int numPartitionsBits) {