Github user svanteschubert commented on the issue: https://github.com/apache/tinkerpop/pull/892 I use to work on the console, stability and clarity comes there before the performance when drafting on a solution (my product version would run from the console anyway). Do you think there is a performance real performance penalty when we do sorting for serialization? I am available for a code review over VOIP if you like to speed up things, Stephen. PS: The regarding your question, I looked it up on the GraphML Primer, it is an internal ID, see http://graphml.graphdrawing.org/primer/graphml-primer.html#AttributesDefinition 2018-07-16 13:45 GMT+02:00 stephen mallette <notificati...@github.com>: > *@spmallette* commented on this pull request. > ------------------------------ > > In gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/ > structure/io/graphml/GraphMLWriter.java > <https://github.com/apache/tinkerpop/pull/892#discussion_r202653620>: > > > @@ -218,20 +220,33 @@ private void writeTypes(final Map<String, String> identifiedVertexKeyTypes, > final Map<String, String> identifiedEdgeKeyTypes, > final XMLStreamWriter writer) throws XMLStreamException { > // <key id="weight" for="edge" attr.name="weight" attr.type="float"/> > - final Collection<String> vertexKeySet = getVertexKeysAndNormalizeIfRequired(identifiedVertexKeyTypes); > + Collection<String> vertexKeySet = getDataStructureKeysAndNormalizeIfRequired(identifiedVertexKeyTypes); > + Collection<String> edgeKeySet = getDataStructureKeysAndNormalizeIfRequired(identifiedEdgeKeyTypes); > + // in case vertex and edge may have the same attribute name, the key id in graphml have to be different > + intersection = CollectionUtils.intersection(vertexKeySet, edgeKeySet); > > In one respect, I think that using a suffix in the id is a good idea > generally - in other words, don't bother to even track the intersections, > just add the suffix, but I'm not sure how different tools use the id > attribute. I assume they use it as we use it, just to look up the > attr.name and attr.value - if so, then the id is simply internal to the > graphml and you don't need to track for intersections to add the suffix. > > If that is not the case and the choice is to not generalize this, then > we'd need tests to cover this logic. > > â > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <https://github.com/apache/tinkerpop/pull/892#pullrequestreview-137390995>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAyW2-CrJxLMO6tNEZK0-nwK9TZ8TIl9ks5uHHzmgaJpZM4VQWY9> > . >
---