tinkerpop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From spmalle...@apache.org
Subject tinkerpop git commit: TINKERPOP-1896 Fixed bug in lambda processing for python
Date Thu, 08 Mar 2018 22:20:24 GMT
Repository: tinkerpop
Updated Branches:
  refs/heads/TINKERPOP-1896 [created] 5c5df83e7


TINKERPOP-1896 Fixed bug in lambda processing for python

When a lambda is in a traversal, the bytecode gets pushed to a scriptengine for evaluation.
For python that means, using the JythonTranslator. Prior to this change the JythonTranslator
largely relied on the PythonTranslator to convert bytecode to Jython-syntaxed Gremlin. The
problem there is that in the JythonScriptEngine, the Traversal bindings are Java objects and
so if translated to pure Python syntax, traversal steps like as() become as_() and thus not
part of the Java API that is expected - and then errors. Not sure why this didn't come up
sooner, but it really only ends up being an issue if you are using bytecode, use gremlin server,
use jython in gremlin server and then submit a traversal with a lambda - so perhaps that hasn't
been a common combination.


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/5c5df83e
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/5c5df83e
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/5c5df83e

Branch: refs/heads/TINKERPOP-1896
Commit: 5c5df83e71ab0071c1d073a2ca68c5822ea9336d
Parents: 7bff988
Author: Stephen Mallette <spmva@genoprime.com>
Authored: Thu Mar 8 17:15:34 2018 -0500
Committer: Stephen Mallette <spmva@genoprime.com>
Committed: Thu Mar 8 17:15:34 2018 -0500

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  1 +
 .../jsr223/GremlinJythonScriptEngine.java       |  7 +--
 .../gremlin/python/jsr223/JythonTranslator.java |  9 ++++
 .../gremlin/python/jsr223/PythonTranslator.java | 19 +++++---
 .../python/jsr223/JythonTranslatorTest.java     | 47 +++++++++-----------
 5 files changed, 44 insertions(+), 39 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5c5df83e/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index a369172..3b084b7 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -29,6 +29,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 * Fixed deserialization of `P.not()` for GraphSON.
 * Bumped to Jackson 2.9.4.
 * Added `idleConnectionTimeout` and `keepAliveInterval` to Gremlin Server that enables a
"ping" and auto-close for seemingly dead clients.
+* Fixed a bug where lambdas in `gremlin-python` would trigger a failure if steps using python-only
symbols were present (such as `as_()`).
 * Fixed a bug in `NumberHelper` that led to wrong min/max results if numbers exceeded the
Integer limits.
 * Delayed setting of the request identifier until `RequestMessage` construction by the builder.
 * Improved error messaging for failed serialization and deserialization of request/response
