[E-node < T501] Implement node.relationship_types - #300
Conversation
antepusic
left a comment
antepusic
left a comment
There was a problem hiding this comment.
The code looks good, but I'd like to see two things addressed before approving:
- the difference in function signature between us and APOC
- if the user gives e.g. "" as a relationship type, that should be handled appropriately
| if (type.starts_with('<')) { | ||
| result[type.substr(1, type.size() - 1)] |= 1; | ||
| } else if (type.ends_with('>')) { | ||
| result[type.substr(0, type.size() - 1)] |= 2; | ||
| } else { | ||
| result[type] |= 3; | ||
| } |
There was a problem hiding this comment.
The case where type both starts with '<' and ends with '>' (e.g. '') should also be addressed.
There was a problem hiding this comment.
We internally discussed it and decided that <> case isn't needed because it it is just like without the <> (since it covers both in and out nodes). Will discuss tomorrow on daily to either throw error or allow it.
There was a problem hiding this comment.
I've checked that and you're right about it 😄. However, if the user gives e.g. "<KNOWS>", the current code would treat that like "<KNOWS" and not "KNOWS".
There was a problem hiding this comment.
My bad, we already said to throw an error. Fixed
imilinovic
added
status: draft
antepusic
left a comment
antepusic
left a comment
There was a problem hiding this comment.
Thanks for the clarification about procedure signature. I think this is good to go now 🚀
3 participants
Description
Implement node.relationship_types
Pull request type
######################################
Reviewer checklist (the reviewer checks this part)
Module/Algorithm
######################################