DiffGraph: allow simultaneous edge and node removal (#347)

mpollmeier · web-flow · commit b9e47982de81 · 2025-09-19T17:45:12.000+02:00
Prior to this, if a user tried to delete a node and an adjacent edge at
the same time, we would run into this:
```
scala.MatchError: flatgraph.DiffGraphBuilder$RemoveEdge@7bcbf3a8 (of class flatgraph.DiffGraphBuilder$RemoveEdge)
	at flatgraph.DiffGraphApplier.splitUpdate$$anonfun$2(DiffGraphApplier.scala:228) ~[io.joern.flatgraph-core_3-0.1.25.jar:0.1.25]
```

Which was because the condition was on the `case RemoveEdge` filtered
out that case and there's no fallback `case _` or similar.
This makes it explicit and adds a test.
diff --git a/core/src/main/scala/flatgraph/DiffGraphApplier.scala b/core/src/main/scala/flatgraph/DiffGraphApplier.scala
@@ -196,24 +196,28 @@ private[flatgraph] class DiffGraphApplier(
             new EdgeRepr(inR.src, inR.dst, inR.edgeKind, inR.subSeq, setEdgeProperty.property),
             graph.schema.neighborOffsetArrayIndex(inR.src.nodeKind, Incoming, inR.edgeKind)
           )
-        case edgeDeletion: RemoveEdge
-            if !AccessHelpers.isDeleted(edgeDeletion.edge.src) && !AccessHelpers.isDeleted(edgeDeletion.edge.dst) =>
-          ndiff += 1
+        case edgeDeletion: RemoveEdge =>
+          if (AccessHelpers.isDeleted(edgeDeletion.edge.src) || AccessHelpers.isDeleted(edgeDeletion.edge.dst)) {
+            // the adjacent nodes have already been deleted - nothing more for us to do here
+            // and, importantly, we do not want to fail if that's the case, but simply relax and enjoy
+          } else {
 
-          /** This is the delEdge case. It is massively annoying.
-            *
-            * In order to support edge properties, we need to grab the right edge from e.src->e.dst. If we assume that our graph was built
-            * normally, i.e. edges were sequentially/batched added without the unsafe unidirectional edges, then our graph has the following
-            * invariant: The kth edge connecting A->B corresponds to the kth edge connecting B<-A This sucks big time, because edge removal
-            * is potentially O(N**2). The degenerate behavior occurs when we have ~k edges of the same type starting in src = X or ending in
-            * the same dst = X. Each deletion then costs us ~k, and if we delete all ~k edges we pay ~ k*k.
-            *
-            * But k~N is possible where N is the graph size!
-            */
-          val (outR, inR) = normalizeRepresentation(edgeDeletion.edge)
-          insert(delEdges, outR, graph.schema.neighborOffsetArrayIndex(outR.src.nodeKind, Outgoing, outR.edgeKind))
-          insert(delEdges, inR, graph.schema.neighborOffsetArrayIndex(inR.src.nodeKind, Incoming, inR.edgeKind))
+            ndiff += 1
 
+            /** This is the delEdge case. It is massively annoying.
+              *
+              * In order to support edge properties, we need to grab the right edge from e.src->e.dst. If we assume that our graph was built
+              * normally, i.e. edges were sequentially/batched added without the unsafe unidirectional edges, then our graph has the
+              * following invariant: The kth edge connecting A->B corresponds to the kth edge connecting B<-A This sucks big time, because
+              * edge removal is potentially O(N**2). The degenerate behavior occurs when we have ~k edges of the same type starting in src =
+              * X or ending in the same dst = X. Each deletion then costs us ~k, and if we delete all ~k edges we pay ~ k*k.
+              *
+              * But k~N is possible where N is the graph size!
+              */
+            val (outR, inR) = normalizeRepresentation(edgeDeletion.edge)
+            insert(delEdges, outR, graph.schema.neighborOffsetArrayIndex(outR.src.nodeKind, Outgoing, outR.edgeKind))
+            insert(delEdges, inR, graph.schema.neighborOffsetArrayIndex(inR.src.nodeKind, Incoming, inR.edgeKind))
+          }
         case setNodeProperty: SetNodeProperty if !AccessHelpers.isDeleted(setNodeProperty.node) =>
           ndiff += 1
           val iter = setNodeProperty.property match {
diff --git a/core/src/test/scala/flatgraph/GraphTests.scala b/core/src/test/scala/flatgraph/GraphTests.scala
@@ -524,6 +524,72 @@ class GraphTests extends AnyWordSpec with Matchers {
     testSerialization(g)
   }
 
+  "permit deleting an edge together with it's node" in {
+    // if a node get's deleted we automatically delete the adjacent edges
+    // but if a user explicitly removes the edge (additionally) we do not want to fail hard for that...
+
+    var g = mkGraph()
+    debugDump(g) shouldBe
+      """#Node numbers (kindId, nnodes) (0: 4), total 4
+        |Node kind 0. (eid, nEdgesOut, nEdgesIn): (0, 7 [dense], 7 [dense]),
+        |   V0_0   [0] -> V0_2, V0_1, V0_3, V0_2, V0_1
+        |   V0_1   [0] <- V0_0, V0_3, V0_0
+        |   V0_2   [0] <- V0_0, V0_0, V0_3
+        |   V0_3   [0] -> V0_1, V0_2
+        |   V0_3   [0] <- V0_0
+        |""".stripMargin
+
+    val nodeToDelete = g.node(0, 0)
+    val edgeToDelete = Accessors.getEdgesOut(nodeToDelete, 0)(0)
+    DiffGraphApplier.applyDiff(
+      g,
+      new DiffGraphBuilder(schema)
+        .removeNode(nodeToDelete)
+        .removeEdge(edgeToDelete)
+    )
+
+    debugDump(g) shouldBe
+      """#Node numbers (kindId, nnodes) (0: 4), total 4
+        |Node kind 0. (eid, nEdgesOut, nEdgesIn): (0, 2 [dense], 2 [dense]),
+        |   V0_1   [0] <- V0_3
+        |   V0_2   [0] <- V0_3
+        |   V0_3   [0] -> V0_1, V0_2
+        |""".stripMargin
+
+    g = mkGraph()
+    DiffGraphApplier.applyDiff(
+      g,
+      new DiffGraphBuilder(schema)
+        .removeNode(g.node(0, 1))
+    )
+    debugDump(g) shouldBe
+      """#Node numbers (kindId, nnodes) (0: 4), total 4
+        |Node kind 0. (eid, nEdgesOut, nEdgesIn): (0, 4 [dense], 4 [dense]),
+        |   V0_0   [0] -> V0_2, V0_3, V0_2
+        |   V0_2   [0] <- V0_0, V0_0, V0_3
+        |   V0_3   [0] -> V0_2
+        |   V0_3   [0] <- V0_0
+        |""".stripMargin
+
+    testSerialization(g)
+
+    g = mkGraph()
+    DiffGraphApplier.applyDiff(
+      g,
+      new DiffGraphBuilder(schema)
+        .removeNode(g.node(0, 2))
+        .removeNode(g.node(0, 3))
+    )
+    debugDump(g) shouldBe
+      """#Node numbers (kindId, nnodes) (0: 4), total 4
+        |Node kind 0. (eid, nEdgesOut, nEdgesIn): (0, 2 [dense], 2 [dense]),
+        |   V0_0   [0] -> V0_1, V0_1
+        |   V0_1   [0] <- V0_0, V0_0
+        |""".stripMargin
+
+    testSerialization(g)
+  }
+
   "dont mess up after node deletion" in {
     val schema = TestSchema.make(1, 0, 1, nodePropertyPrototypes = Array(Array[String]("")), edgePropertyPrototypes = null)
     val g      = new Graph(schema)
