janusgraph icon indicating copy to clipboard operation
janusgraph copied to clipboard

Fixes Issue #1514, add customized edgeid

Open 9lan9 opened this issue 5 years ago • 7 comments

Fixes Issue #1514

Hi, I used an assumption under #1514 to fix this issue.

Here are the ideas:
The current edgeid is composed of long_encode(relationId) + long_encode(inV) + long_encode(relationtype.longId()) + long_encode(outV). The relationId is related to the add order(for the edge), the relationtype is related to the add order also(for the edgelabel). The same edgelabel has the same relationtype. So if the user can provide the relationId, the edgeid should be customized, and can locate it by this edgeid.

Here is the main modification:
1、add ALLOW_SETTING_EDGE_ID in GraphDatabaseConfiguration, whether user can provide relationId as the prefix of the edgeid
2、add relationId parameter in addEdge func and addE func

JanusGraphEdge addEdge(String relationId, JanusGraphVertex outVertex, JanusGraphVertex inVertex, EdgeLabel label)

public GraphTraversal<Edge, Edge> addE(final String relationId, final String label)

When the ALLOW_SETTING_EDGE_ID is True, addEdge func and addE func would firstly locate the edge, if this edge already existed, will throw UnsupportedOperationException(follow the vertex situation, if the vertexid already existed, will throw Exception), otherwise, the relationId would be the prefix of the edgeid.
When the ALLOW_SETTING_EDGE_ID is False, just use the former way would be enough.

Thanks for the help and support from @sinnergarden.
Please review it and provide your comments, thanks.


Thank you for contributing to JanusGraph!

In order to streamline the review of the contribution we ask you to ensure the following steps have been taken:

For all changes:

  • [x] Is there an issue associated with this PR? Is it referenced in the commit message?
  • [x] Does your PR body contain #xyz where xyz is the issue number you are trying to resolve?
  • [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
  • [x] Is your initial contribution a single, squashed commit?

For code changes:

  • [x] Have you written and/or updated unit tests to verify your changes?
  • [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • [ ] If applicable, have you updated the LICENSE.txt file, including the main LICENSE.txt file in the root of this repository?
  • [ ] If applicable, have you updated the NOTICE.txt file, including the main NOTICE.txt file found in the root of this repository?

For documentation related changes:

  • [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
  • [ ] If this PR is a documentation-only change, have you added a [doc only] tag to the first line of your commit message to avoid spending CPU cycles in Travis CI when no code, tests, or build configuration are modified?

Note:

Please ensure that once the PR is submitted, you check Travis CI for build issues and submit an update to your PR as soon as possible.

9lan9 avatar May 25 '20 13:05 9lan9

Hi @9lan9 / @porunov ,

What is the status of this pull request? What has been implemented so far and what's the list of outstanding sub-tasks/issues that still need to be addressed in order to complete this and have it reviewed, merged and accepted?

Thanks in advance! Kind regards,

Martin

carlspring avatar Sep 21 '20 01:09 carlspring

Hi @9lan9 / @porunov ,

What is the status of this pull request? What has been implemented so far and what's the list of outstanding sub-tasks/issues that still need to be addressed in order to complete this and have it reviewed, merged and accepted?

Thanks in advance! Kind regards,

Martin

Hi @carlspring, I think the feature can work well already. But this PR stuck in the check of doc, I follow the comment and still can not go through it. And I am not sure anyone is reviewing it. Thanks!

9lan9 avatar Sep 23 '20 05:09 9lan9

ping @porunov , @farodin91 could you guys review? :)

steve-todorov avatar Sep 23 '20 16:09 steve-todorov

@porunov , @farodin91 ,

What would be required in order to accept and merge this pull request?

carlspring avatar Sep 23 '20 16:09 carlspring

Hi @9lan9 / @porunov , What is the status of this pull request? What has been implemented so far and what's the list of outstanding sub-tasks/issues that still need to be addressed in order to complete this and have it reviewed, merged and accepted? Thanks in advance! Kind regards, Martin

Hi @carlspring, I think the feature can work well already. But this PR stuck in the check of doc, I follow the comment and still can not go through it. And I am not sure anyone is reviewing it. Thanks!

Thanks for your reply, @9lan9 ! :)

carlspring avatar Sep 23 '20 16:09 carlspring

LGTM! Thank you @9lan9

Thanks, @farodin91

9lan9 avatar Oct 01 '20 07:10 9lan9

@porunov Do you want to review it again?

farodin91 avatar Oct 01 '20 10:10 farodin91