Removing the transient attribute from the DetachedVertexProperty's vertex field [1] makes all Spark tests pass. Easy peasy, but holy cow, that little change probably has a huge impact on things I can't even think of right now.
[1] https://github.com/apache/tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/detached/DetachedVertexProperty.java#L40 Cheers, Daniel On Sat, Nov 10, 2018 at 11:49 AM Daniel Kuppitz <m...@gremlin.guru> wrote: > Treating null parents as a wildcard makes the majority of tests just pass, > but Spark integration tests fail. Spark is all about detached elements (or > reference elements) and I guess there's no way to make those tests pass > without making detached properties remember their parent elements (or at > least just their parent element ids). > > Cheers, > Daniel > > > On Fri, Nov 9, 2018 at 4:53 AM Stephen Mallette <spmalle...@gmail.com> > wrote: > >> If we can, I think that we should just keep derser semantics as they are. >> We've gone this long with them having null parent element ids and there >> haven't been any raised complains that I'm aware of. >> >> > I thought about skipping the parent id comparison in case one of the >> elements parents is *null* >> >> I suppose the question we need to answer stated directly is: What do we >> want equals() to mean for "detached" properties? Maybe it's not wrong to >> consider "detached" properties without a parent to have a form of wildcard >> status. If you were to "attach" such a property then its id space would be >> restricted to the element you're adding it to, so it seems safe in that >> respect. In the "tree test" that's failing that you referenced, the >> deserialized tree is a wildcard match for the original tree...the idea >> sorta works in that capacity, but perhaps there are other scenarios where >> that's a really bad thing? >> >> Does a "null" parent element mean "detached"? I guess the answer is >> "sometimes" because in some cases "detached" keeps the parent element. Are >> there other scenarios though where the parent element can be null? I can't >> really think of any right now. >> >> On Thu, Nov 8, 2018 at 12:48 PM Daniel Kuppitz <m...@gremlin.guru> wrote: >> >> > Working on TINKERPOP-2051 >> > <https://issues.apache.org/jira/browse/TINKERPOP-2051>, I thought I >> found >> > a >> > pretty easy solution: In order to allow locally unique property ids, we >> > only need to compare the ids of two properties as well as the ids of >> their >> > parent elements. That was basically a one-line change and had the >> expected >> > effect: >> > >> > >> > gremlin> g = TinkerGraph.open().traversal() >> > ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard] >> > gremlin> g.addV("person").property("name", "alice", id, "name").as("a"). >> > ......1> addV("person").property("name", "bob", id, "name"). >> > ......2> addE("knows").from("a") >> > ==>e[2][0-knows->1] >> > gremlin> >> > gremlin> g.V().properties("name") >> > ==>vp[name->alice] >> > ==>vp[name->bob] >> > gremlin> g.V().properties("name").dedup() >> > ==>vp[name->alice] >> > ==>vp[name->bob] >> > >> > >> > Without the change in id comparison, the last statement would have only >> > returned 1 of the properties. Looked easy enough, but - of course - it >> > broke the test suite. This change had an impact on 2 types of tests: >> > >> > - those, that deal with serialization and deserialization >> > - those, that make use of bytecode >> > >> > Perhaps the latter can be seen as a more specific case of the former. >> The >> > problem is that serialized/deserialized/detached properties no longer >> have >> > a reference to their former parent element. I thought about skipping the >> > parent id comparison in case one of the elements parents is *null*, but >> at >> > some point that just felt wrong and made me think of all kind of horror >> > scenarios this could/would lead to. >> > >> > So my question is, what would be the smartest way to handle that? Adjust >> > all test cases, so they no longer rely on full equality? Fix >> > (de)serialization to include parent element ids? Something else? >> > >> > *Example of a failing test:* >> > >> > [ERROR] SerializationTest$GryoV1d0Test.shouldSerializeTree:254 The >> > objects differ >> > expected: >> > >> > >> org.apache.tinkerpop.gremlin.process.traversal.step.util.Tree<{v[1]={v[2]={vp[name->vadas]={}}, >> > v[3]={vp[name->lop]={}}, v[4]={vp[name->josh]={}}}}> >> > but was: >> > >> > >> org.apache.tinkerpop.gremlin.process.traversal.step.util.Tree<{v[1]={v[2]={vp[name->vadas]={}}, >> > v[3]={vp[name->lop]={}}, v[4]={vp[name->josh]={}}}}> >> > >> > >> > Crazy thought: We could just change the tests to compare the toString() >> > versions of results.... >> > >> > Cheers, >> > Daniel >> > >> >
