-
Notifications
You must be signed in to change notification settings - Fork 32
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.
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
Adds properties on edges #306
Conversation
5b5b266
to
04f29e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks fine. Can you add a test to serve as an example and verify that this feature works?
04f29e3
to
ccc752e
Compare
@antepusic I changed the code a bit. Had some CI problems, not sure why it didn't like the first version... |
8451528
to
bd7f510
Compare
Related docs: |
Hi @andrejtonev, the related how to guide needs to be properly updated (reference guide is automated), and you should rebase to main. We no longer use the develop branch for the release; everything goes directly to the main. FYI, as discussed, this will be included in the next release (July 10th). Work on Memgraph is a priority, so we can move the milestone due date if you can't make it. |
0db37f2
to
01bfcd6
Compare
one_to_many: passing field parameters (which already existed but was not used) many_to_many: changed from parameters to column_names_mapping DescriptionResponding to user-made issue. Requesting the ability to define properties on edges. Pull request typePlease delete options that are not relevant.
Related issuesDelete section if this PR doesn't resolve any issues. Closes #301 Checklist:
###################################### Reviewer checklist (the reviewer checks this part)
###################################### |
Do not merge
|
@antepusic @Josipmrden I didn't like my previous implementation. Made a few changes. Please take another look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes make sense, well done! Just make sure that the checks pass (formatting & two tests) before we may merge
@katarinasupe I updated the relevant docs. |
f4f793c
to
8a53ae1
Compare
@andrejtonev Merge the latest main for tests to pass. |
one_to_many: passing field parameters (which already existed but was not used) many_to_many: changed from parameters to column_names_mapping using the mapping and columns to define properties ignoring from to keys when adding properties
8a53ae1
to
dfc20a1
Compare
dfc20a1
to
1d24658
Compare
Define relationship properties in many_to_many via name_mappings and properties field