You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2022/05/17 14:43:47 UTC
[GitHub] [tinkerpop] mikepersonick commented on a diff in pull request #1657: TINKERPOP-2742 Give graph providers a hint of property cardinality
mikepersonick commented on code in PR #1657:
URL: https://github.com/apache/tinkerpop/pull/1657#discussion_r874888967
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/Attachable.java:
##########
@@ -291,12 +290,18 @@ public static Vertex createVertex(final Attachable<Vertex> attachableVertex, fin
final Vertex vertex = hostGraph.features().vertex().willAllowId(baseVertex.id()) ?
hostGraph.addVertex(T.id, baseVertex.id(), T.label, baseVertex.label()) :
hostGraph.addVertex(T.label, baseVertex.label());
- baseVertex.properties().forEachRemaining(vp -> {
- final VertexProperty vertexProperty = hostGraph.features().vertex().properties().willAllowId(vp.id()) ?
- vertex.property(hostGraph.features().vertex().getCardinality(vp.key()), vp.key(), vp.value(), T.id, vp.id()) :
- vertex.property(hostGraph.features().vertex().getCardinality(vp.key()), vp.key(), vp.value());
- vp.properties().forEachRemaining(p -> vertexProperty.property(p.key(), p.value()));
- });
+ Map<String, List<VertexProperty<Object>>> propertyMap = new HashMap<>();
Review Comment:
nit: final
##########
tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/TinkerGraphProvider.java:
##########
@@ -107,7 +107,7 @@ protected static boolean requiresPersistence(final Class<?> test, final String t
*/
protected static boolean requiresListCardinalityAsDefault(final LoadGraphWith.GraphData loadGraphWith,
final Class<?> test, final String testMethodName) {
- return loadGraphWith == LoadGraphWith.GraphData.CREW
+ return loadGraphWith == LoadGraphWith.GraphData.CREW && !testMethodName.startsWith("shouldReadWriteCrew")
Review Comment:
Is it possible to fix the test case instead? I don't think we should hard code test names into the graph provider code. If it's not possible to fix we'll need to create an annotation for this case.
##########
tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java:
##########
@@ -411,6 +404,15 @@ public boolean willAllowId(final Object id) {
public VertexProperty.Cardinality getCardinality(final String key) {
return defaultVertexPropertyCardinality;
}
+
+ @Override
+ public VertexProperty.Cardinality getCardinality(final String key, final Object... values) {
+ VertexProperty.Cardinality cardinality = defaultVertexPropertyCardinality;
Review Comment:
nit: final
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/Attachable.java:
##########
@@ -291,12 +290,18 @@ public static Vertex createVertex(final Attachable<Vertex> attachableVertex, fin
final Vertex vertex = hostGraph.features().vertex().willAllowId(baseVertex.id()) ?
hostGraph.addVertex(T.id, baseVertex.id(), T.label, baseVertex.label()) :
hostGraph.addVertex(T.label, baseVertex.label());
- baseVertex.properties().forEachRemaining(vp -> {
- final VertexProperty vertexProperty = hostGraph.features().vertex().properties().willAllowId(vp.id()) ?
- vertex.property(hostGraph.features().vertex().getCardinality(vp.key()), vp.key(), vp.value(), T.id, vp.id()) :
- vertex.property(hostGraph.features().vertex().getCardinality(vp.key()), vp.key(), vp.value());
- vp.properties().forEachRemaining(p -> vertexProperty.property(p.key(), p.value()));
- });
+ Map<String, List<VertexProperty<Object>>> propertyMap = new HashMap<>();
+ baseVertex.properties().forEachRemaining(vp -> propertyMap.computeIfAbsent(vp.key(), k -> new ArrayList<>()).add(vp));
+ for (Map.Entry<String, List<VertexProperty<Object>>> entry : propertyMap.entrySet()) {
+ VertexProperty.Cardinality cardinality = hostGraph.features().vertex().getCardinality(
Review Comment:
nit: final
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org