You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2023/04/07 15:13:34 UTC
[tinkerpop] 01/01: TINKERPOP-2922 Fixed bugs in translator and grammar around Map
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2922
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 744a89e1dbbe397a301917016e9c12080f91d9b4
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Fri Apr 7 11:10:53 2023 -0400
TINKERPOP-2922 Fixed bugs in translator and grammar around Map
The grammar wasn't parsing groovy syntax for Map keys that hand parens that weren't enum related. Also didn't handle complex objects as keys. The translator was modified to produce slightly more idiomatic Map keys for string values.
---
CHANGELOG.asciidoc | 3 +++
.../process/traversal/translator/GroovyTranslator.java | 8 +++++---
.../language/grammar/GeneralLiteralVisitorTest.java | 5 +++++
.../traversal/translator/GroovyTranslatorTest.java | 17 +++++++++++++++--
.../translator/ParameterizedGroovyTranslatorTest.java | 2 +-
gremlin-language/src/main/antlr4/Gremlin.g4 | 6 ++++--
.../tinkerpop/gremlin/features/StepDefinition.java | 3 ---
7 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 8b8fb1c145..773f653a5e 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,9 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
This release also includes changes from <<release-3-5-6, 3.5.6>>.
* Fixed bug in grammar that prevented declaration of a `Map` key named `new` without quotes.
+* Fixed bug in grammar that prevented parsing of `Map` key surrounded by parenthesis which is allowable in Groovy.
+* Fixed bug in `GroovyTranslator` that surrounded `String` keys with parenthesis for `Map` when not necessary.
+* Added support to the grammar allowing `List` and `Map` key declarations for `Map` entries.
* Improved performance of comparison (equals) between not compatible types and nulls
[[release-3-6-2]]
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java
index bd0a64abc4..09e38709bc 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java
@@ -232,11 +232,13 @@ public final class GroovyTranslator implements Translator.ScriptTranslator {
final Iterator<? extends Map.Entry<?, ?>> itty = ((Map<?, ?>) o).entrySet().iterator();
while (itty.hasNext()) {
final Map.Entry<?,?> entry = itty.next();
- script.append("(");
+ final Object k = entry.getKey();
+ final boolean wrap = !(k instanceof String);
+ if (wrap) script.append("(");
convertToScript(entry.getKey());
- script.append("):(");
+ if (wrap) script.append(")");
+ script.append(":");
convertToScript(entry.getValue());
- script.append(")");
if (itty.hasNext())
script.append(",");
}
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/language/grammar/GeneralLiteralVisitorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/language/grammar/GeneralLiteralVisitorTest.java
index e9c1aa9f43..1350b03202 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/language/grammar/GeneralLiteralVisitorTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/language/grammar/GeneralLiteralVisitorTest.java
@@ -518,6 +518,7 @@ public class GeneralLiteralVisitorTest {
public static Iterable<Object[]> generateTestParameters() {
return Arrays.asList(new Object[][]{
{"[\"name\":\"simba\"]", 1},
+ {"[(\"name\"):\"simba\"]", 1},
{"[name:\"simba\", age: 29]", 2},
{"[:]", 0},
{"[1:'a']", 1},
@@ -527,6 +528,10 @@ public class GeneralLiteralVisitorTest {
{"[(from): 'source', (Direction.to): 'target']", 2},
{"[\"name\":\"simba\",\"age\":32]", 2},
{"[\"name\":\"simba\",\"age\":[2,3]]", 2},
+ {"[(3I):\"32\",([1I, 2I, 3.1D]):4I,\"x\":4I,\"+x\":8I]", 4},
+ {"[(3I):\"32\",[1I, 2I, 3.1D]:4I,\"x\":4I,\"+x\":8I]", 4},
+ {"[[label: 'person', T.id: 1]:\"x\"]", 1},
+ {"[([label: 'person', T.id: 1]):\"x\"]", 1},
{"[new: true]", 1},
{"[new : true]", 1},
{"['new' : true]", 1},
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslatorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslatorTest.java
index d2094da418..b9910fbbc8 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslatorTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslatorTest.java
@@ -120,8 +120,10 @@ public class GroovyTranslatorTest {
final String script = translator.translate(g.V().id().is(new LinkedHashMap<Object,Object>() {{
put(3, "32");
put(Arrays.asList(1, 2, 3.1d), 4);
+ put("x", 4);
+ put("+x", 8);
}})).getScript();
- assertEquals("g.V().id().is([((int) 3):(\"32\"),([(int) 1, (int) 2, 3.1d]):((int) 4)])", script);
+ assertEquals("g.V().id().is([((int) 3):\"32\",([(int) 1, (int) 2, 3.1d]):(int) 4,\"x\":(int) 4,\"+x\":(int) 8])", script);
}
@Test
@@ -166,7 +168,6 @@ public class GroovyTranslatorTest {
final Date date = Date.from(ZonedDateTime.of(2018, 03, 22, 0, 0, 0, 0, UTC).toInstant());
assertEquals("g.inject(datetime('2018-03-22T00:00:00Z'),datetime('2018-03-22T00:35:44.741Z'))",
t.translate(g.inject(date, datetime)).getScript());
-
}
@Test
@@ -176,7 +177,19 @@ public class GroovyTranslatorTest {
assertEquals("g.addE(\"knows\").from(new Vertex(1I,\"person\"))",
t.translate(g.addE("knows").from(new ReferenceVertex(1, "person"))).getScript());
+ }
+ @Test
+ public void shouldTranslateMapsUsingLanguageTypeTranslator() {
+ final Translator.ScriptTranslator t = GroovyTranslator.of("g",
+ new GroovyTranslator.LanguageTypeTranslator(false));
+ final String script = t.translate(g.V().id().is(new LinkedHashMap<Object,Object>() {{
+ put(3, "32");
+ put(Arrays.asList(1, 2, 3.1d), 4);
+ put("x", 4);
+ put("+x", 8);
+ }})).getScript();
+ assertEquals("g.V().id().is([(3I):\"32\",([1I, 2I, 3.1D]):4I,\"x\":4I,\"+x\":8I])", script);
}
@Test
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/ParameterizedGroovyTranslatorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/ParameterizedGroovyTranslatorTest.java
index 7bea6a3d52..cf1da8689f 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/ParameterizedGroovyTranslatorTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/ParameterizedGroovyTranslatorTest.java
@@ -167,7 +167,7 @@ public class ParameterizedGroovyTranslatorTest {
assertEquals(Integer.valueOf(2), bindings.get("_args_3"));
assertEquals(Double.valueOf(3.1), bindings.get("_args_4"));
assertEquals(Integer.valueOf(4), bindings.get("_args_5"));
- assertEquals("g.V().id().is([(_args_0):(_args_1),([_args_2, _args_3, _args_4]):(_args_5)])", script.getScript());
+ assertEquals("g.V().id().is([(_args_0):_args_1,([_args_2, _args_3, _args_4]):_args_5])", script.getScript());
}
@Test
diff --git a/gremlin-language/src/main/antlr4/Gremlin.g4 b/gremlin-language/src/main/antlr4/Gremlin.g4
index 215a0e740d..2e0ce504f5 100644
--- a/gremlin-language/src/main/antlr4/Gremlin.g4
+++ b/gremlin-language/src/main/antlr4/Gremlin.g4
@@ -1396,10 +1396,12 @@ genericLiteralMap
// allow builds of Map that sorta make sense in the Gremlin context
mapEntry
: NEW COLON genericLiteral // explicit for [new: true] - if we had other keywords like that maybe we'd group them up?
- | stringLiteral COLON genericLiteral
- | numericLiteral COLON genericLiteral
+ | (LPAREN stringLiteral RPAREN | stringLiteral) COLON genericLiteral
+ | (LPAREN numericLiteral RPAREN | numericLiteral) COLON genericLiteral
| (LPAREN traversalToken RPAREN | traversalToken) COLON genericLiteral
| (LPAREN traversalDirection RPAREN | traversalDirection) COLON genericLiteral
+ | (LPAREN genericLiteralCollection RPAREN | genericLiteralCollection) COLON genericLiteral
+ | (LPAREN genericLiteralMap RPAREN | genericLiteralMap) COLON genericLiteral
| Identifier COLON genericLiteral
;
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java
index bd5808cd95..803320a0cd 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java
@@ -33,11 +33,9 @@ import org.apache.tinkerpop.gremlin.language.grammar.GremlinAntlrToJava;
import org.apache.tinkerpop.gremlin.language.grammar.GremlinLexer;
import org.apache.tinkerpop.gremlin.language.grammar.GremlinParser;
import org.apache.tinkerpop.gremlin.process.traversal.Merge;
-import org.apache.tinkerpop.gremlin.process.traversal.Translator;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
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.translator.GroovyTranslator;
import org.apache.tinkerpop.gremlin.structure.Direction;
import org.apache.tinkerpop.gremlin.structure.Edge;
import org.apache.tinkerpop.gremlin.structure.T;
@@ -76,7 +74,6 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.Optional;
import java.util.function.Function;
import java.util.regex.Matcher;
import java.util.regex.Pattern;