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/01/07 18:40:12 UTC

[tinkerpop] branch TINKERPOP-2481 updated: TINKERPOP-2499 match() returns more consistent results

This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a commit to branch TINKERPOP-2481
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git


The following commit(s) were added to refs/heads/TINKERPOP-2481 by this push:
     new ea2bf7e  TINKERPOP-2499 match() returns more consistent results
ea2bf7e is described below

commit ea2bf7e2ad70566dd03560ca89e42ca693d36f6e
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Thu Jan 7 13:36:49 2021 -0500

    TINKERPOP-2499 match() returns more consistent results
    
    Basically sacrificed some optimization from PathRetractionStrategy to ensure users aren't hit with inconsistent traversal results. This approach also avoids situations where one strategy allows another to work while otherwise allowing for different query results if no strategies were applied. For instance, IdentityRemovalStrategy would let match().identity() return all labels but match().unfold() would be an empty result. This approach is much more consistent. If we could implement TI [...]
---
 CHANGELOG.asciidoc                                 |  1 +
 docs/src/upgrade/release-3.5.x.asciidoc            | 62 ++++++++++++++++++++++
 .../optimization/PathRetractionStrategy.java       | 41 +++++++++-----
 .../gremlin/process/traversal/util/PathUtil.java   |  1 -
 .../optimization/PathRetractionStrategyTest.java   | 37 +++++++++----
 .../gremlin-javascript/test/cucumber/gremlin.js    |  1 +
 gremlin-python/src/main/python/radish/gremlin.py   |  1 +
 .../driver/ClientConnectionIntegrateTest.java      |  4 +-
 gremlin-test/features/map/Match.feature            | 11 ++++
 .../process/traversal/step/map/MatchTest.java      | 17 ++++++
 .../tinkergraph/structure/TinkerGraphTest.java     | 15 ++++++
 11 files changed, 166 insertions(+), 25 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 4af15ba..4b684f8 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -40,6 +40,7 @@ This release also includes changes from <<release-3-4-3, 3.4.3>>.
 * Added `ByModulatorOptimizationStrategy` which replaces certain standard traversals w/ optimized traversals (e.g. `TokenTraversal`).
 * Improved `IdentityRemovalStrategy` by accounting for `EndStep` situations.
 * Added `IdentityRemovalStrategy` to the standard list of `TraversalStrategies`.
+* Modified `PathRetractionStrategy` to leave labels more often with `match()` cases to return more consistent results.
 * Refactored `MapStep` to move its logic to `ScalarMapStep` so that the old behavior could be preserved while allow other implementations to have more flexibility.
 * Modified TinkerGraph to support `null` property values and can be configured to disable that feature.
 * Modified `null` handling in mutations to be consistent for a new `Vertex` as well as update to an existing one.
diff --git a/docs/src/upgrade/release-3.5.x.asciidoc b/docs/src/upgrade/release-3.5.x.asciidoc
index 95bf6fc..01296aa 100644
--- a/docs/src/upgrade/release-3.5.x.asciidoc
+++ b/docs/src/upgrade/release-3.5.x.asciidoc
@@ -435,6 +435,68 @@ configuration options may have changed.
 * Experimental support for multi/meta-properties in Neo4j which were previously deprecated have now been permanently
 removed.
 
