[S2GRAPH-130]: Edge.propsWithTs data type should be changed into mutable to support setter interface exist in tp3. #100
Conversation
…ble to support setter interface exist in tp3. - Make Vertex/Edge/Graph to implement Tinkerpop3. - Change data type of Edge's propsWithTs to java.util.Map[String, S2Property[_]].
- change colId into ServiceColumn in VertexId.
| val iter = props.entrySet().iterator() | ||
| while (iter.hasNext) { | ||
| val e = iter.next() | ||
| if (e.getValue.ts > lastDeletedAt) ret = false |
There was a problem hiding this comment.
The while statement does not terminate the loop even if the condition '(e.getValue.ts> lastDeletedAt)' is satisfied.
There was a problem hiding this comment.
I guess you are suggesting to break when we first meet (e.getValue.ts > lastDeletedAt) condition. then I will fix on this part to break early on while loop.
There was a problem hiding this comment.
@daewon I just fixed it. can you review this again?
|
When you add a field, you should not forget to modify the function such as 'equals', 'hashcode', etc. It would be nice if a test for this would be added. I'm a little sorry to lose the advantage of the 'case class' provided by scala, but I think there is more to be gained when you support 'tinkerpop' as a native. +1 |
|
|
@daewon actually adding @HyunsungJo I will remove commented out codes. About EdgeTest, IndexEdgeTest, I think #101 has necessary code for them(just the methods @daewon mentioned above). Sorry for splitting up same logical commit into different PR(I found out this while I was working on #101 ^^;;). Maybe I will fix up removed test cases(EdgeTest, IndexEdgeTest, etc) on #101. |
…ble to support setter interface exist in tp3. JIRA: [S2GRAPH-130] https://issues.apache.org/jira/browse/S2GRAPH-130 Pull Request: Closes #100 Authors DO YUNG YOON: steamshon@apache.org
Note that this PR based on #99 PR.