diff --git a/docs/asciidoc/modules/ROOT/nav.adoc b/docs/asciidoc/modules/ROOT/nav.adoc index b53bb7daae..c30f1ebf01 100644 --- a/docs/asciidoc/modules/ROOT/nav.adoc +++ b/docs/asciidoc/modules/ROOT/nav.adoc @@ -142,6 +142,7 @@ include::partial$generated-documentation/nav.adoc[] ** xref::operational/init-script.adoc[] ** xref::operational/warmup.adoc[] ** xref::operational/log.adoc[] + ** xref::operational/rebind.adoc[] * xref:misc/index.adoc[] ** xref::misc/text-functions.adoc[] diff --git a/docs/asciidoc/modules/ROOT/pages/operational/index.adoc b/docs/asciidoc/modules/ROOT/pages/operational/index.adoc index 8f89545d70..5df51d751f 100644 --- a/docs/asciidoc/modules/ROOT/pages/operational/index.adoc +++ b/docs/asciidoc/modules/ROOT/pages/operational/index.adoc @@ -9,3 +9,4 @@ For more information on how to use these procedures, see: * xref::operational/init-script.adoc[] * xref::operational/warmup.adoc[] * xref::operational/log.adoc[] +* xref::operational/rebind.adoc[] \ No newline at end of file diff --git a/docs/asciidoc/modules/ROOT/pages/operational/rebind.adoc b/docs/asciidoc/modules/ROOT/pages/operational/rebind.adoc new file mode 100644 index 0000000000..f0df312d00 --- /dev/null +++ b/docs/asciidoc/modules/ROOT/pages/operational/rebind.adoc @@ -0,0 +1,84 @@ +APOC provides a list of functions to rebind nodes and relationships: + +* `apoc.node.rebind(node)` +* `apoc.rel.rebind(rel)` +* `apoc.any.rebind(map/list/paths/...)` + +== Why use these functions + +Unlike versions up to 3.5, +in Neo4j 4.x the entities hold a reference to their originating transaction. + +This can cause problems when for example, we create a node (called n1) in a transaction, open a new transaction in which we create another node (called n2) and execute +via `org.neo4j.graphdb.Node.createRelationshipTo()`, +that is `n1.createRelationshipTo(n2)`. + + +Is what happens when we perform the following operation: + +[source,cypher] +---- +// node creation +CREATE (:Article {content: 'contentBody'}); + +// iterate all (:Article) nodes and new transaction and rel creation +CALL apoc.periodic.iterate('MATCH (art:Article) RETURN art', +'CREATE (node:Category) with art, node call apoc.create.relationship(art, "CATEGORY", {b: 1}, node) yield rel return rel', {}); +---- + +Basically, we create a node `(:Article)`, +then the second parameter of the xref::overview/apoc.periodic/apoc.periodic.iterate.adoc[apoc.periodic.iterate] open a new transaction and create a node `(:Category)`, +and finally we try to create a relationship between `(:Article)` and `(:Category)` via xref::overview/apoc.create/apoc.create.relationship.adoc[apoc.create.relationship] (which uses under the hood the `org.neo4j.graphdb.Node.createRelationshipTo()` ). + +If we try to execute the second query the apoc.periodic.iterate will return an `errorMessage` similar to this: +---- +Failed to invoke procedure `apoc.create.relationship`: Caused by: org.neo4j.graphdb.NotFoundException: Node[10] is deleted and cannot be used to create a relationship": 1 +---- + + +== How to solve + +To solve the previous `apoc.periodic,iterate`, +we can leverage return the internal id from the first statement and then match the node via id, +that means doing a `MATCH (n) WHERE id(n) = id(nodeId)` +(this operation is called rebinding). + +That is: +[source,cypher] +---- +CALL apoc.periodic.iterate('MATCH (art:Article) RETURN id(art) as id', +'CREATE (node:Category) WITH id, node MATCH (art) where id(art) = id + WITH art, node call apoc.create.relationship(art, "CATEGORY", {b: 1}, node) yield rel return rel', {}); +---- + +Alternatively, we can wrap with the `apoc.node.rebind` function the node that have to be rebound, like this: +[source,cypher] +---- +CALL apoc.periodic.iterate('MATCH (art:Article) RETURN art', +'CREATE (node:Category) with art, node call apoc.create.relationship(art, "CATEGORY", {b: 1}, node) yield rel return rel', {}); +---- + +Regarding relationships, we can use `apoc.rel.rebind`: +[source,cypher] +---- +// other operations... +MATCH (:Start)-[rel:REL]->(:End) /*...*/ RETURN apoc.rel.rebind(rel) +---- + +We can also use the `apoc.any.rebind(ANY)` to rebind multiple entities placed in maps, lists, paths, or a combination of these three. +This will return the same structure passed in the argument, but with rebound entities. +For example: + +.Map of entities +[source,cypher] +---- +CREATE (a:Foo)-[r1:MY_REL]->(b:Bar)-[r2:ANOTHER_REL]->(c:Baz) WITH a,b,c,r1,r2 +RETURN apoc.any.rebind({first: a, second: b, third: c, rels: [r1, r2]}) as rebind +---- + +.List of paths +[source,cypher] +---- +CREATE p1=(a:Foo)-[r1:MY_REL]->(b:Bar), p2=(:Bar)-[r2:ANOTHER_REL]->(c:Baz) +RETURN apoc.any.rebind([p1, p2]) as rebind +---- \ No newline at end of file diff --git a/full/src/main/java/apoc/nodes/NodesExtended.java b/full/src/main/java/apoc/nodes/NodesExtended.java new file mode 100644 index 0000000000..b2432255d4 --- /dev/null +++ b/full/src/main/java/apoc/nodes/NodesExtended.java @@ -0,0 +1,37 @@ +package apoc.nodes; + +import apoc.Extended; +import apoc.util.EntityUtil; +import apoc.util.Util; +import org.neo4j.graphdb.Node; +import org.neo4j.graphdb.Relationship; +import org.neo4j.graphdb.Transaction; +import org.neo4j.procedure.Context; +import org.neo4j.procedure.Description; +import org.neo4j.procedure.Name; +import org.neo4j.procedure.UserFunction; + +@Extended +public class NodesExtended { + + @Context + public Transaction tx; + + @UserFunction("apoc.node.rebind") + @Description("apoc.node.rebind(node - to rebind a node (i.e. executing a Transaction.getNodeById(node.getId()) ") + public Node nodeRebind(@Name("node") Node node) { + return Util.rebind(tx, node); + } + + @UserFunction("apoc.rel.rebind") + @Description("apoc.rel.rebind(rel) - to rebind a rel (i.e. executing a Transaction.getRelationshipById(rel.getId()) ") + public Relationship relationshipRebind(@Name("rel") Relationship rel) { + return Util.rebind(tx, rel); + } + + @UserFunction("apoc.any.rebind") + @Description("apoc.any.rebind(Object) - to rebind any rel, node, path, map, list or combination of them (i.e. executing a Transaction.getNodeById(node.getId()) / Transaction.getRelationshipById(rel.getId()))") + public Object anyRebind(@Name("any") Object any) { + return EntityUtil.anyRebind(tx, any); + } +} diff --git a/full/src/main/java/apoc/util/EntityUtil.java b/full/src/main/java/apoc/util/EntityUtil.java new file mode 100644 index 0000000000..3c0a2d5e58 --- /dev/null +++ b/full/src/main/java/apoc/util/EntityUtil.java @@ -0,0 +1,37 @@ +package apoc.util; + +import org.neo4j.graphalgo.impl.util.PathImpl; +import org.neo4j.graphdb.Entity; +import org.neo4j.graphdb.Path; +import org.neo4j.graphdb.Relationship; +import org.neo4j.graphdb.Transaction; +import org.neo4j.internal.helpers.collection.Iterables; + +import java.util.Map; +import java.util.stream.Collectors; + +public class EntityUtil { + + public static T anyRebind(Transaction tx, T any) { + if (any instanceof Map) { + return (T) ((Map) any).entrySet().stream() + .collect(Collectors.toMap(e -> e.getKey(), e -> anyRebind(tx, e.getValue()))); + } + if (any instanceof Path) { + final Path path = (Path) any; + PathImpl.Builder builder = new PathImpl.Builder(Util.rebind(tx, path.startNode())); + for (Relationship rel: path.relationships()) { + builder = builder.push(Util.rebind(tx, rel)); + } + return (T) builder.build(); + } + if (any instanceof Iterable) { + return (T) Iterables.stream((Iterable) any) + .map(i -> anyRebind(tx, i)).collect(Collectors.toList()); + } + if (any instanceof Entity) { + return (T) Util.rebind(tx, (Entity) any); + } + return any; + } +} diff --git a/full/src/main/resources/extended.txt b/full/src/main/resources/extended.txt index 70a95deb03..0acb60971d 100644 --- a/full/src/main/resources/extended.txt +++ b/full/src/main/resources/extended.txt @@ -162,8 +162,11 @@ apoc.uuid.install apoc.uuid.list apoc.uuid.remove apoc.uuid.removeAll +apoc.any.rebind apoc.coll.avgDuration apoc.data.email +apoc.node.rebind +apoc.rel.rebind apoc.static.get apoc.static.getAll apoc.trigger.nodesByLabel diff --git a/full/src/test/java/apoc/nodes/NodesExtendedTest.java b/full/src/test/java/apoc/nodes/NodesExtendedTest.java new file mode 100644 index 0000000000..e0411c0297 --- /dev/null +++ b/full/src/test/java/apoc/nodes/NodesExtendedTest.java @@ -0,0 +1,69 @@ +package apoc.nodes; + +import apoc.create.Create; +import apoc.util.TestUtil; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.neo4j.graphdb.Label; +import org.neo4j.graphdb.Path; +import org.neo4j.graphdb.Relationship; +import org.neo4j.graphdb.RelationshipType; +import org.neo4j.internal.helpers.collection.Iterables; +import org.neo4j.test.rule.DbmsRule; +import org.neo4j.test.rule.ImpermanentDbmsRule; + +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import static org.junit.Assert.assertEquals; + + +public class NodesExtendedTest { + + @Rule + public DbmsRule db = new ImpermanentDbmsRule(); + + @Before + public void setUp() throws Exception { + TestUtil.registerProcedure(db, NodesExtended.class, Create.class); + } + + @Test + public void rebind() { + TestUtil.testCall(db, "CREATE (a:Foo)-[r1:MY_REL]->(b:Bar)-[r2:ANOTHER_REL]->(c:Baz) WITH a,b,c,r1,r2 \n" + + "RETURN apoc.any.rebind({first: a, second: b, third: c, rels: [r1, r2]}) as rebind", + (row) -> { + final Map rebind = (Map) row.get("rebind"); + final List rels = (List) rebind.get("rels"); + final Relationship firstRel = rels.get(0); + final Relationship secondRel = rels.get(1); + assertEquals(firstRel.getStartNode(), rebind.get("first")); + assertEquals(firstRel.getEndNode(), rebind.get("second")); + assertEquals(secondRel.getStartNode(), rebind.get("second")); + assertEquals(secondRel.getEndNode(), rebind.get("third")); + }); + + TestUtil.testCall(db, "CREATE p1=(a:Foo)-[r1:MY_REL]->(b:Bar), p2=(:Bar)-[r2:ANOTHER_REL]->(c:Baz) \n" + + "RETURN apoc.any.rebind([p1, p2]) as rebind", + (row) -> { + final List rebindList = (List) row.get("rebind"); + assertEquals(2, rebindList.size()); + final Path firstPath = rebindList.get(0); + assertPath(firstPath, List.of("Foo", "Bar"), List.of("MY_REL")); + final Path secondPath = rebindList.get(1); + assertPath(secondPath, List.of("Bar", "Baz"), List.of("ANOTHER_REL")); + }); + } + + private void assertPath(Path rebind, List labels, List relTypes) { + final List actualLabels = Iterables.stream(rebind.nodes()) + .map(i -> i.getLabels().iterator().next()) + .map(Label::name).collect(Collectors.toList()); + assertEquals(labels, actualLabels); + final List actualRelTypes = Iterables.stream(rebind.relationships()).map(Relationship::getType) + .map(RelationshipType::name).collect(Collectors.toList()); + assertEquals(relTypes, actualRelTypes); + } +} diff --git a/full/src/test/java/apoc/periodic/PeriodicExtendedTest.java b/full/src/test/java/apoc/periodic/PeriodicExtendedTest.java index 284f6912cb..faf840ada6 100644 --- a/full/src/test/java/apoc/periodic/PeriodicExtendedTest.java +++ b/full/src/test/java/apoc/periodic/PeriodicExtendedTest.java @@ -1,6 +1,9 @@ package apoc.periodic; +import apoc.create.Create; import apoc.load.Jdbc; +import apoc.nlp.gcp.GCPProcedures; +import apoc.nodes.NodesExtended; import apoc.util.TestUtil; import org.junit.Before; import org.junit.Rule; @@ -15,12 +18,16 @@ import org.neo4j.test.rule.ImpermanentDbmsRule; import java.sql.SQLException; +import java.util.Collections; import java.util.Map; +import java.util.function.Consumer; import static apoc.util.TestUtil.testCall; +import static apoc.util.TestUtil.testCallCount; import static apoc.util.TestUtil.testResult; import static apoc.util.Util.map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; public class PeriodicExtendedTest { @@ -30,7 +37,57 @@ public class PeriodicExtendedTest { @Before public void initDb() { - TestUtil.registerProcedure(db, Periodic.class, PeriodicExtended.class, Jdbc.class); + TestUtil.registerProcedure(db, Periodic.class, NodesExtended.class, GCPProcedures.class, Create.class, PeriodicExtended.class, Jdbc.class); + } + + @Test + public void testRebindWithNlpWriteProcedure() { + // use case: https://community.neo4j.com/t5/neo4j-graph-platform/use-of-apoc-periodic-iterate-with-apoc-nlp-gcp-classify-graph/m-p/56846#M33854 + final String iterate = "MATCH (node:Article) RETURN node"; + final String action = "CALL apoc.nlp.gcp.classify.graph(node, $nlpConf) YIELD graph RETURN null"; + testRebindCommon(iterate, action, 0, this::assertNodeDeletedErr); + + final String actionRebind = "CALL apoc.nlp.gcp.classify.graph(apoc.node.rebind(node), $nlpConf) YIELD graph RETURN null"; + testRebindCommon(iterate, actionRebind, 2, this::assertNoErrors); + + // "manual" rebind, i.e. "return id(node) as id" in iterate query, and "match .. where id(n)=id" in action query + final String iterateId = "MATCH (node:Article) RETURN id(node) AS id"; + final String actionId = "MATCH (node) WHERE id(node) = id CALL apoc.nlp.gcp.classify.graph(node, $nlpConf) YIELD graph RETURN null"; + + testRebindCommon(iterateId, actionId, 2, this::assertNoErrors); + } + + @Test + public void testRebindWithMapIterationAndCreateRelationshipProcedure() { + final String iterate = "MATCH (art:Article) RETURN {key: art, key2: 'another'} as map"; + final String action = "CREATE (node:Category) with map.key as art, node call apoc.create.relationship(art, 'CATEGORY', {b: 1}, node) yield rel return rel"; + testRebindCommon(iterate, action, 0, this::assertNodeDeletedErr); + + final String actionRebind = "WITH apoc.any.rebind(map) AS map " + action; + testRebindCommon(iterate, actionRebind, 1, this::assertNoErrors); + } + + private void assertNoErrors(Map r) { + assertEquals(Collections.emptyMap(), r.get("errorMessages")); + } + + private void assertNodeDeletedErr(Map r) { + assertTrue(((Map) r.get("errorMessages")) + .keySet().stream() + .anyMatch(k -> k.matches("Node\\[\\d+] is deleted and cannot be used to create a relationship"))); + } + + private void testRebindCommon(String iterate, String action, int expected, Consumer> assertions) { + final Map nlpConf = map("key", "myKey", "nodeProperty", "content", "write", true, "unsupportedDummyClient", true); + final Map config = map("params", map("nlpConf", nlpConf)); + + db.executeTransactionally("CREATE (:Article {content: 'contentBody'})"); + testCall(db,"CALL apoc.periodic.iterate($iterate, $action, $config)", + map( "iterate" , iterate, "action", action, "config", config), + assertions); + + testCallCount(db, "MATCH p=(:Category)<-[:CATEGORY]-(:Article) RETURN p", expected); + db.executeTransactionally("MATCH (n) DETACH DELETE n"); } @Test