Skip to content

inject() should accept bound variables #3636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
danielbodart wants to merge 1 commit into from
Closed

inject() should accept bound variables #3636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
[[release-4-0-0]]
=== TinkerPop 4.0.0 (Release Date: NOT OFFICIALLY RELEASED YET)

* Allowed `inject()` to accept bound variables, aligning it with the other variadic steps (`V()`, `E()`, `within()`, `without()`, etc.) which already accept a `genericArgument`; previously `inject()` was the only such step restricted to literals.
* Fixed `gremlin-go` to report a malformed or truncated GraphBinary response as a deserialization error rather than a bare decoder message.
* Made `TinkerGraph` an interface and renamed the in-memory implementation to `TinkerMemoryGraph`; `TinkerGraph.open()` and `gremlin.graph=...TinkerGraph` behave as before. *(breaking)*
* Renamed `TinkerTransactionGraph` to `TinkerStorageGraph`. *(breaking)*
Expand Down
2 changes: 1 addition & 1 deletion ...e/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/TraversalMethodVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1037,7 +1037,7 @@ public GraphTraversal visitTraversalMethod_inV(final GremlinParser.TraversalMeth
*/
@Override
public GraphTraversal visitTraversalMethod_inject(final GremlinParser.TraversalMethod_injectContext ctx) {
return graphTraversal.inject(antlr.genericVisitor.parseObjectVarargs(ctx.genericLiteralVarargs()));
return graphTraversal.inject(antlr.argumentVisitor.parseObjectVarargs(ctx.genericArgumentVarargs()));
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...java/org/apache/tinkerpop/gremlin/language/grammar/TraversalSourceSpawnMethodVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public GraphTraversal visitTraversalSourceSpawnMethod_V(final GremlinParser.Trav
*/
@Override
public GraphTraversal visitTraversalSourceSpawnMethod_inject(final GremlinParser.TraversalSourceSpawnMethod_injectContext ctx) {
return this.traversalSource.inject(antlr.genericVisitor.parseObjectVarargs(ctx.genericLiteralVarargs()));
return this.traversalSource.inject(antlr.argumentVisitor.parseObjectVarargs(ctx.genericArgumentVarargs()));
}

/**
Expand Down
40 changes: 18 additions & 22 deletions ...rc/main/java/org/apache/tinkerpop/gremlin/language/translator/GroovyTranslateVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import org.antlr.v4.runtime.ParserRuleContext;
import org.apache.tinkerpop.gremlin.language.grammar.GremlinParser;
import org.apache.tinkerpop.gremlin.language.grammar.GremlinParser.GenericLiteralVarargsContext;
import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertex;

/**
Expand Down Expand Up @@ -267,29 +266,26 @@ public Void visitTraversalMethod_hasLabel_String_String(final GremlinParser.Trav
* inject() ends up being ambiguous with groovy's jdk extension of inject(Object initialValue, Closure closure)
*/
private Void handleInject(final ParserRuleContext ctx) {
if (ctx.getChildCount() > 3 && ctx.getChild(2) instanceof GremlinParser.GenericLiteralVarargsContext) {
final GremlinParser.GenericLiteralVarargsContext varArgs = (GremlinParser.GenericLiteralVarargsContext) ctx.getChild(2);
if (varArgs.getChildCount() == 1 && varArgs.getChild(0) instanceof GremlinParser.GenericLiteralExprContext) {
GremlinParser.GenericLiteralExprContext injectArgs = (GremlinParser.GenericLiteralExprContext) varArgs.getChild(0);
if (injectArgs.getChildCount() > 2 && "null".equals(injectArgs.getChild(2).getText())) {
sb.append(ctx.getChild(0).getText());
sb.append("(");
for (int i = 0; i < injectArgs.getChildCount(); i += 2) {
if (i == 2) {
sb.append("(Object) null");
} else {
visit(injectArgs.getChild(i));
}

if (i < injectArgs.getChildCount() - 1) {
sb.append(", ");
}
}

sb.append(")");
return null;
// ctx is always K_INJECT LPAREN genericArgumentVarargs RPAREN, so child(2) is the varargs node.
final GremlinParser.GenericArgumentVarargsContext injectArgs =
(GremlinParser.GenericArgumentVarargsContext) ctx.getChild(2);
if (injectArgs.getChildCount() > 2 && "null".equals(injectArgs.getChild(2).getText())) {
sb.append(ctx.getChild(0).getText());
sb.append("(");
for (int i = 0; i < injectArgs.getChildCount(); i += 2) {
if (i == 2) {
sb.append("(Object) null");
} else {
visit(injectArgs.getChild(i));
}

if (i < injectArgs.getChildCount() - 1) {
sb.append(", ");
}
}

sb.append(")");
return null;
}

return visitChildren(ctx);
Expand Down
5 changes: 5 additions & 0 deletions ...e/src/test/java/org/apache/tinkerpop/gremlin/language/grammar/GremlinQueryParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ public void shouldParseVariablesInVarargs() {
t = (GraphTraversal<?, ?>) GremlinQueryParser.parse("g.V(x, y, 300).has('name',gt(z))", gremlinAntlrToJava);
assertEquals(g.V(100, 200, 300).has("name", P.gt(50)).asAdmin().getGremlinLang(),
t.asAdmin().getGremlinLang());

// inject() accepts bound variables like the other varargs steps (V, within, etc.)
t = (GraphTraversal<?, ?>) GremlinQueryParser.parse("g.inject(x, y, 300)", gremlinAntlrToJava);
assertEquals(g.inject(100, 200, 300).asAdmin().getGremlinLang(),
t.asAdmin().getGremlinLang());
}

@Test(expected = GremlinParserException.class)
Expand Down
9 changes: 9 additions & 0 deletions ...src/test/java/org/apache/tinkerpop/gremlin/language/translator/GremlinTranslatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,15 @@ public static Collection<Object[]> data() {
"g.inject(null, null).inject(null, null)",
"g.inject(null, null).inject(null, null)",
"g.inject(None, None).inject(None, None)"},
{"g.inject(1, 2)",
null,
"g.inject(number0, number1)",
"g.Inject<object>(1, 2)",
"g.Inject(1, 2)",
null,
null,
null,
null},
{"g.V().hasLabel(null)",
null,
"g.V().hasLabel(string0)",
Expand Down
36 changes: 15 additions & 21 deletions gremlin-js/gremlin-javascript/lib/language/translator/GroovyTranslateVisitor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,29 +290,23 @@ export default class GroovyTranslateVisitor extends TranslateVisitor {
private handleInject(ctx: any): void {
if (ctx.getChildCount() > 3) {
const child2 = ctx.getChild(2);
if (child2?.constructor?.name === 'GenericLiteralVarargsContext') {
const varArgs = child2;
if (varArgs.getChildCount() === 1) {
const child0 = varArgs.getChild(0);
if (child0?.constructor?.name === 'GenericLiteralExprContext') {
const injectArgs = child0;
if (injectArgs.getChildCount() > 2 && injectArgs.getChild(2).getText() === 'null') {
this.sb.push(ctx.getChild(0).getText());
this.sb.push('(');
for (let i = 0; i < injectArgs.getChildCount(); i += 2) {
if (i === 2) {
this.sb.push('(Object) null');
} else {
this.visit(injectArgs.getChild(i));
}
if (i < injectArgs.getChildCount() - 1) {
this.sb.push(', ');
}
}
this.sb.push(')');
return;
if (child2?.constructor?.name === 'GenericArgumentVarargsContext') {
const injectArgs = child2;
if (injectArgs.getChildCount() > 2 && injectArgs.getChild(2).getText() === 'null') {
this.sb.push(ctx.getChild(0).getText());
this.sb.push('(');
for (let i = 0; i < injectArgs.getChildCount(); i += 2) {
if (i === 2) {
this.sb.push('(Object) null');
} else {
this.visit(injectArgs.getChild(i));
}
if (i < injectArgs.getChildCount() - 1) {
this.sb.push(', ');
}
}
this.sb.push(')');
return;
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions gremlin-language/src/main/antlr4/Gremlin.g4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ traversalSourceSpawnMethod_V
;

traversalSourceSpawnMethod_inject
: K_INJECT LPAREN genericLiteralVarargs RPAREN
: K_INJECT LPAREN genericArgumentVarargs RPAREN
;

traversalSourceSpawnMethod_io
Expand Down Expand Up @@ -626,7 +626,7 @@ traversalMethod_index
;

traversalMethod_inject
: K_INJECT LPAREN genericLiteralVarargs RPAREN
: K_INJECT LPAREN genericArgumentVarargs RPAREN
;

traversalMethod_is
Expand Down
10 changes: 10 additions & 0 deletions ...anguage/src/test/java/org/apache/tinkerpop/gremlin/language/grammar/BasicGrammarTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,14 @@ public class BasicGrammarTest extends AbstractGrammarTest {
public void shouldParseV() {
parse("g.V()", ParserRule.QUERY_LIST);
}

@Test
public void shouldParseInjectWithVariable() {
parse("g.inject(x)", ParserRule.QUERY_LIST);
}

@Test
public void shouldParseInjectStepWithVariable() {
parse("g.V().inject(x)", ParserRule.QUERY_LIST);
}
}
Loading