opencypher / cypher-for-gremlin

Cypher for Gremlin adds Cypher support to any Gremlin graph database.
Apache License 2.0
359 stars 48 forks source link

Unsupported Set parameter array #236

Closed mad closed 5 years ago

mad commented 5 years ago

Example query and exception

16:45:09.428 [main] DEBUG o.a.t.g.driver.Client - CREATE (n:person {loc: 'uk'})}} to - Connection{host=Host{address=localhost/127.0.0.1:45155, hostUri=ws://localhost:45155/gremlin}}
16:45:09.429 [gremlin-server-worker-1] INFO  o.o.g.s.o.c.CypherOpProcessor - Cypher: CREATE (n:person {loc: 'uk'})
16:45:09.480 [gremlin-server-worker-1] INFO  o.o.g.s.o.c.CypherOpProcessor - Gremlin: g.addV('person').property(single, 'loc', 'uk').barrier().limit(0)
16:45:09.489 [main] DEBUG o.a.t.g.driver.Client - MATCH (n:person)SET n += {props} RETURN properties(n) as p, bindings={props={phone=[1, 2], name2=2, name1=1}}}} to - Connection{host=Host{address=localhost/127.0.0.1:45155, hostUri=ws://localhost:45155/gremlin}}
16:45:09.490 [gremlin-server-worker-1] INFO  o.o.g.s.o.c.CypherOpProcessor - Cypher: MATCH (n:person)SET n += {props} RETURN properties(n) as p
16:45:09.605 [gremlin-server-worker-1] WARN  o.a.t.g.s.h.OpExecutorHandler - [1, 2] (of class java.util.ArrayList)
scala.MatchError: [1, 2] (of class java.util.ArrayList)
    at org.opencypher.gremlin.translation.walker.NodeUtils$.toLiteral(NodeUtils.scala:204)
    at org.opencypher.gremlin.translation.walker.SetWalker$$anonfun$asMap$2.apply(SetWalker.scala:86)
    at org.opencypher.gremlin.translation.walker.SetWalker$$anonfun$asMap$2.apply(SetWalker.scala:85)
    at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
    at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
    at scala.collection.Iterator$class.foreach(Iterator.scala:891)
    at scala.collection.AbstractIterator.foreach(Iterator.scala:1334)
    at scala.collection.IterableLike$class.foreach(IterableLike.scala:72)
    at scala.collection.AbstractIterable.foreach(Iterable.scala:54)
    at scala.collection.TraversableLike$class.map(TraversableLike.scala:234)
    at scala.collection.AbstractTraversable.map(Traversable.scala:104)
    at org.opencypher.gremlin.translation.walker.SetWalker.asMap(SetWalker.scala:85)
    at org.opencypher.gremlin.translation.walker.SetWalker$$anonfun$walkSetClause$1.apply(SetWalker.scala:60)
    at org.opencypher.gremlin.translation.walker.SetWalker$$anonfun$walkSetClause$1.apply(SetWalker.scala:56)

Modified test

      @Test
    public void addPropertiesWithMapParameter() {
        submitAndGet("CREATE (n:person {loc: 'uk'})");

        Map<Object, Object> props = new HashMap<>();
        props.put("name1", 1);
        props.put("name2", 2);
        props.put("phone", Lists.newArrayList(1, 2));

        String cypher = "MATCH (n:person)" +
            "SET n += {props} " +
            "RETURN properties(n) as p";

        List<Map<String, Object>> update = submitAndGet(cypher, "props", props);

        assertThat(update)
            .extracting("p")
            .containsExactly(ImmutableMap.of(
                "loc", "uk",
                "name1", 1L,
                "name2", 2L,
                "phone", "[1, 2]"));
    }
dwitry commented 5 years ago

Hello @mad,

root issue here is that in Cypher for Gremlin collections in properties are supported only in in-memory TinkerGraph implementation (see CreateTest#createListProperty for more straightforward example).

Properties with "set" and "list" cardinality in Gremlin, and composite types in Cypher are different things, and we were unable to find how to correctly map this functionality.

Assuming you are using JanusGraph, implementation of collections in properties for TinkerGraph will not address you requirement, right?

mad commented 5 years ago

Hello @dwitry

Two way to support collections data in JanusGraph implementation

First, add custom serializer/deserializer for ArrayList type (https://docs.janusgraph.org/latest/serializer.html) I think, this is to close to Cypher composite types implementation

Second, create property with list/set cardinality (https://docs.janusgraph.org/latest/schema.html#property-cardinality). This way required translate any set/read operation to correct gremlin property(cardinality,...)/properties

First way it is enough for me

May be add TranslatorFeature.SERIALIZED_COLLECTIONS and after that cypher translator, set any collection as is?

dwitry commented 5 years ago

Hello @mad,

thank you for suggestions.

We have not implemented 'list'/'set' cardinality because there's no way known to us to distinguish between single value and list with a single value.

ArrayListSerializer potentially could address this requirement, unfortunately don't have time to look into this right now.

Setting label enhancement, might look into it later to improve compatibility with JanusGraph.

dwitry commented 5 years ago

Resolved by #267