messages.

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5c5df83e/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/GremlinJythonScriptEngine.java
----------------------------------------------------------------------
diff --git a/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/GremlinJythonScriptEngine.java
b/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/GremlinJythonScriptEngine.java
index 10f4790..a28a58f 100644
--- a/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/GremlinJythonScriptEngine.java
+++ b/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/GremlinJythonScriptEngine.java
@@ -41,11 +41,11 @@ import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Map;
 import java.util.stream.Collectors;
 
 /**
  * @author Marko A. Rodriguez (http://markorodriguez.com)
+ * @author Stephen Mallette (http://stephen.genoprime.com)
  */
 public class GremlinJythonScriptEngine implements GremlinScriptEngine {
 
@@ -123,10 +123,7 @@ public class GremlinJythonScriptEngine implements GremlinScriptEngine
{
         // extract the named traversalsource prior to that happening so that bytecode bindings
can share the same
         // namespace as global bindings (e.g. traversalsources and graphs).
         if (traversalSource.equals(HIDDEN_G))
-            throw new IllegalArgumentException("The traversalSource cannot have the name
" + HIDDEN_G+ " - it is reserved");
-
-        if (bindings.containsKey(HIDDEN_G))
-            throw new IllegalArgumentException("Bindings cannot include " + HIDDEN_G + "
- it is reserved");
+            throw new IllegalArgumentException("The traversalSource cannot have the name
" + HIDDEN_G + " - it is reserved");
 
         if (!bindings.containsKey(traversalSource))
             throw new IllegalArgumentException("The bindings available to the ScriptEngine
do not contain a traversalSource named: " + traversalSource);

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5c5df83e/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslator.java
----------------------------------------------------------------------
diff --git a/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslator.java
b/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslator.java
index 0eb8961..f8a5bc3 100644
--- a/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslator.java
+++ b/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslator.java
@@ -23,6 +23,7 @@ import org.apache.tinkerpop.gremlin.util.function.Lambda;
 
 /**
  * @author Marko A. Rodriguez (http://markorodriguez.com)
+ * @author Stephen Mallette (http://stephen.genoprime.com)
  */
 public final class JythonTranslator extends PythonTranslator {
 
@@ -54,4 +55,12 @@ public final class JythonTranslator extends PythonTranslator {
         else
             return "JythonUnknownArgLambda(" + lambdaString + ")";
     }
+
+    @Override
+    protected String resolveSymbol(final String methodName) {
+        // since this is Jython we should expect the Gremlin to conform to the java classes
to which the engine is
+        // bound - therefore, unlike the python engine which converts java names to python
friendly ones, this
+        // jython one can just pass them through.
+        return methodName;
+    }
 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5c5df83e/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/PythonTranslator.java
----------------------------------------------------------------------
diff --git a/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/PythonTranslator.java
b/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/PythonTranslator.java
index 4af4578..7c74b85 100644
--- a/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/PythonTranslator.java
+++ b/gremlin-python/src/main/java/org/apache/tinkerpop/gremlin/python/jsr223/PythonTranslator.java
@@ -56,6 +56,7 @@ import java.util.stream.Stream;
 
 /**
  * @author Marko A. Rodriguez (http://markorodriguez.com)
+ * @author Stephen Mallette (http://stephen.genoprime.com)
  */
 public class PythonTranslator implements Translator.ScriptTranslator {
 
@@ -114,7 +115,7 @@ public class PythonTranslator implements Translator.ScriptTranslator {
                     instruction.getArguments()[0].toString().contains("TranslationStrategy"))
                 continue;
             else if (0 == arguments.length)
-                traversalScript.append(".").append(SymbolHelper.toPython(methodName)).append("()");
+                traversalScript.append(".").append(resolveSymbol(methodName)).append("()");
             else if (methodName.equals("range") && 2 == arguments.length)
                 traversalScript.append("[").append(arguments[0]).append(":").append(arguments[1]).append("]");
             else if (methodName.equals("limit") && 1 == arguments.length)
@@ -123,7 +124,7 @@ public class PythonTranslator implements Translator.ScriptTranslator {
                 traversalScript.append(".").append(arguments[0]);
             else {
                 traversalScript.append(".");
-                String temp = SymbolHelper.toPython(methodName) + "(";
+                String temp = resolveSymbol(methodName) + "(";
                 for (final Object object : arguments) {
                     temp = temp + convertToString(object) + ",";
                 }
@@ -131,7 +132,7 @@ public class PythonTranslator implements Translator.ScriptTranslator {
             }
             // clip off __.
             if (this.importStatics && traversalScript.substring(0, 3).startsWith("__.")
-                    && !NO_STATIC.stream().filter(name -> traversalScript.substring(3).startsWith(SymbolHelper.toPython(name))).findAny().isPresent())
{
+                    && !NO_STATIC.stream().filter(name -> traversalScript.substring(3).startsWith(resolveSymbol(name))).findAny().isPresent())
{
                 traversalScript.delete(0, 3);
             }
         }
@@ -183,13 +184,13 @@ public class PythonTranslator implements Translator.ScriptTranslator
{
         else if (object instanceof Class)
             return ((Class) object).getCanonicalName();
         else if (object instanceof VertexProperty.Cardinality)
-            return "Cardinality." + SymbolHelper.toPython(object.toString());
+            return "Cardinality." + resolveSymbol(object.toString());
         else if (object instanceof SackFunctions.Barrier)
-            return "Barrier." + SymbolHelper.toPython(object.toString());
+            return "Barrier." + resolveSymbol(object.toString());
         else if (object instanceof TraversalOptionParent.Pick)
-            return "Pick." + SymbolHelper.toPython(object.toString());
+            return "Pick." + resolveSymbol(object.toString());
         else if (object instanceof Enum)
-            return convertStatic(((Enum) object).getDeclaringClass().getSimpleName() + ".")
+ SymbolHelper.toPython(object.toString());
+            return convertStatic(((Enum) object).getDeclaringClass().getSimpleName() + ".")
+ resolveSymbol(object.toString());
         else if (object instanceof P)
             return convertPToString((P) object, new StringBuilder()).toString();
         else if (object instanceof Element) {
@@ -237,4 +238,8 @@ public class PythonTranslator implements Translator.ScriptTranslator {
         return lambdaString.startsWith("lambda") ? lambdaString : "lambda " + lambdaString;
     }
 
+    protected String resolveSymbol(final String methodName) {
+        return SymbolHelper.toPython(methodName);
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5c5df83e/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslatorTest.java
----------------------------------------------------------------------
diff --git a/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslatorTest.java
b/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslatorTest.java
index 5165120..86e3b78 100644
--- a/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslatorTest.java
+++ b/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/jsr223/JythonTranslatorTest.java
@@ -22,11 +22,11 @@ package org.apache.tinkerpop.gremlin.python.jsr223;
 import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.TranslationStrategy;
 import org.apache.tinkerpop.gremlin.structure.Vertex;
 import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerFactory;
 import org.apache.tinkerpop.gremlin.util.function.Lambda;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import java.util.ArrayList;
@@ -37,39 +37,15 @@ import static org.junit.Assert.assertFalse;
 
 /**
  * @author Marko A. Rodriguez (http://markorodriguez.com)
+ * @author Stephen Mallette (http://stephen.genoprime.com)
  */
-@Ignore
 public class JythonTranslatorTest {
 
-    /*@Test
-    public void shouldHandleStrategies() throws Exception {
-        GraphTraversalSource g = TinkerFactory.createModern().traversal();
-        g = g.withStrategies(new HashMap<String, Object>() {{
-            put(SubgraphStrategy.STRATEGY, SubgraphStrategy.class.getCanonicalName());
-            put(SubgraphStrategy.VERTICES, __.has("name", "marko"));
-        }});
-        final Bindings bindings = new SimpleBindings();
-        bindings.put("g", g);
-        //System.out.println(JythonTranslator.of("g").translate(g.V().values("name").asAdmin().getBytecode()));
-        Traversal.Admin<Vertex, String> traversal = ((GremlinScriptEngine) ScriptEngineCache.get("gremlin-jython")).eval(g.V().values("name").asAdmin().getBytecode(),
bindings);
-        assertEquals("marko", traversal.next());
-        assertFalse(traversal.hasNext());
-        //
-        g = g.withStrategies(new HashMap<String, Object>() {{
-            put(SubgraphStrategy.STRATEGY, SubgraphStrategy.class.getCanonicalName());
-            put(SubgraphStrategy.VERTICES, __.has("name", "marko"));
-        }}, Collections.singletonMap(ReadOnlyStrategy.STRATEGY, ReadOnlyStrategy.class.getCanonicalName()));
-        //System.out.println(JythonTranslator.of("g").translate(g.V().values("name").asAdmin().getBytecode()));
-        traversal = ((GremlinScriptEngine) ScriptEngineCache.get("gremlin-jython")).eval(g.V().values("name").asAdmin().getBytecode(),
bindings);
-        assertEquals("marko", traversal.next());
-        assertFalse(traversal.hasNext());
-    }*/
-
     @Test
     public void shouldSupportStringSupplierLambdas() throws Exception {
         GraphTraversalSource g = TinkerFactory.createModern().traversal();
         g = g.withStrategies(new TranslationStrategy(g, JythonTranslator.of("g")));
-        GraphTraversal.Admin<Vertex, Integer> t = g.withSideEffect("lengthSum", 0).withSack(1)
+        final GraphTraversal.Admin<Vertex, Integer> t = g.withSideEffect("lengthSum",
0).withSack(1)
                 .V()
                 .filter(Lambda.predicate("x : x.get().label() == 'person'"))
                 .flatMap(Lambda.function("lambda x : x.get().vertices(Direction.OUT)"))
@@ -110,4 +86,21 @@ public class JythonTranslatorTest {
     public void shouldHaveValidToString() {
         assertEquals("translator[h:gremlin-jython]", JythonTranslator.of("h").toString());
     }
+
+    @Test
+    public void shouldTranslateToJythonAndNotPython() throws Exception {
+        // the jython translation bind "g" to java classes and thus does not require the
strict python syntax which
+        // converts steps like as() to as_(). if those steps are converted then the traversal
does not evaluate
+        // properly in the python engine. not much of an assertion here to worry about -
just need to ensure that
+        // the traversal with such steps evaluates to success
+        GraphTraversalSource g = TinkerFactory.createModern().traversal();
+        g = g.withStrategies(new TranslationStrategy(g, JythonTranslator.of("g")));
+        final List<Object> o = g.V().has("name").
+                match(__.as("x").label().as("lbl"),
+                        __.as("x").id().as("id")).
+                select("lbl", "id").
+                                 map(Lambda.function("lambda x: type(x.get())")).toList();
+
+        assertEquals(6, o.size());
+    }
 }


Mime
View raw message