Here's what is weird when I tried this kind of thing when i was working on the
PR. We basically want to fold any `single` cardinality key/value pair, so for
that the logic would be:
```java
if ((endStep instanceof AddVertexStep || endStep instanceof AddEdgeStep
||
endStep instanceof AddVertexStartStep || endStep instanceof
AddEdgeStartStep) &&
keyValues.length == 0 &&
(key instanceof T || (key instanceof String &&
((cardinality == VertexProperty.Cardinality.single) ||
(null == cardinality &&
asAdmin().getGraph().orElse(EmptyGraph.instance()).
features().vertex().getCardinality((String) key) ==
VertexProperty.Cardinality.single))))) {
((Mutating) endStep).addPropertyMutations(key, value);
} else {
this.asAdmin().addStep(new AddPropertyStep(this.asAdmin(),
cardinality, key, value));
((AddPropertyStep)
this.asAdmin().getEndStep()).addPropertyMutations(keyValues);
}
```
The problem, which will occur for graphs like TinkerGraph which don't have a
schema, is that:
```text
gremlin> g.addV().property(single, 'k',1).property(single,'k',2).profile()
==>Traversal Metrics
Step Count
Traversers Time (ms) % Dur
=============================================================================================================
AddVertexStartStep({k=[1, 2]}) 1
1 0.138 100.00
>TOTAL -
- 0.138 -
gremlin> g.addV().property(single, 'k',1).property(single,'k',2).valueMap()
==>[k:[1,2]]
```
If the user specifies the `single` and the key/values gets folded, the
specified cardinality is lost and the graph just does whatever it wants with
with the arguments given to `Graph.addVertex()`. For TinkerGraph, its default
cardinality is `list` so despite the user saying otherwise, it goes in as
`list`. Because the old logic uses `AddPropertyStep` the cardinality is
preserved as an argument and things work according to how the user specified
them.
I also tried this logic (which only takes into account the cardinality check
you suggested):
```java
if ((endStep instanceof AddVertexStep || endStep instanceof AddEdgeStep
||
endStep instanceof AddVertexStartStep || endStep instanceof
AddEdgeStartStep) &&
keyValues.length == 0 &&
(key instanceof T || (key instanceof String &&
(null == cardinality &&
asAdmin().getGraph().orElse(EmptyGraph.instance()).
features().vertex().getCardinality((String) key) ==
VertexProperty.Cardinality.single)))) {
((Mutating) endStep).addPropertyMutations(key, value);
} else {
this.asAdmin().addStep(new AddPropertyStep(this.asAdmin(),
cardinality, key, value));
((AddPropertyStep)
this.asAdmin().getEndStep()).addPropertyMutations(keyValues);
}
```
and that seemed to work a little better as `single` gets preserved properly:
```text
gremlin> g.addV().property(single, 'k',1).property(single,'k',2).profile()
==>Traversal Metrics
Step Count
Traversers Time (ms) % Dur
=============================================================================================================
AddVertexStartStep({}) 1
1 0.081 31.67
AddPropertyStep({key=[k], value=[1]}) 1
1 0.089 34.66
AddPropertyStep({key=[k], value=[2]}) 1
1 0.086 33.67
>TOTAL -
- 0.257 -
gremlin>
gremlin> g = TinkerGraph.open().traversal()
==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard]
gremlin> g.addV().property(single, 'k',1).property(single,'k',2).valueMap()
==>[k:[2]]
```
wdyt?
[ Full content available at: https://github.com/apache/tinkerpop/pull/1076 ]
This message was relayed via gitbox.apache.org for [email protected]
