You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "xiazcy (via GitHub)" <gi...@apache.org> on 2023/03/22 20:50:11 UTC

[GitHub] [tinkerpop] xiazcy commented on a diff in pull request #1993: perf: improved performance of comparison (equals) between not compatible types and nulls

xiazcy commented on code in PR #1993:
URL: https://github.com/apache/tinkerpop/pull/1993#discussion_r1145394117


##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/GremlinValueComparator.java:
##########
@@ -129,8 +129,18 @@ public boolean equals(final Object f, final Object s) {
             if (containersOfDifferentSize(f, s))
                 return false;
 
+            // For Compare, NaN always produces ERROR
+            if (eitherAreNaN(f, s))
+                return false;
+
+            // For Compare we do not cross type boundaries, including null
+            if (!comparable(f, s))
+                return false;
+
             try {
-                return this.compare(f, s) == 0;
+              // comparable(f, s) assures that type(f) == type(s)

Review Comment:
   LGTM, just one quick question, do we still need this try/catch if we are no longer using the function that throws `GremlinTypeErrorException`? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org