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 2021/09/02 15:19:26 UTC
[tinkerpop] 01/10: TINKERPOP-2605 hasLabel(null) filters results
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2605
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 086f7faddc485ed2002e51e4f893e79b17f2a8e6
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Tue Aug 24 09:26:59 2021 -0400
TINKERPOP-2605 hasLabel(null) filters results
This change gets rid of the general NullPointerException, fixes .NET translation problems around ambiguous method calls, introduces some additional validations around calling has(T,...) with a null T value and adds null as a valid value for StringLiterals
---
CHANGELOG.asciidoc | 3 +
docs/src/upgrade/release-3.5.x.asciidoc | 14 +++
.../traversal/dsl/graph/GraphTraversal.java | 46 ++++++++--
.../process/traversal/step/map/GraphStep.java | 3 +-
.../process/traversal/step/map/PropertiesStep.java | 3 +-
.../process/traversal/step/util/HasContainer.java | 10 +-
.../traversal/translator/DotNetTranslator.java | 19 +++-
.../traversal/translator/DotNetTranslatorTest.java | 14 +++
.../Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs | 13 ++-
.../gremlin-javascript/test/cucumber/gremlin.js | 13 ++-
gremlin-language/src/main/antlr4/Gremlin.g4 | 1 +
gremlin-python/src/main/python/radish/gremlin.py | 13 ++-
gremlin-test/features/filter/Has.feature | 92 ++++++++++++++++++-
.../process/traversal/step/filter/HasTest.java | 101 ++++++++++++++++++++-
.../traversal/step/sideEffect/Neo4jGraphStep.java | 13 ++-
15 files changed, 319 insertions(+), 39 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index b157ee7..825b951 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -28,6 +28,9 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
* Fixed bug where `RepeatUnrollStrategy`, `InlineFilterStrategy` and `MessagePassingReductionStrategy` were all being applied more than necessary.
* Ensured `PathRetractionStrategy` is applied after `InlineFilterStrategy` which prevents an error in traverser mapping in certain conditions.
* Deprecated `JsonBuilder` serialization for GraphSON and Gryo.
+* Allowed `null` string values in the Gremlin grammar.
+* Prevented exception with `hasLabel(null)` and instead filter away traversers as labels can't ever be null.
+* Refined `DotNetTranslator` to be more explicit with `null` arguments to ensure that the right overloads are called.
* Created `GremlinParser` to construct `Traversal` objects from `gremlin-language`.
[[release-3-5-1]]
diff --git a/docs/src/upgrade/release-3.5.x.asciidoc b/docs/src/upgrade/release-3.5.x.asciidoc
index c59a972..5a0d66b 100644
--- a/docs/src/upgrade/release-3.5.x.asciidoc
+++ b/docs/src/upgrade/release-3.5.x.asciidoc
@@ -28,6 +28,20 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
Please see the link:https://github.com/apache/tinkerpop/blob/3.5.2/CHANGELOG.asciidoc#release-3-5-2[changelog] for a
complete list of all the modifications that are part of this release.
+=== Upgrading for Users
+
+==== Refinements to null
+
+Release 3.5.0 introduce the ability for there to be traversers that contained a `null` value. Since that time it has
+been noted that supplying a `null` value as an argument to certain Gremlin steps might cause exception behaviors that
+were either not clear or perhaps even unexpected. The following changes were made to help make these behaviors more
+consistent and to further solidify the semantics of `null` usage in the Gremlin language:
+
+* `hasLabel(null)` and `has(T.label,null)` - Formerly, these methods threw general `NullPointerException` failures,
+but now filter results. As a `T.label` can't be `null`, it will effectively filter all elements.
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-2605[TINKERPOP-2605]
+
== TinkerPop 3.5.1
*Release Date: July 19, 2021*
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
index deb3908..63a394e 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
@@ -1387,6 +1387,14 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> {
* @since 3.0.0-incubating
*/
public default GraphTraversal<S, E> has(final T accessor, final P<?> predicate) {
+ if (null == accessor)
+ throw new IllegalArgumentException("The T accessor value of has(T,Object) cannot be null");
+
+ // Groovy can get the overload wrong for has(T, null) which should probably go at has(T,Object). users could
+ // explicit cast but a redirect here makes this a bit more seamless
+ if (null == predicate)
+ return has(accessor, (Object) null);
+
this.asAdmin().getBytecode().addStep(Symbols.has, accessor, predicate);
return TraversalHelper.addHasContainer(this.asAdmin(), new HasContainer(accessor.getAccessor(), predicate));
}
@@ -1421,6 +1429,9 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> {
* @since 3.0.0-incubating
*/
public default GraphTraversal<S, E> has(final T accessor, final Object value) {
+ if (null == accessor)
+ throw new IllegalArgumentException("The T accessor value of has(T,Object) cannot be null");
+
if (value instanceof P)
return this.has(accessor, (P) value);
else if (value instanceof Traversal)
@@ -1474,6 +1485,14 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> {
* @since 3.1.0-incubating
*/
public default GraphTraversal<S, E> has(final T accessor, final Traversal<?, ?> propertyTraversal) {
+ if (null == accessor)
+ throw new IllegalArgumentException("The T accessor value of has(T,Object) cannot be null");
+
+ // Groovy can get the overload wrong for has(T, null) which should probably go at has(T,Object). users could
+ // explicit cast but a redirect here makes this a bit more seamless
+ if (null == propertyTraversal)
+ return has(accessor, (Object) null);
+
this.asAdmin().getBytecode().addStep(Symbols.has, accessor, propertyTraversal);
switch (accessor) {
case id:
@@ -1482,8 +1501,8 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> {
new IdStep<>(propertyTraversal.asAdmin()))));
case label:
return this.asAdmin().addStep(
- new TraversalFilterStep<>(this.asAdmin(), propertyTraversal.asAdmin().addStep(0,
- new LabelStep<>(propertyTraversal.asAdmin()))));
+ new TraversalFilterStep<>(this.asAdmin(), propertyTraversal.asAdmin().addStep(0,
+ new LabelStep<>(propertyTraversal.asAdmin()))));
default:
throw new IllegalArgumentException("has(T,Traversal) can only take id or label as its argument");
}
@@ -1542,15 +1561,21 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> {
* @since 3.2.2
*/
public default GraphTraversal<S, E> hasLabel(final String label, final String... otherLabels) {
- final String[] labels = new String[otherLabels.length + 1];
+ this.asAdmin().getBytecode().addStep(Symbols.hasLabel, label, otherLabels);
+
+ // groovy evaluation seems to do strange things with varargs given hasLabel(null, null). odd someone would
+ // do this but the failure is ugly if not handled.
+ final int otherLabelsLength = null == otherLabels ? 0 : otherLabels.length;
+ final String[] labels = new String[otherLabelsLength + 1];
labels[0] = label;
- System.arraycopy(otherLabels, 0, labels, 1, otherLabels.length);
- this.asAdmin().getBytecode().addStep(Symbols.hasLabel, labels);
+ if (otherLabelsLength > 0)
+ System.arraycopy(otherLabels, 0, labels, 1, otherLabelsLength);
return TraversalHelper.addHasContainer(this.asAdmin(), new HasContainer(T.label.getAccessor(), labels.length == 1 ? P.eq(labels[0]) : P.within(labels)));
}
/**
- * Filters vertices, edges and vertex properties based on their label.
+ * Filters vertices, edges and vertex properties based on their label. Note that calling this step with
+ * {@code null} is the same as calling {@link #hasLabel(String, String...)} with a single {@code null}.
*
* @param predicate the filter to apply to the label of the {@link Element}
* @return the traversal with an appended {@link HasStep}
@@ -1558,8 +1583,13 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> {
* @since 3.2.4
*/
public default GraphTraversal<S, E> hasLabel(final P<String> predicate) {
- this.asAdmin().getBytecode().addStep(Symbols.hasLabel, predicate);
- return TraversalHelper.addHasContainer(this.asAdmin(), new HasContainer(T.label.getAccessor(), predicate));
+ // if calling hasLabel(null), the likely use the caller is going for is not a "no predicate" but a eq(null)
+ if (null == predicate) {
+ return hasLabel((String) null);
+ } else {
+ this.asAdmin().getBytecode().addStep(Symbols.hasLabel, predicate);
+ return TraversalHelper.addHasContainer(this.asAdmin(), new HasContainer(T.label.getAccessor(), predicate));
+ }
}
/**
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java
index aa3a29f..7982e37 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java
@@ -42,6 +42,7 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
+import java.util.Objects;
import java.util.function.Supplier;
/**
@@ -177,7 +178,7 @@ public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implemen
int result = super.hashCode() ^ this.returnClass.hashCode();
if (null != this.ids) {
for (final Object id : this.ids) {
- result ^= id.hashCode();
+ result ^= Objects.hashCode(id);
}
}
return result;
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertiesStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertiesStep.java
index 7cc193a..dce4fed 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertiesStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertiesStep.java
@@ -30,6 +30,7 @@ import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
import java.util.Arrays;
import java.util.Collections;
import java.util.Iterator;
+import java.util.Objects;
import java.util.Set;
/**
@@ -81,7 +82,7 @@ public class PropertiesStep<E> extends FlatMapStep<Element, E> implements AutoCl
public int hashCode() {
int result = super.hashCode() ^ this.returnType.hashCode();
for (final String propertyKey : this.propertyKeys) {
- result ^= propertyKey.hashCode();
+ result ^= Objects.hashCode(propertyKey);
}
return result;
}
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java
index 111e23c..31cea06 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java
@@ -104,23 +104,23 @@ public class HasContainer implements Serializable, Cloneable, Predicate<Element>
return false;
}
- protected boolean testId(Element element) {
+ protected boolean testId(final Element element) {
return this.predicate.test(element.id());
}
- protected boolean testIdAsString(Element element) {
+ protected boolean testIdAsString(final Element element) {
return this.predicate.test(element.id().toString());
}
- protected boolean testLabel(Element element) {
+ protected boolean testLabel(final Element element) {
return this.predicate.test(element.label());
}
- protected boolean testValue(Property property) {
+ protected boolean testValue(final Property property) {
return this.predicate.test(property.value());
}
- protected boolean testKey(Property property) {
+ protected boolean testKey(final Property property) {
return this.predicate.test(property.key());
}
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslator.java
index 181ee1f..d4dcc35 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslator.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslator.java
@@ -340,11 +340,20 @@ public final class DotNetTranslator implements Translator.ScriptTranslator {
convertToScript(instruction.getArguments()[1]);
script.append(",");
} else {
- for (final Object object : instruction.getArguments()) {
- // overloads might have trouble with null. add more as we find them i guess
- if (null == object && methodName.equals(GraphTraversal.Symbols.addV))
- script.append("(string) ");
- convertToScript(object);
+ final Object[] instArgs = instruction.getArguments();
+ for (int idx = 0; idx < instArgs.length; idx++) {
+ final Object instArg = instArgs[idx];
+ // overloads might have trouble with null in calling the right one. add more as we find
+ // them i guess
+ if (null == instArg) {
+ if ((methodName.equals(GraphTraversal.Symbols.addV) && idx % 2 == 0) ||
+ methodName.equals(GraphTraversal.Symbols.hasLabel)) {
+ script.append("(string) ");
+ } else if (methodName.equals(GraphTraversal.Symbols.has)) {
+ if (instArgs[0] instanceof T && idx > 0) script.append("(object) ");
+ }
+ }
+ convertToScript(instArg);
script.append(",");
}
}
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslatorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslatorTest.java
index a1ec5cb..3e5654c 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslatorTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslatorTest.java
@@ -154,6 +154,20 @@ public class DotNetTranslatorTest {
}
@Test
+ public void shouldTranslateHasLabelNull() {
+ String script = translator.translate(g.V().hasLabel(null).asAdmin().getBytecode()).getScript();
+ assertEquals("g.V().HasLabel((string) null)", script);
+ script = translator.translate(g.V().hasLabel(null, null).asAdmin().getBytecode()).getScript();
+ assertEquals("g.V().HasLabel((string) null,(string) null)", script);
+ script = translator.translate(g.V().hasLabel(null, "person").asAdmin().getBytecode()).getScript();
+ assertEquals("g.V().HasLabel((string) null,\"person\")", script);
+ script = translator.translate(g.V().hasLabel(null, "person", null).asAdmin().getBytecode()).getScript();
+ assertEquals("g.V().HasLabel((string) null,\"person\",(string) null)", script);
+ script = translator.translate(g.V().has(T.label, (Object) null).asAdmin().getBytecode()).getScript();
+ assertEquals("g.V().Has(T.Label,(object) null)", script);
+ }
+
+ @Test
public void shouldEscapeStrings() {
final String script = translator.translate(g.addV("customer")
.property("customer_id", 501L)
diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs
index 407c9b8..0e1ae90 100644
--- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs
+++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs
@@ -186,8 +186,8 @@ namespace Gremlin.Net.IntegrationTest.Gherkin
{"g_VX1X_out_hasXid_2_3X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V(p["vid1"]).Out().HasId(p["vid2"],p["vid3"])}},
{"g_VX1X_out_hasXid_2AsString_3AsStringX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V(p["vid1"]).Out().HasId(p["vid2"],p["vid3"])}},
{"g_V_hasXblahX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Has("blah")}},
- {"g_EX7X_hasXlabelXknowsX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.E(p["eid7"]).HasLabel("knows")}},
- {"g_E_hasXlabelXknowsX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.E().HasLabel("knows")}},
+ {"g_EX7X_hasLabelXknowsX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.E(p["eid7"]).HasLabel("knows")}},
+ {"g_E_hasLabelXknowsX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.E().HasLabel("knows")}},
{"g_E_hasLabelXuses_traversesX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.E().HasLabel("uses","traverses")}},
{"g_V_hasLabelXperson_software_blahX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("person","software","blah")}},
{"g_V_hasXperson_name_markoX_age", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Has("person","name","marko").Values<object>("age")}},
@@ -224,6 +224,15 @@ namespace Gremlin.Net.IntegrationTest.Gherkin
{"g_V_hasXage_gtX18X_andXltX30XXorXgtx35XXX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Has("age",P.Gt(18).And(P.Lt(30)).Or(P.Gt(35)))}},
{"g_V_hasXage_gtX18X_andXltX30XXorXltx35XXX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Has("age",P.Gt(18).And(P.Lt(30)).And(P.Lt(35)))}},
{"g_V_hasXk_withinXcXX_valuesXkX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV().Property("k","\u8F49\u6CE8").AddV().Property("k","\u2726").AddV().Property("k","\u2660").AddV().Property("k","A"), (g,p) =>g.V().Has("k",P.Within(new List<object> {"\u8F49\u6CE8", "\u2726", "\u2660"})).Values<object>("k")}},
+ {"g_V_hasXnullX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Has(null)}},
+ {"g_E_hasXnullX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.E().Has(null)}},
+ {"g_V_hasLabelXnullX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel((string) null)}},
+ {"g_V_hasXlabel_nullX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Has(T.Label,(object) null)}},
+ {"g_V_hasLabelXnull_nullX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel((string) null,(string) null)}},
+ {"g_V_hasLabelXnull_personX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel((string) null,"person")}},
+ {"g_E_hasLabelXnullX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.E().HasLabel((string) null)}},
+ {"g_E_hasXlabel_nullX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.E().Has(T.Label,(object) null)}},
+ {"g_V_properties_hasLabelXnullX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Properties<object>().HasLabel((string) null)}},
{"g_V_valuesXageX_isX32X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Values<object>("age").Is(32)}},
{"g_V_valuesXageX_isXlte_30X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Values<object>("age").Is(P.Lte(30))}},
{"g_V_valuesXageX_isXgte_29X_isXlt_34X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Values<object>("age").Is(P.Gte(29)).Is(P.Lt(34))}},
diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js
index be4f51f..8a7e31f 100644
--- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js
+++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js
@@ -175,8 +175,8 @@ const gremlins = {
g_VX1X_out_hasXid_2_3X: [function({g, vid3, vid2, vid1}) { return g.V(vid1).out().hasId(vid2,vid3) }],
g_VX1X_out_hasXid_2AsString_3AsStringX: [function({g, vid3, vid2, vid1}) { return g.V(vid1).out().hasId(vid2,vid3) }],
g_V_hasXblahX: [function({g}) { return g.V().has("blah") }],
- g_EX7X_hasXlabelXknowsX: [function({g, eid7}) { return g.E(eid7).hasLabel("knows") }],
- g_E_hasXlabelXknowsX: [function({g}) { return g.E().hasLabel("knows") }],
+ g_EX7X_hasLabelXknowsX: [function({g, eid7}) { return g.E(eid7).hasLabel("knows") }],
+ g_E_hasLabelXknowsX: [function({g}) { return g.E().hasLabel("knows") }],
g_E_hasLabelXuses_traversesX: [function({g}) { return g.E().hasLabel("uses","traverses") }],
g_V_hasLabelXperson_software_blahX: [function({g}) { return g.V().hasLabel("person","software","blah") }],
g_V_hasXperson_name_markoX_age: [function({g}) { return g.V().has("person","name","marko").values("age") }],
@@ -213,6 +213,15 @@ const gremlins = {
g_V_hasXage_gtX18X_andXltX30XXorXgtx35XXX: [function({g}) { return g.V().has("age",P.gt(18).and(P.lt(30)).or(P.gt(35))) }],
g_V_hasXage_gtX18X_andXltX30XXorXltx35XXX: [function({g}) { return g.V().has("age",P.gt(18).and(P.lt(30)).and(P.lt(35))) }],
g_V_hasXk_withinXcXX_valuesXkX: [function({g}) { return g.addV().property("k","\u8F49\u6CE8").addV().property("k","\u2726").addV().property("k","\u2660").addV().property("k","A") }, function({g}) { return g.V().has("k",P.within(["\u8F49\u6CE8", "\u2726", "\u2660"])).values("k") }],
+ g_V_hasXnullX: [function({g}) { return g.V().has(null) }],
+ g_E_hasXnullX: [function({g}) { return g.E().has(null) }],
+ g_V_hasLabelXnullX: [function({g}) { return g.V().hasLabel(null) }],
+ g_V_hasXlabel_nullX: [function({g}) { return g.V().has(T.label,null) }],
+ g_V_hasLabelXnull_nullX: [function({g}) { return g.V().hasLabel(null,null) }],
+ g_V_hasLabelXnull_personX: [function({g}) { return g.V().hasLabel(null,"person") }],
+ g_E_hasLabelXnullX: [function({g}) { return g.E().hasLabel(null) }],
+ g_E_hasXlabel_nullX: [function({g}) { return g.E().has(T.label,null) }],
+ g_V_properties_hasLabelXnullX: [function({g}) { return g.V().properties().hasLabel(null) }],
g_V_valuesXageX_isX32X: [function({g}) { return g.V().values("age").is(32) }],
g_V_valuesXageX_isXlte_30X: [function({g}) { return g.V().values("age").is(P.lte(30)) }],
g_V_valuesXageX_isXgte_29X_isXlt_34X: [function({g}) { return g.V().values("age").is(P.gte(29)).is(P.lt(34)) }],
diff --git a/gremlin-language/src/main/antlr4/Gremlin.g4 b/gremlin-language/src/main/antlr4/Gremlin.g4
index 37c8228..2d8d030 100644
--- a/gremlin-language/src/main/antlr4/Gremlin.g4
+++ b/gremlin-language/src/main/antlr4/Gremlin.g4
@@ -1293,6 +1293,7 @@ booleanLiteral
stringLiteral
: NonEmptyStringLiteral
| EmptyStringLiteral
+ | NullLiteral
| gremlinStringConstants
;
diff --git a/gremlin-python/src/main/python/radish/gremlin.py b/gremlin-python/src/main/python/radish/gremlin.py
index 4d58428..9560589 100644
--- a/gremlin-python/src/main/python/radish/gremlin.py
+++ b/gremlin-python/src/main/python/radish/gremlin.py
@@ -160,8 +160,8 @@ world.gremlins = {
'g_VX1X_out_hasXid_2_3X': [(lambda g, vid3=None,vid2=None,vid1=None:g.V(vid1).out().hasId(vid2,vid3))],
'g_VX1X_out_hasXid_2AsString_3AsStringX': [(lambda g, vid3=None,vid2=None,vid1=None:g.V(vid1).out().hasId(vid2,vid3))],
'g_V_hasXblahX': [(lambda g:g.V().has('blah'))],
- 'g_EX7X_hasXlabelXknowsX': [(lambda g, eid7=None:g.E(eid7).hasLabel('knows'))],
- 'g_E_hasXlabelXknowsX': [(lambda g:g.E().hasLabel('knows'))],
+ 'g_EX7X_hasLabelXknowsX': [(lambda g, eid7=None:g.E(eid7).hasLabel('knows'))],
+ 'g_E_hasLabelXknowsX': [(lambda g:g.E().hasLabel('knows'))],
'g_E_hasLabelXuses_traversesX': [(lambda g:g.E().hasLabel('uses','traverses'))],
'g_V_hasLabelXperson_software_blahX': [(lambda g:g.V().hasLabel('person','software','blah'))],
'g_V_hasXperson_name_markoX_age': [(lambda g:g.V().has('person','name','marko').age)],
@@ -198,6 +198,15 @@ world.gremlins = {
'g_V_hasXage_gtX18X_andXltX30XXorXgtx35XXX': [(lambda g:g.V().has('age',P.gt(18).and_(P.lt(30)).or_(P.gt(35))))],
'g_V_hasXage_gtX18X_andXltX30XXorXltx35XXX': [(lambda g:g.V().has('age',P.gt(18).and_(P.lt(30)).and_(P.lt(35))))],
'g_V_hasXk_withinXcXX_valuesXkX': [(lambda g:g.addV().property('k','轉注').addV().property('k','✦').addV().property('k','♠').addV().property('k','A')), (lambda g:g.V().has('k',P.within(['轉注','✦','♠'])).k)],
+ 'g_V_hasXnullX': [(lambda g:g.V().has(None))],
+ 'g_E_hasXnullX': [(lambda g:g.E().has(None))],
+ 'g_V_hasLabelXnullX': [(lambda g:g.V().hasLabel(None))],
+ 'g_V_hasXlabel_nullX': [(lambda g:g.V().has(T.label,None))],
+ 'g_V_hasLabelXnull_nullX': [(lambda g:g.V().hasLabel(None,None))],
+ 'g_V_hasLabelXnull_personX': [(lambda g:g.V().hasLabel(None,'person'))],
+ 'g_E_hasLabelXnullX': [(lambda g:g.E().hasLabel(None))],
+ 'g_E_hasXlabel_nullX': [(lambda g:g.E().has(T.label,None))],
+ 'g_V_properties_hasLabelXnullX': [(lambda g:g.V().properties().hasLabel(None))],
'g_V_valuesXageX_isX32X': [(lambda g:g.V().age.is_(32))],
'g_V_valuesXageX_isXlte_30X': [(lambda g:g.V().age.is_(P.lte(30)))],
'g_V_valuesXageX_isXgte_29X_isXlt_34X': [(lambda g:g.V().age.is_(P.gte(29)).is_(P.lt(34)))],
diff --git a/gremlin-test/features/filter/Has.feature b/gremlin-test/features/filter/Has.feature
index d142c0e..3be3586 100644
--- a/gremlin-test/features/filter/Has.feature
+++ b/gremlin-test/features/filter/Has.feature
@@ -259,7 +259,7 @@ Feature: Step - has()
When iterated to list
Then the result should be empty
- Scenario: g_EX7X_hasXlabelXknowsX
+ Scenario: g_EX7X_hasLabelXknowsX
Given the modern graph
And using the parameter eid7 defined as "e[marko-knows->vadas].id"
And the traversal of
@@ -271,7 +271,7 @@ Feature: Step - has()
| result |
| e[marko-knows->vadas] |
- Scenario: g_E_hasXlabelXknowsX
+ Scenario: g_E_hasLabelXknowsX
Given the modern graph
And the traversal of
"""
@@ -731,4 +731,90 @@ Feature: Step - has()
| result |
| 轉注 |
| ✦ |
- | ♠ |
\ No newline at end of file
+ | ♠ |
+
+ Scenario: g_V_hasXnullX
+ Given the modern graph
+ And the traversal of
+ """
+ g.V().has(null)
+ """
+ When iterated to list
+ Then the result should be empty
+
+ Scenario: g_E_hasXnullX
+ Given the modern graph
+ And the traversal of
+ """
+ g.E().has(null)
+ """
+ When iterated to list
+ Then the result should be empty
+
+ Scenario: g_V_hasLabelXnullX
+ Given the modern graph
+ And the traversal of
+ """
+ g.V().hasLabel(null)
+ """
+ When iterated to list
+ Then the result should be empty
+
+ Scenario: g_V_hasXlabel_nullX
+ Given the modern graph
+ And the traversal of
+ """
+ g.V().has(T.label, null)
+ """
+ When iterated to list
+ Then the result should be empty
+
+ Scenario: g_V_hasLabelXnull_nullX
+ Given the modern graph
+ And the traversal of
+ """
+ g.V().hasLabel(null, null)
+ """
+ When iterated to list
+ Then the result should be empty
+
+ Scenario: g_V_hasLabelXnull_personX
+ Given the modern graph
+ And the traversal of
+ """
+ g.V().hasLabel(null, "person")
+ """
+ When iterated to list
+ Then the result should be unordered
+ | result |
+ | v[marko] |
+ | v[vadas] |
+ | v[josh] |
+ | v[peter] |
+
+ Scenario: g_E_hasLabelXnullX
+ Given the modern graph
+ And the traversal of
+ """
+ g.E().hasLabel(null)
+ """
+ When iterated to list
+ Then the result should be empty
+
+ Scenario: g_E_hasXlabel_nullX
+ Given the modern graph
+ And the traversal of
+ """
+ g.E().has(T.label, null)
+ """
+ When iterated to list
+ Then the result should be empty
+
+ Scenario: g_V_properties_hasLabelXnullX
+ Given the modern graph
+ And the traversal of
+ """
+ g.V().properties().hasLabel(null)
+ """
+ When iterated to list
+ Then the result should be empty
\ No newline at end of file
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasTest.java
index 3e2a006..77d3338 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasTest.java
@@ -29,6 +29,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
import org.apache.tinkerpop.gremlin.structure.Edge;
import org.apache.tinkerpop.gremlin.structure.Element;
import org.apache.tinkerpop.gremlin.structure.Graph;
+import org.apache.tinkerpop.gremlin.structure.Property;
import org.apache.tinkerpop.gremlin.structure.T;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.hamcrest.CoreMatchers;
@@ -41,7 +42,6 @@ import java.util.List;
import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.CREW;
import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.MODERN;
-import static org.apache.tinkerpop.gremlin.process.traversal.Contains.within;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -68,6 +68,14 @@ public abstract class HasTest extends AbstractGremlinProcessTest {
public abstract Traversal<Vertex, Vertex> get_g_V_hasXblahX();
+ public abstract Traversal<Vertex, Vertex> get_g_V_hasXnullX();
+
+ public abstract Traversal<Edge, Edge> get_g_E_hasXnullX();
+
+ public abstract Traversal<Vertex, Vertex> get_g_V_hasLabelXnullX();
+
+ public abstract Traversal<Vertex, Vertex> get_g_V_hasLabelXnull_personX();
+
public abstract Traversal<Vertex, Vertex> get_g_VX1X_hasXage_gt_30X(final Object v1Id);
public abstract Traversal<Vertex, Vertex> get_g_VXv1X_hasXage_gt_30X(final Object v1Id);
@@ -88,6 +96,10 @@ public abstract class HasTest extends AbstractGremlinProcessTest {
public abstract Traversal<Edge, Edge> get_g_E_hasLabelXknowsX();
+ public abstract Traversal<Edge, Edge> get_g_E_hasLabelXnullX();
+
+ public abstract Traversal<Vertex, ? extends Property> get_g_V_properties_hasLabelXnullX();
+
public abstract Traversal<Edge, Edge> get_g_EX11X_outV_outE_hasXid_10X(final Object e11Id, final Object e8Id);
public abstract Traversal<Edge, Edge> get_g_E_hasLabelXuses_traversesX();
@@ -339,7 +351,7 @@ public abstract class HasTest extends AbstractGremlinProcessTest {
assertTrue(traversal.hasNext());
assertThat(traversal.next().id(), CoreMatchers.anyOf(is(id2), is(id3)));
assertThat(traversal.next().id(), CoreMatchers.anyOf(is(id2), is(id3)));
- assertFalse(traversal.hasNext());
+ assertThat(traversal.hasNext(), is(false));
}
@Test
@@ -347,13 +359,46 @@ public abstract class HasTest extends AbstractGremlinProcessTest {
public void g_V_hasXblahX() {
final Traversal<Vertex, Vertex> traversal = get_g_V_hasXblahX();
printTraversalForm(traversal);
- assertFalse(traversal.hasNext());
+ assertThat(traversal.hasNext(), is(false));
+ }
+
+ @Test
+ @LoadGraphWith(MODERN)
+ public void g_V_hasXnullX() {
+ final Traversal<Vertex, Vertex> traversal = get_g_V_hasXnullX();
+ printTraversalForm(traversal);
+ assertThat(traversal.hasNext(), is(false));
}
+ @Test
+ @LoadGraphWith(MODERN)
+ public void g_E_hasXnullX() {
+ final Traversal<Edge, Edge> traversal = get_g_E_hasXnullX();
+ printTraversalForm(traversal);
+ assertThat(traversal.hasNext(), is(false));
+ }
+
+ @Test
+ @LoadGraphWith(MODERN)
+ public void g_V_hasLabelXnullX() {
+ final Traversal<Vertex, Vertex> traversal = get_g_V_hasLabelXnullX();
+ printTraversalForm(traversal);
+ assertThat(traversal.hasNext(), is(false));
+ }
+
+ @Test
+ @LoadGraphWith(MODERN)
+ public void g_V_hasLabelXnull_personX() {
+ final Traversal<Vertex, Vertex> traversal = get_g_V_hasLabelXnull_personX();
+ printTraversalForm(traversal);
+ final List<Vertex> vertices = traversal.toList();
+ assertEquals(4, vertices.size());
+ assertThat(vertices.stream().allMatch(v -> v.label().equals("person")), is(true));
+ }
@Test
@LoadGraphWith(MODERN)
- public void g_EX7X_hasXlabelXknowsX() {
+ public void g_EX7X_hasLabelXknowsX() {
final Traversal<Edge, Edge> traversal = get_g_EX7X_hasLabelXknowsX(convertToEdgeId("marko", "knows", "vadas"));
printTraversalForm(traversal);
int counter = 0;
@@ -366,7 +411,7 @@ public abstract class HasTest extends AbstractGremlinProcessTest {
@Test
@LoadGraphWith(MODERN)
- public void g_E_hasXlabelXknowsX() {
+ public void g_E_hasLabelXknowsX() {
final Traversal<Edge, Edge> traversal = get_g_E_hasLabelXknowsX();
printTraversalForm(traversal);
int counter = 0;
@@ -378,6 +423,22 @@ public abstract class HasTest extends AbstractGremlinProcessTest {
}
@Test
+ @LoadGraphWith(MODERN)
+ public void g_E_hasLabelXnullX() {
+ final Traversal<Edge, Edge> traversal = get_g_E_hasLabelXnullX();
+ printTraversalForm(traversal);
+ assertThat(traversal.hasNext(), is(false));
+ }
+
+ @Test
+ @LoadGraphWith(MODERN)
+ public void g_V_properties_hasLabelXnullX() {
+ final Traversal<Vertex, ? extends Property> traversal = get_g_V_properties_hasLabelXnullX();
+ printTraversalForm(traversal);
+ assertThat(traversal.hasNext(), is(false));
+ }
+
+ @Test
@LoadGraphWith(CREW)
public void g_E_hasLabelXuses_traversesX() {
final Traversal<Edge, Edge> traversal = get_g_E_hasLabelXuses_traversesX();
@@ -756,6 +817,31 @@ public abstract class HasTest extends AbstractGremlinProcessTest {
}
@Override
+ public Traversal<Vertex, Vertex> get_g_V_hasXnullX() {
+ return g.V().has(null);
+ }
+
+ @Override
+ public Traversal<Edge, Edge> get_g_E_hasXnullX() {
+ return g.E().has(null);
+ }
+
+ @Override
+ public Traversal<Vertex, Vertex> get_g_V_hasLabelXnullX() {
+ return g.V().hasLabel(null);
+ }
+
+ @Override
+ public Traversal<Vertex, Vertex> get_g_V_hasLabelXnull_personX() {
+ return g.V().hasLabel(null, "person");
+ }
+
+ @Override
+ public Traversal<Vertex, ? extends Property> get_g_V_properties_hasLabelXnullX() {
+ return g.V().properties().hasLabel(null);
+ }
+
+ @Override
public Traversal<Vertex, Vertex> get_g_VX1X_hasXage_gt_30X(final Object v1Id) {
return g.V(v1Id).has("age", P.gt(30));
}
@@ -806,6 +892,11 @@ public abstract class HasTest extends AbstractGremlinProcessTest {
}
@Override
+ public Traversal<Edge, Edge> get_g_E_hasLabelXnullX() {
+ return g.E().hasLabel(null);
+ }
+
+ @Override
public Traversal<Edge, Edge> get_g_E_hasLabelXuses_traversesX() {
return g.E().hasLabel("uses", "traverses");
}
diff --git a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java
index 43dcd39..89b17a1 100644
--- a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java
+++ b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java
@@ -81,24 +81,26 @@ public final class Neo4jGraphStep<S, E extends Element> extends GraphStep<S, E>
Optional<String> label = hasContainers.stream()
.filter(hasContainer -> hasContainer.getKey().equals(T.label.getAccessor()))
.filter(hasContainer -> Compare.eq == hasContainer.getBiPredicate())
+ .filter(hasContainer -> hasContainer.getValue() != null)
.map(hasContainer -> (String) hasContainer.getValue())
.findAny();
if (!label.isPresent())
label = hasContainers.stream()
.filter(hasContainer -> hasContainer.getKey().equals(T.label.getAccessor()))
.filter(hasContainer -> hasContainer.getPredicate() instanceof LabelP)
+ .filter(hasContainer -> hasContainer.getValue() != null)
.map(hasContainer -> (String) hasContainer.getValue())
.findAny();
if (label.isPresent()) {
// find a vertex by label and key/value
- String labelValue = label.get();
- Neo4jGraphAPI baseGraph = graph.getBaseGraph();
+ final String labelValue = label.get();
+ final Neo4jGraphAPI baseGraph = graph.getBaseGraph();
for (final HasContainer hasContainer : hasContainers) {
- String key = hasContainer.getKey();
- Object value = hasContainer.getValue();
+ final String key = hasContainer.getKey();
+ final Object value = hasContainer.getValue();
if (!key.equals(T.label.getAccessor()) && baseGraph.hasSchemaIndex(labelValue, key)) {
- BiPredicate<?, ?> predicate = hasContainer.getBiPredicate();
+ final BiPredicate<?, ?> predicate = hasContainer.getBiPredicate();
Iterable<Neo4jNode> nodes = null;
if (Compare.eq == predicate) {
nodes = baseGraph.findNodes(labelValue, key, value);
@@ -116,6 +118,7 @@ public final class Neo4jGraphStep<S, E extends Element> extends GraphStep<S, E>
}
}
}
+
// find a vertex by label
return IteratorUtils.stream(graph.getBaseGraph().findNodes(label.get()))
.map(node -> (Vertex) new Neo4jVertex(node, graph))