Fix bug in apoc.refactor.mergeNodes

Better handling of constraints.
neo4j · Jan 13, 2025 · 8f69f50 · 8f69f50
1 parent 18f6e0a
commit 8f69f50
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 28 deletions.
diff --git a/core/src/main/java/apoc/refactor/GraphRefactoring.java b/core/src/main/java/apoc/refactor/GraphRefactoring.java
@@ -913,37 +913,21 @@ private void mergeNodes(Node source, Node target, RefactorConfig conf, List<Stri
 
     private void copyRelationships(Node source, Node target, boolean delete, boolean createNewSelfRel) {
         for (Relationship rel : source.getRelationships()) {
-            copyRelationship(rel, source, target, createNewSelfRel);
-            if (delete) rel.delete();
-        }
-    }
-
-    private Node copyLabels(Node source, Node target) {
-        for (Label label : source.getLabels()) {
-            if (!target.hasLabel(label)) {
-                target.addLabel(label);
-            }
-        }
-        return target;
-    }
-
-    private void copyRelationship(Relationship rel, Node source, Node target, boolean createNewSelfRelf) {
-        Node startNode = rel.getStartNode();
-        Node endNode = rel.getEndNode();
+            final var type = rel.getType();
+            var startNode = rel.getStartNode();
+            if (startNode.getElementId().equals(source.getElementId())) startNode = target;
+            var endNode = rel.getEndNode();
+            if (endNode.getElementId().equals(source.getElementId())) endNode = target;
 
-        if (startNode.getElementId().equals(endNode.getElementId()) && !createNewSelfRelf) {
-            return;
-        }
+            final var properties = rel.getAllProperties();
 
-        if (startNode.getElementId().equals(source.getElementId())) {
-            startNode = target;
-        }
+            // Delete first to avoid breaking constraints.
+            if (delete) rel.delete();
 
-        if (endNode.getElementId().equals(source.getElementId())) {
-            endNode = target;
+            if (createNewSelfRel || !startNode.getElementId().equals(endNode.getElementId())) {
+                final var newRel = startNode.createRelationshipTo(endNode, type);
+                properties.forEach(newRel::setProperty);
+            }
         }
-
-        Relationship newrel = startNode.createRelationshipTo(endNode, rel.getType());
-        copyProperties(rel, newrel);
     }
 }
diff --git a/core/src/test/java/apoc/refactor/GraphRefactoringTest.java b/core/src/test/java/apoc/refactor/GraphRefactoringTest.java
@@ -1962,6 +1962,42 @@ OPTIONAL MATCH (a)-[r]->(b)
         }
     }
 
+    @Test
+    public void mergeNodesWithConstraints() {
+        db.executeTransactionally("CREATE CONSTRAINT foo_uniq FOR ()-[r:MY_REL]-() REQUIRE r.foo IS UNIQUE");
+        db.executeTransactionally(
+                """
+                   CREATE
+                       (n1 {name: "n1"})-[r1:MY_REL {foo: "a"}]->(n2 {name: "n2"}),
+                       (n3 {name: "n3"})-[r2:MY_REL {foo: "b"}]->(n4 {name: "n4"})""");
+        db.executeTransactionally("CALL db.awaitIndexes()");
+        db.executeTransactionally(
+                """
+                   MATCH (n1 {name: "n1"}), (n3 {name: "n3"})
+                   CALL apoc.refactor.mergeNodes([n1, n3], {properties: 'discard'}) YIELD node
+                   FINISH""");
+
+        try (final var tx = db.beginTx()) {
+            final var query =
+                    """
+                    MATCH (a)
+                    OPTIONAL MATCH (a)-[r]->(b)
+                    RETURN
+                      a.name,
+                      CASE WHEN r.foo IS NULL THEN 'null' ELSE r.foo END AS `r.foo`,
+                      CASE WHEN b.name IS NULL THEN 'null' ELSE b.name END AS `b.name`
+                    ORDER BY `a.name`, `r.foo`, `b.name`
+                    """;
+            assertThat(tx.execute(query).stream())
+                    .containsExactly(
+                            Map.of("a.name", "n1", "r.foo", "a", "b.name", "n2"),
+                            Map.of("a.name", "n1", "r.foo", "b", "b.name", "n4"),
+                            Map.of("a.name", "n2", "r.foo", "null", "b.name", "null"),
+                            Map.of("a.name", "n4", "r.foo", "null", "b.name", "null"));
+            tx.commit();
+        }
+    }
+
     private void issue2797Common(String extractQuery) {
         db.executeTransactionally("CREATE CONSTRAINT unique_book FOR (book:MyBook) REQUIRE book.name IS UNIQUE");
         db.executeTransactionally("CREATE (n:MyBook {name: 1})");