[tinkerpop] branch master updated: TINKERPOP-2742: Warn users of vertex property cardinality mismatch

[sp...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git


The following commit(s) were added to refs/heads/master by this push:
     new 68d15dde6f TINKERPOP-2742: Warn users of vertex property cardinality mismatch
     new 08faf9221d Merge pull request #1719 from li-boxuan/warn-cardinality-in-io-step
68d15dde6f is described below

commit 68d15dde6fe9ce0facc1adbfaee2e3da11bad214
Author: Boxuan Li <li...@connect.hku.hk>
AuthorDate: Sat Jun 18 11:11:00 2022 -0400

    TINKERPOP-2742: Warn users of vertex property cardinality mismatch
---
 docs/src/reference/the-traversal.asciidoc          |  4 +++
 .../gremlin/structure/util/Attachable.java         | 29 +++++++++++++++++-----
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/docs/src/reference/the-traversal.asciidoc b/docs/src/reference/the-traversal.asciidoc
index e2613238a1..a9244695b2 100644
--- a/docs/src/reference/the-traversal.asciidoc
+++ b/docs/src/reference/the-traversal.asciidoc
@@ -1622,6 +1622,10 @@ time is not critical) that are loading to an empty graph - incremental loading i
 writing perspective is not that different in there are no parallel operations in play, however streaming the output
 to disk requires a single pass of the data without high memory requirements for larger datasets.
 
+IMPORTANT: Default graph formats don't contain information about property cardinality, so it is up to the graph
+provider to choose the appropriate one. You will see a warning message if the chosen cardinality is SINGLE
+while your graph input contains multiple values for that property.
+
 In general, TinkerPop recommends that users examine the native bulk import/export tools of the graph implementation
 that they choose. Those tools will often outperform the `io()`-step and perhaps be easier to use with a greater
 feature set. That said, graph providers do have the option to optimize `io()` to back it with their own
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/Attachable.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/Attachable.java
index 3cd76f2b0f..701121f63e 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/Attachable.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/Attachable.java
@@ -27,8 +27,14 @@ import org.apache.tinkerpop.gremlin.structure.T;
 import org.apache.tinkerpop.gremlin.structure.Vertex;
 import org.apache.tinkerpop.gremlin.structure.VertexProperty;
 import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.function.Function;
 
@@ -43,6 +49,8 @@ import java.util.function.Function;
  */
 public interface Attachable<V> {
 
+    Logger logger = LoggerFactory.getLogger(Attachable.class);
+
     /**
      * Get the raw object trying to be attached.
      *
@@ -291,12 +299,21 @@ public interface Attachable<V> {
             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()));
-            });
+            final 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()) {
+                final VertexProperty.Cardinality cardinality = hostGraph.features().vertex().getCardinality(entry.getKey());
+                if (VertexProperty.Cardinality.single == cardinality && entry.getValue().size() > 1) {
+                    logger.warn("{} has SINGLE cardinality but with more than one value: {}. Only last value will be retained.",
+                        entry.getKey(), entry.getValue());
+                }
+                for (VertexProperty<Object> vp : entry.getValue()) {
+                    final VertexProperty<Object> vertexProperty = hostGraph.features().vertex().properties().willAllowId(vp.id()) ?
+                        vertex.property(cardinality, vp.key(), vp.value(), T.id, vp.id()) :
+                        vertex.property(cardinality, vp.key(), vp.value());
+                    vp.properties().forEachRemaining(p -> vertexProperty.property(p.key(), p.value()));
+                }
+            }
             return vertex;
         }