Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

apoc.convert.toTree() returns wrong result if there is a cycle from root of the path #3996

Closed
snj07 opened this issue Mar 7, 2024 · 3 comments · Fixed by #4044
Closed

apoc.convert.toTree() returns wrong result if there is a cycle from root of the path #3996

snj07 opened this issue Mar 7, 2024 · 3 comments · Fixed by #4044

Comments

@snj07
Copy link

snj07 commented Mar 7, 2024 •
edited

In the path responses below, there is a cycle starting from node(10), which is the root of the path while preparing the tree. This line blindly replaces the map, leading to the issue where few nodes are getting missed in the tree response.

(10)-[FLOW,19]->(4)-[FLOW,2]->(12)-[FLOW,17]->(6)-[FLOW,7]->(11)-[FLOW,16]->(8), 

(10)-[FLOW,19]->(4)-[FLOW,8]->(17), 

(10)-[FLOW,19]->(4)-[FLOW,8]->(17)-[FLOW,12]->(6), 

(10)-[FLOW,19]->(4)-[FLOW,8]->(17)-[FLOW,12]->(6)-[FLOW,3]->(9), 

(10)-[FLOW,19]->(4)-[FLOW,8]->(17)-[FLOW,12]->(6)-[FLOW,3]->(9)-[FLOW,18]->(7), 

(10)-[FLOW,19]->(4)-[FLOW,8]->(17)-[FLOW,12]->(6)-[FLOW,6]->(15), 

(10)-[FLOW,19]->(4)-[FLOW,8]->(17)-[FLOW,12]->(6)-[FLOW,6]->(15)-[FLOW,11]->(3), 

(10)-[FLOW,19]->(4)-[FLOW,8]->(17)-[FLOW,12]->(6)-[FLOW,7]->(11), 

(10)-[FLOW,19]->(4)-[FLOW,8]->(17)-[FLOW,12]->(6)-[FLOW,7]->(11)-[FLOW,16]->(8), 

(10)-[FLOW,19]->(4)-[FLOW,9]->(13), 

(10)-[FLOW,19]->(4)-[FLOW,9]->(13)-[FLOW,14]->(7), 

(10)-[FLOW,19]->(4)-[FLOW,9]->(13)-[FLOW,14]->(7)-[FLOW,4]->(10),  // cycle

(10)-[FLOW,19]->(4)-[FLOW,9]->(13)-[FLOW,14]->(7)-[FLOW,10]->(16), 

(10)-[FLOW,19]->(4)-[FLOW,9]->(13)-[FLOW,14]->(7)-[FLOW,10]->(16)-[FLOW,15]->(3), 

(16)-[FLOW,15]->(3), 

(16)-[FLOW,15]->(3)-[FLOW,5]->(14), 

(16)-[FLOW,15]->(3)-[FLOW,5]->(14)-[FLOW,13]->(5)]

Self-loop also has incorrect response.
Graph:

v1 -> v2 -> v3-> v4 -> v5
v2-> v2 //self-loop

Response:  v1->v2->v2->v3->v4->v5
Expected:  
v1->v2->v2
      ->v3->v4->v5
@JoelBergstrand
Copy link
Contributor

Thanks for reporting! We'll look into it 🤨

@JoelBergstrand JoelBergstrand added team-cypher-surface Cypher Surface team should review the PR and removed team-cypher-surface Cypher Surface team should review the PR labels Mar 14, 2024
@gem-neo4j
Copy link
Contributor

Hello! Thank you for writing in :) There were some changes made in 5.18 to this procedure that might change the behaviour (adds rel id into the mix). I am not sure if that is the exact issue you are facing though, but might be worth a try.

I am struggling a little bit understanding what the issue is with the list of paths only, can you send me the queries you used for the simple case? e.g a CREATE for the dataset, followed by the actual call to APOC so I know which paths are going in to the results.

@snj07
Copy link
Author

snj07 commented Apr 3, 2024

I have shared the list of paths above. If you look at the given paths response, vertex(10) has a cycle, which is also the start/root of the path. While iterating through the list of paths, if you visit this path first:

(10)-[FLOW,19]->(4)-[FLOW,8]->(17)-[FLOW,12]->(6)-[FLOW,6]->(15)-[FLOW,11]->(3),

and after that visit:

(10)-[FLOW,19]->(4)-[FLOW,9]->(13)-[FLOW,14]->(7)-[FLOW,4]->(10),

then the line will replace the value in the map, so you won't get (15)-[FLOW,11]->(3) in the tree response, as it doesn't appear in any other path.
So, the correctness of apoc.convert.toTree() result will depend on the order of the paths.

@ncordon ncordon mentioned this issue Apr 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
APOC Extended Larus
Status: Done
Milestone
No milestone
3 participants
@snj07 @JoelBergstrand @gem-neo4j