+==== match() Consistency
+
+The `match()` step behavior might have seemed inconsistent those first using it. While there are a number of examples
+that might demonstrate this issue, the easiest one to consume would be:
+
+[source,text]
+----
+gremlin> g.V().match(__.as("a").out("knows").as("b"))
+==>[a:v[1],b:v[2]]
+==>[a:v[1],b:v[4]]
+gremlin> g.V().match(__.as("a").out("knows").as("b")).unfold()
+gremlin> g.V().match(__.as("a").out("knows").as("b")).identity()
+==>[]
+==>[]
+----
+
+The output is unexpected if there isn't awareness of some underlying optimizations at play, where `match()` as the
+final step in the traversal implies that the user wants all of the labels as part of the output. With the addition
+of the extra steps, `unfold()` and `identity()` in the above case, the implication is that the traversal must be
+explicit in the labels to preserve from match, thus:
+
+[source,text]
+----
+gremlin> g.V().match(__.as("a").out("knows").as("b")).select('a','b').unfold()
+==>a=v[1]
+==>b=v[2]
+==>a=v[1]
+==>b=v[4]
+gremlin> g.V().match(__.as("a").out("knows").as("b")).select('a','b').identity()
+==>[a:v[1],b:v[2]]
+==>[a:v[1],b:v[4]]
+----
+
+Being explicit, as is the preference in writing Gremlin to good form, helps restrict the path history required to
+execute the traversal and therefore preserves memory. Of course, making `match()` a special form of end step is a
+confusing approach as the behavior of the step changes simply because another step is in play. Furthermore, correct
+execution of the traversal, relies on the execution of traversal strategies when the same traversal should produce
+the same results irrespective of the strategies applied to it.
+
+In 3.5.0, we look to better adhere to that guiding design principle and ensure a more consistent output for these types
+of traversals. While the preferred method is to specify the labels to preserve from `match()` with a following
+`select()` step as shown above, `match()` will now consistently return all labels when they are not specified
+explicitly.
+
+[source,text]
+----
+gremlin> g.V().match(__.as("a").out("knows").as("b"))
+==>[a:v[1],b:v[2]]
+==>[a:v[1],b:v[4]]
+gremlin> g.V().match(__.as("a").out("knows").as("b")).identity()
+==>[a:v[1],b:v[2]]
+==>[a:v[1],b:v[4]]
+gremlin> g.V().match(__.as("a").out("knows").as("b")).unfold()
+==>a=v[1]
+==>b=v[2]
+==>a=v[1]
+==>b=v[4]
+----
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-2481[TINKERPOP-2481],
+link:https://issues.apache.org/jira/browse/TINKERPOP-2499[TINKERPOP-2499]
+
 ==== Deprecation Removal
 
 The following deprecated classes, methods or fields have been removed in this version:
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java
index 62ade2b..fc6cc7c 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java
@@ -33,6 +33,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.filter.NoneStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.MatchStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.NoOpBarrierStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.SelectOneStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.map.SelectStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.EmptyStep;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
@@ -76,14 +77,7 @@ public final class PathRetractionStrategy extends AbstractTraversalStrategy<Trav
 
     @Override
     public void apply(final Traversal.Admin<?, ?> traversal) {
-        // do not apply this strategy if there are lambdas as you can't introspect to know what path information the lambdas are using
-        // do not apply this strategy if a PATH requirement step is being used (in the future, we can do PATH requirement lookhead to be more intelligent about its usage)
-        // do not apply this strategy if a VertexProgramStep is present with LABELED_PATH requirements
-        if (traversal.isRoot() &&
-                TraversalHelper.anyStepRecursively(step -> step instanceof LambdaHolder ||
-                        step.getRequirements().contains(TraverserRequirement.PATH) ||
-                        (step instanceof VertexProgramStep &&
-                                step.getRequirements().contains(TraverserRequirement.LABELED_PATH)), traversal)) {
+        if (traversal.isRoot() && isNotApplicable(traversal)) {
             TraversalHelper.applyTraversalRecursively(t -> t.getEndStep().addLabel(MARKER), traversal);
         }
 
@@ -113,10 +107,16 @@ public final class PathRetractionStrategy extends AbstractTraversalStrategy<Trav
             // add the keep labels to the path processor
             if (currentStep instanceof PathProcessor) {
                 final PathProcessor pathProcessor = (PathProcessor) currentStep;
-                if (currentStep instanceof MatchStep &&
-                        (currentStep.getNextStep().equals(EmptyStep.instance()) ||
-                                currentStep.getNextStep() instanceof DedupGlobalStep ||
-                                currentStep.getNextStep() instanceof SelectOneStep && currentStep.getNextStep().getNextStep() instanceof FilterStep)) {
+                // in versions prior to 3.5.0 we use to only keep labels if match() was last
+                // or was followed by dedup() or select().where(), but the pattern matching
+                // wasn't too smart and thus produced inconsistent/unexpected outputs. trying
+                // to make gremlin be a bit less surprising for users and ultimately this seemed
+                // like too much magic. finally, we really shouldn't rely have strategies relying
+                // to heavily on one another for results to be consistent. a traversal should
+                // produce the same results irrespective of zero, one or more strategies being
+                // applied. so, for now this change adds back a bit of overhead in managing some
+                // labels but produces results users expect.
+                if (currentStep instanceof MatchStep) {
                     pathProcessor.setKeepLabels(((MatchStep) currentStep).getMatchStartLabels());
                     pathProcessor.getKeepLabels().addAll(((MatchStep) currentStep).getMatchEndLabels());
                 } else {
@@ -219,7 +219,7 @@ public final class PathRetractionStrategy extends AbstractTraversalStrategy<Trav
             keeperTrail.addAll(levelLabels);
         }
 
-        for (final Step currentStep : traversal.getSteps()) {
+        for (final Step<?,?> currentStep : traversal.getSteps()) {
             // go back through current level and add all keepers
             // if there is one more RepeatSteps in this traversal's lineage, preserve keep labels
             if (currentStep instanceof PathProcessor) {
@@ -230,6 +230,21 @@ public final class PathRetractionStrategy extends AbstractTraversalStrategy<Trav
         }
     }
 
+    /**
+     * Determines if the strategy should be applied or not. It returns {@code true} and is "not applicable" when the
+     * following conditions are met:
+     * <ul>
+     *     <li>If there are lambdas as you can't introspect to know what path information the lambdas are using</li>
+     *     <li>If a PATH requirement step is being used (in the future, we can do PATH requirement lookhead to be more intelligent about its usage)</li>
+     *     <li>If a VertexProgramStep is present with LABELED_PATH requirements</li>
+     * </ul>
+     */
+    private static boolean isNotApplicable(final Traversal.Admin<?, ?> traversal) {
+        return TraversalHelper.anyStepRecursively(step -> step instanceof LambdaHolder ||
+                step.getRequirements().contains(TraverserRequirement.PATH) ||
+                (step instanceof VertexProgramStep && step.getRequirements().contains(TraverserRequirement.LABELED_PATH)), traversal);
+    }
+
     private void applyToChildren(final Set<String> keepLabels, final List<Traversal.Admin<Object, Object>> children) {
         for (final Traversal.Admin<Object, Object> child : children) {
             TraversalHelper.applyTraversalRecursively(trav -> addLabels(trav, keepLabels), child);
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/PathUtil.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/PathUtil.java
index cefc62a..8dafa5d 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/PathUtil.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/PathUtil.java
@@ -58,7 +58,6 @@ public class PathUtil {
                 }
             }
             referencedLabels.addAll(labels);
-
         }
 
         return referencedLabels;
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java
index 0669b08..3a82653 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java
@@ -91,14 +91,15 @@ public class PathRetractionStrategyTest {
             final Traversal.Admin<?, ?> currentTraversal = this.traversal.clone();
             currentTraversal.setStrategies(currentStrategies);
             currentTraversal.applyStrategies();
-            assertEquals(this.labels, getKeepLabels(currentTraversal).toString());
+            assertEquals("Using Strategies: " + currentStrategies.toString(),
+                    this.labels, getKeepLabels(currentTraversal).toString());
             if (null != optimized)
                 assertEquals(currentTraversal, optimized);
         }
     }
 
     private List<Object> getKeepLabels(final Traversal.Admin<?, ?> traversal) {
-        List<Object> keepLabels = new ArrayList<>();
+        final List<Object> keepLabels = new ArrayList<>();
         for (Step step : traversal.getSteps()) {
             if (step instanceof PathProcessor) {
                 final Set<String> keepers = ((PathProcessor) step).getKeepLabels();
@@ -139,11 +140,26 @@ public class PathRetractionStrategyTest {
                 {__.V().as("a").out().where(neq("a")).out().select("a"), "[[a], []]", null},
                 {__.V().as("a").out().as("b").where(neq("a")).out().select("a", "b").out().select("b"), "[[a, b], [b], []]", null},
                 {__.V().match(as("a").out().as("b")), "[[a, b]]", null},
-                {__.V().match(as("a").out().as("b")).select("a"), "[[a], []]", null},
+                {__.V().match(as("a").out().as("b")).dedup(), "[[a, b], []]", null},
+                {__.V().match(as("a").out().as("b")).identity().dedup().identity(), "[[a, b], []]", null},
+                // MatchPredicateStrategy folds the dedup("a") into the match() so the step labels don't assert
+                // cleanly here given limitations of the suite that don't let us ignore a particular run. so the
+                // following will assert with "[[a, b], []]" but also "[[a, b]]" when MatchPredicateStrategy does
+                // its thing:
+                // {__.V().match(as("a").out("knows").as("b")).dedup("a"), "[[a, b], []]", null},
+                // would be nice if we could better detect dedup("a") with some form of look-ahead. note that
+                // MatchPredicateStrategy misses this pattern because of identity()
+                {__.V().match(as("a").out().as("b")).identity().dedup("a").identity(), "[[a, b], []]", null},
+                {__.V().match(as("a").out().as("b")).identity(), "[[a, b]]", null},
+                {__.V().match(as("a").out().as("b")).unfold(), "[[a, b]]", null},
+                {__.V().match(as("a").out().as("b")).unfold().identity(), "[[a, b]]", null},
+                // would be nice if we could better detect select("a") with some form of look-ahead
+                {__.V().match(as("a").out().as("b")).select("a"), "[[a, b], []]", null},
+                // would be nice to detect and retract "a" and "c" earlier
                 {__.V().out().out().match(
                         as("a").in("created").as("b"),
                         as("b").in("knows").as("c")).select("c").out("created").where(neq("a")).values("name"),
-                        "[[a, c], [a], []]", null},
+                        "[[a, b, c], [a], []]", null},
                 {__.V().as("a").out().select("a").path(), PATH_RETRACTION_STRATEGY_DISABLED, null},
                 {__.V().as("a").out().select("a").map(t -> t.path().get("a")), PATH_RETRACTION_STRATEGY_DISABLED, null}, // lambda introspection is not possible
                 {__.V().as("a").out().select("a").subgraph("b"), "[[]]", null},
@@ -158,15 +174,17 @@ public class PathRetractionStrategyTest {
                         "[[[a, b]], [b], []]", null},
                 {__.outE().inV().group().by(__.inE().outV().groupCount().by(__.both().count().is(P.gt(2)))), "[]", null},
                 {__.V().as("a").repeat(out().where(neq("a"))).emit().select("a").values("test"), "[[[a]], []]", null},
-                // given the way this test harness is structured, I have to manual test for RepeatUnrollStrategy (and it works) TODO: add more test parameters
+                // given the way this test harness is structured, I have to manual test for RepeatUnrollStrategy (and it works)
                 // {__.V().as("a").repeat(__.out().where(neq("a"))).times(3).select("a").values("test"), Arrays.asList(Collections.singleton("a"), Collections.singleton("a"), Collections.singleton("a"), Collections.emptySet())}
                 {__.V().as("a").out().as("b").select("a").out().out(), "[[]]", __.V().as("a").out().as("b").select("a").barrier(MAX_BARRIER_SIZE).out().out()},
                 {__.V().as("a").out().as("b").select("a").count(), "[[]]", __.V().as("a").out().as("b").select("a").count()},
                 {__.V().as("a").out().as("b").select("a").barrier().count(), "[[]]", __.V().as("a").out().as("b").select("a").barrier().count()},
                 {__.V().as("a").out().as("b").dedup("a", "b").out(), "[[]]", __.V().as("a").out().as("b").dedup("a", "b").out()},
                 {__.V().as("a").out().as("b").match(as("a").out().as("b")), "[[a, b]]", __.V().as("a").out().as("b").match(as("a").out().as("b"))},
-                {__.V().as("a").out().as("b").match(as("a").out().as("b")).select("a"), "[[a], []]", __.V().as("a").out().as("b").match(as("a").out().as("b")).select("a")},
-                {__.V().as("a").out().as("b").match(as("a").out().as("b")).select("a").out().dedup("a"), "[[a], [a], []]", __.V().as("a").out().as("b").match(as("a").out().as("b")).select("a").barrier(MAX_BARRIER_SIZE).out().dedup("a")},
+                // would be nice if we could better detect select("a") with some form of look-ahead
+                {__.V().as("a").out().as("b").match(as("a").out().as("b")).select("a"), "[[a, b], []]", __.V().as("a").out().as("b").match(as("a").out().as("b")).select("a")},
+                // would be nice if we could better detect select("a")/dedup("a") with some form of look-ahead
+                {__.V().as("a").out().as("b").match(as("a").out().as("b")).select("a").out().dedup("a"), "[[a, b], [a], []]", __.V().as("a").out().as("b").match(as("a").out().as("b")).select("a").barrier(MAX_BARRIER_SIZE).out().dedup("a")},
                 {__.V().as("a").out().as("b").where(P.gt("a")).out().out(), "[[]]", __.V().as("a").out().as("b").where(P.gt("a")).barrier(MAX_BARRIER_SIZE).out().out()},
                 {__.V().as("a").out().as("b").where(P.gt("a")).count(), "[[]]", __.V().as("a").out().as("b").where(P.gt("a")).count()},
                 {__.V().as("a").out().as("b").select("a").as("c").where(P.gt("b")).out(), "[[b], []]", __.V().as("a").out().as("b").select("a").as("c").barrier(MAX_BARRIER_SIZE).where(P.gt("b")).barrier(MAX_BARRIER_SIZE).out()},
@@ -181,9 +199,8 @@ public class PathRetractionStrategyTest {
                         local(as("c").out().as("d", "e").select("c", "e").out().select("c")).
                         out().select("c"),
                         "[[b, c, e], [c, e], [[c], [c]], []]", null},
-                // TODO: same as above but note how path() makes things react
-//                {__.V().as("a").out().as("b").select("a").select("b").path().local(as("c").out().as("d", "e").select("c", "e").out().select("c")).out().select("c"),
-//                        "[[[c, e], [c, e]]]", null},
+                {__.V().as("a").out().as("b").select("a").select("b").path().local(as("c").out().as("d", "e").select("c", "e").out().select("c")).out().select("c"),
+                        PATH_RETRACTION_STRATEGY_DISABLED, null},
                 {__.V().as("a").out().as("b").select("a").select("b").repeat(out().as("c").select("b", "c").out().select("c")).out().select("c").out().select("b"),
                         "[[b, c], [b, c], [[b, c], [b, c]], [b], []]", null},
                 {__.V().as("a").out().as("b").select("a").select("b").repeat(out().as("c").select("b")).out().select("c").out().select("b"),
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 b69bb3b..43545bd 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
@@ -391,6 +391,7 @@ const gremlins = {
     g_V_matchXa_followedBy_count_isXgtX10XX_b__a_0followedBy_count_isXgtX10XX_bX_count: [function({g}) { return g.V().match(__.as("a").out("followedBy").count().is(P.gt(10)).as("b"),__.as("a").in_("followedBy").count().is(P.gt(10)).as("b")).count() }], 
     g_V_matchXa_0sungBy_b__a_0writtenBy_c__b_writtenBy_dX_whereXc_sungBy_dX_whereXd_hasXname_GarciaXX: [function({g}) { return g.V().match(__.as("a").in_("sungBy").as("b"),__.as("a").in_("writtenBy").as("c"),__.as("b").out("writtenBy").as("d")).where(__.as("c").out("sungBy").as("d")).where(__.as("d").has("name","Garcia")) }], 
     g_V_matchXa_hasXname_GarciaX__a_0writtenBy_b__b_followedBy_c__c_writtenBy_d__whereXd_neqXaXXX: [function({g}) { return g.V().match(__.as("a").has("name","Garcia"),__.as("a").in_("writtenBy").as("b"),__.as("b").out("followedBy").as("c"),__.as("c").out("writtenBy").as("d"),__.where("d",P.neq("a"))) }], 
+    g_V_matchXa_outXknowsX_name_bX_identity: [function({g}) { return g.V().match(__.as("a").out("knows").values("name").as("b")).identity() }], 
     g_V_outE_mathX0_minus_itX_byXweightX: [function({g}) { return g.V().outE().math("0-_").by("weight") }], 
     g_V_hasXageX_valueMap_mathXit_plus_itXbyXselectXageX_unfoldXX: [function({g}) { return g.V().has("age").valueMap().math("_+_").by(__.select("age").unfold()) }], 
     g_V_asXaX_outXknowsX_asXbX_mathXa_plus_bX_byXageX: [function({g}) { return g.V().as("a").out("knows").as("b").math("a + b").by("age") }], 
diff --git a/gremlin-python/src/main/python/radish/gremlin.py b/gremlin-python/src/main/python/radish/gremlin.py
index f2586e4..5e16f57 100644
--- a/gremlin-python/src/main/python/radish/gremlin.py
+++ b/gremlin-python/src/main/python/radish/gremlin.py
@@ -376,6 +376,7 @@ world.gremlins = {
     'g_V_matchXa_followedBy_count_isXgtX10XX_b__a_0followedBy_count_isXgtX10XX_bX_count': [(lambda g:g.V().match(__.as_('a').out('followedBy').count().is_(P.gt(10)).as_('b'),__.as_('a').in_('followedBy').count().is_(P.gt(10)).as_('b')).count())], 
     'g_V_matchXa_0sungBy_b__a_0writtenBy_c__b_writtenBy_dX_whereXc_sungBy_dX_whereXd_hasXname_GarciaXX': [(lambda g:g.V().match(__.as_('a').in_('sungBy').as_('b'),__.as_('a').in_('writtenBy').as_('c'),__.as_('b').out('writtenBy').as_('d')).where(__.as_('c').out('sungBy').as_('d')).where(__.as_('d').has('name','Garcia')))], 
     'g_V_matchXa_hasXname_GarciaX__a_0writtenBy_b__b_followedBy_c__c_writtenBy_d__whereXd_neqXaXXX': [(lambda g:g.V().match(__.as_('a').has('name','Garcia'),__.as_('a').in_('writtenBy').as_('b'),__.as_('b').out('followedBy').as_('c'),__.as_('c').out('writtenBy').as_('d'),__.where('d',P.neq('a'))))], 
+    'g_V_matchXa_outXknowsX_name_bX_identity': [(lambda g:g.V().match(__.as_('a').out('knows').name.as_('b')).identity())], 
     'g_V_outE_mathX0_minus_itX_byXweightX': [(lambda g:g.V().outE().math('0-_').by('weight'))], 
     'g_V_hasXageX_valueMap_mathXit_plus_itXbyXselectXageX_unfoldXX': [(lambda g:g.V().has('age').valueMap().math('_+_').by(__.select('age').unfold()))], 
     'g_V_asXaX_outXknowsX_asXbX_mathXa_plus_bX_byXageX': [(lambda g:g.V().as_('a').out('knows').as_('b').math('a + b').by('age'))], 
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/ClientConnectionIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/ClientConnectionIntegrateTest.java
index ff58cd4..ce6b4cb 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/ClientConnectionIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/ClientConnectionIntegrateTest.java
@@ -71,7 +71,7 @@ public class ClientConnectionIntegrateTest extends AbstractGremlinServerIntegrat
     public void shouldCloseConnectionDeadDueToUnRecoverableError() throws Exception {
         // Set a low value of maxContentLength to intentionally trigger CorruptedFrameException
         final Cluster cluster = TestClientFactory.build()
-                                                 .serializer(Serializers.GRYO_V3D0)
+                                                 .serializer(Serializers.GRAPHBINARY_V1D0)
                                                  .maxContentLength(64)
                                                  .minConnectionPoolSize(1)
                                                  .maxConnectionPoolSize(2)
@@ -89,6 +89,8 @@ public class ClientConnectionIntegrateTest extends AbstractGremlinServerIntegrat
                 assertThat(re.getCause() instanceof CorruptedFrameException, is(true));
             }
 
+            Thread.sleep(3000);
+
             // Assert that the host has not been marked unavailable
             assertEquals(1, cluster.availableHosts().size());
 
diff --git a/gremlin-test/features/map/Match.feature b/gremlin-test/features/map/Match.feature
index d0dee12..fc60608 100644
--- a/gremlin-test/features/map/Match.feature
+++ b/gremlin-test/features/map/Match.feature
@@ -540,3 +540,14 @@ Feature: Step - match()
       | m[{"a":"v[Garcia]","b":"v[CRYPTICAL ENVELOPMENT]","c":"v[THE OTHER ONE]","d":"v[Weir]"}] |
       | m[{"a":"v[Garcia]","b":"v[CRYPTICAL ENVELOPMENT]","c":"v[WHARF RAT]","d":"v[Hunter]"}] |
 
+  Scenario: g_V_matchXa_outXknowsX_name_bX_identity
+    Given the modern graph
+    And the traversal of
+      """
+      g.V().match(__.as("a").out("knows").values("name").as("b")).identity()
+      """
+    When iterated to list
+    Then the result should be unordered
+      | result |
+      | m[{"a":"v[marko]","b":"vadas"}] |
+      | m[{"a":"v[marko]","b":"josh"}] |
\ No newline at end of file
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchTest.java
index 965def1..a5a7595 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchTest.java
@@ -69,6 +69,8 @@ public abstract class MatchTest extends AbstractGremlinProcessTest {
 
     public abstract Traversal<Vertex, Map<String, Object>> get_g_V_valueMap_matchXa_selectXnameX_bX();
 
+    public abstract Traversal<Vertex, Map<String, Object>> get_g_V_matchXa_outXknowsX_name_bX_identity();
+
     // very basic query
     public abstract Traversal<Vertex, Map<String, Vertex>> get_g_V_matchXa_out_bX();
 
@@ -608,6 +610,16 @@ public abstract class MatchTest extends AbstractGremlinProcessTest {
                 "a", "josh", "b", "ripple"), traversal);
     }
 
+    @Test
+    @LoadGraphWith(MODERN)
+    public void g_V_matchXa_outXknowsX_name_bX_identity() {
+        final Traversal<Vertex, Map<String, Object>> traversal = get_g_V_matchXa_outXknowsX_name_bX_identity();
+        printTraversalForm(traversal);
+        checkResults(makeMapList(2,
+                "a", convertToVertex(graph, "marko"), "b", "vadas",
+                "a", convertToVertex(graph, "marko"), "b", "josh"), traversal);
+    }
+
     public static class GreedyMatchTraversals extends Traversals {
         @Before
         public void setupTest() {
@@ -627,6 +639,11 @@ public abstract class MatchTest extends AbstractGremlinProcessTest {
         }
 
         @Override
+        public Traversal<Vertex, Map<String, Object>> get_g_V_matchXa_outXknowsX_name_bX_identity() {
+            return g.V().match(__.as("a").out("knows").values("name").as("b")).identity();
+        }
+
+        @Override
         public Traversal<Vertex, Map<String, Vertex>> get_g_V_matchXa_out_bX() {
             return g.V().match(as("a").out().as("b"));
         }
diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java
index 555c7ca..baa84a6 100644
--- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java
+++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java
@@ -26,6 +26,7 @@ import org.apache.tinkerpop.gremlin.process.computer.Computer;
 import org.apache.tinkerpop.gremlin.process.traversal.P;
 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.optimization.IdentityRemovalStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ReservedKeysVerificationStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.util.Metrics;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalMetrics;
@@ -76,6 +77,7 @@ import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
+import static org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource.traversal;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.is;
@@ -712,6 +714,19 @@ public class TinkerGraphTest {
         }
     }
 
+    @Test
+    public void shouldWorkWithoutIdentityStrategy() {
+        final Graph graph = TinkerFactory.createModern();
+        final GraphTraversalSource g = traversal().withEmbedded(graph).withoutStrategies(IdentityRemovalStrategy.class);
+        final List<Map<String,Object>> result = g.V().match(__.as("a").out("knows").values("name").as("b")).identity().toList();
+        assertEquals(2, result.size());
+        result.stream().forEach(m -> {
+            assertEquals(2, m.size());
+            assertThat(m.containsKey("a"), is(true));
+            assertThat(m.containsKey("b"), is(true));
+        });
+    }
+
     /**
      * Coerces a {@code Color} to a {@link TinkerGraph} during serialization.  Demonstrates how custom serializers
      * can be developed that can coerce one value to another during serialization.