You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2022/02/01 15:41:14 UTC

[GitHub] [tinkerpop] mikepersonick opened a new pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

mikepersonick opened a new pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557


   https://issues.apache.org/jira/browse/TINKERPOP-2695


-- 
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



[GitHub] [tinkerpop] mikepersonick commented on a change in pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
mikepersonick commented on a change in pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#discussion_r803720226



##########
File path: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/P.cs
##########
@@ -85,79 +85,79 @@ public P Or(P otherPredicate)
 
         public static P Between(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("between", value);
         }
 
         public static P Eq(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("eq", value);
         }
 
         public static P Gt(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;

Review comment:
       Florian, I've been working on a clearly defined semantics for `null` (and other values). This includes well defined behavior for comparisons against it. All described [here](https://github.com/apache/tinkerpop/blob/1b2ed0c1c3dfd54dfe9cf95bb124571f9849d8a0/docs/src/dev/provider/gremlin-semantics.asciidoc#comparability-equality-orderability-and-equivalence) and [here](https://github.com/apache/tinkerpop/blob/1b2ed0c1c3dfd54dfe9cf95bb124571f9849d8a0/docs/src/dev/provider/gremlin-semantics.asciidoc#nulltype). As such, `null` is a perfectly valid argument for the `P` predicates. It might be evaluated to an `error` once evaluated, but it should not be disallowed as an argument. .NET was the only language that was disallowing it up front, which was making testing the semantics (and true error cases) impossible.




-- 
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



[GitHub] [tinkerpop] mikepersonick commented on a change in pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
mikepersonick commented on a change in pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#discussion_r803728562



##########
File path: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/P.cs
##########
@@ -85,79 +85,79 @@ public P Or(P otherPredicate)
 
         public static P Between(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("between", value);
         }
 
         public static P Eq(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("eq", value);
         }
 
         public static P Gt(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;

Review comment:
       > `P.Gt(null)` for example doesn't look valid to me. So, I think we should ideally check for `null` here and then throw an `ArgumentNullException`.
   
   The main problem with this is that in a ternary boolean logics system, (true || error) = true. So if P.Gt(null) we connected to a predicate that evaluated to `true` we would want to evaluate the whole value expression to `true`, not throw an ArgumentNullException up front. We defer this judgment until the top level of the boolean expression and then reduce `error` to `false` if it has not been eliminated by a connective.




-- 
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



[GitHub] [tinkerpop] spmallette commented on pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#issuecomment-1028866577


   > after looking at mergeV/mergeE it seems possible that I missed some related changes necessary for the parser changes I made.
   
   it's getting harder to remember all the places where changes matter. nothing is standing out to me as glaringly missing. i would like if we didn't have to use a binding for "Infinity" in the scriptengine but we don't really have anything else like this and it seems like the easiest way to do it. 


-- 
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



[GitHub] [tinkerpop] mikepersonick commented on a change in pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
mikepersonick commented on a change in pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#discussion_r803720226



##########
File path: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/P.cs
##########
@@ -85,79 +85,79 @@ public P Or(P otherPredicate)
 
         public static P Between(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("between", value);
         }
 
         public static P Eq(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("eq", value);
         }
 
         public static P Gt(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;

Review comment:
       Florian, I've been working on a clearly defined semantics for `null`. This includes well defined behavior for comparisons against it. All described [here](https://github.com/apache/tinkerpop/blob/1b2ed0c1c3dfd54dfe9cf95bb124571f9849d8a0/docs/src/dev/provider/gremlin-semantics.asciidoc). As such, `null` is a perfectly valid argument for the `P` predicates. It might be evaluated to an `error` once evaluated, but it should not be disallowed as an argument. .NET was the only language that was disallowing it up front.




-- 
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



[GitHub] [tinkerpop] mikepersonick commented on pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
mikepersonick commented on pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#issuecomment-1028007256


   @spmallette If you wouldn't mind reviewing this one - after looking at mergeV/mergeE it seems possible that I missed some related changes necessary for the parser changes I made.


-- 
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



[GitHub] [tinkerpop] mikepersonick commented on a change in pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
mikepersonick commented on a change in pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#discussion_r803720226



##########
File path: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/P.cs
##########
@@ -85,79 +85,79 @@ public P Or(P otherPredicate)
 
         public static P Between(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("between", value);
         }
 
         public static P Eq(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("eq", value);
         }
 
         public static P Gt(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;

Review comment:
       Florian, I've been working on a clearly defined semantics for `null` (and other values). This includes well defined behavior for comparisons against it. All described [here](https://github.com/apache/tinkerpop/blob/1b2ed0c1c3dfd54dfe9cf95bb124571f9849d8a0/docs/src/dev/provider/gremlin-semantics.asciidoc). As such, `null` is a perfectly valid argument for the `P` predicates. It might be evaluated to an `error` once evaluated, but it should not be disallowed as an argument. .NET was the only language that was disallowing it up front.




-- 
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



[GitHub] [tinkerpop] mikepersonick commented on a change in pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
mikepersonick commented on a change in pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#discussion_r802762709



##########
File path: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java
##########
@@ -419,24 +426,30 @@ protected Script produceScript(final Vertex o) {
 
         @Override
         protected String getSyntax(final Number o) {
+            if (o instanceof Double || o instanceof Float) {
+                if (NumberHelper.isNaN(o))
+                    return "NaN";
+                if (NumberHelper.isPositiveInfinity(o))
+                    return "Infinity";
+                if (NumberHelper.isNegativeInfinity(o))
+                    return "-Infinity";
+
+                return o + (o instanceof Double ? "D" : "F");
+            }
             if (o instanceof Long)
                 return o + "L";
-            else if (o instanceof Double)
-                return o + "D";
-            else if (o instanceof Float)
-                return o + "F";
-            else if (o instanceof Integer)
+            if (o instanceof Integer)
                 return o + "I";
-            else if (o instanceof Byte)

Review comment:
       I was seeing a mix and match of cases with and without else so I just got rid of all of them. In this case it's fine since every case is a return statement. This would be a lot cleaner in a more modern version of Java where we could use a switch.




-- 
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



[GitHub] [tinkerpop] spmallette commented on pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#issuecomment-1033190806


   VOTE +1


-- 
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



[GitHub] [tinkerpop] mikepersonick commented on a change in pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
mikepersonick commented on a change in pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#discussion_r803720226



##########
File path: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/P.cs
##########
@@ -85,79 +85,79 @@ public P Or(P otherPredicate)
 
         public static P Between(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("between", value);
         }
 
         public static P Eq(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("eq", value);
         }
 
         public static P Gt(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;

Review comment:
       Florian, I've been working on a clearly defined semantics for `nulltype` (and other types/values). This includes well defined behavior for comparisons against it under a ternary boolean logics system. All described [here](https://github.com/apache/tinkerpop/blob/1b2ed0c1c3dfd54dfe9cf95bb124571f9849d8a0/docs/src/dev/provider/gremlin-semantics.asciidoc#comparability-equality-orderability-and-equivalence) and [here](https://github.com/apache/tinkerpop/blob/1b2ed0c1c3dfd54dfe9cf95bb124571f9849d8a0/docs/src/dev/provider/gremlin-semantics.asciidoc#nulltype). As such, `null` is a perfectly valid argument for the `P` predicates. It might be evaluated to an `error` once evaluated, but it should not be disallowed as an argument. .NET was the only language that was disallowing it up front, which was making testing the semantics (and true error cases) impossible.




-- 
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



[GitHub] [tinkerpop] mikepersonick commented on a change in pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
mikepersonick commented on a change in pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#discussion_r803728562



##########
File path: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/P.cs
##########
@@ -85,79 +85,79 @@ public P Or(P otherPredicate)
 
         public static P Between(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("between", value);
         }
 
         public static P Eq(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("eq", value);
         }
 
         public static P Gt(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;

Review comment:
       > `P.Gt(null)` for example doesn't look valid to me. So, I think we should ideally check for `null` here and then throw an `ArgumentNullException`.
   
   One problem with this is that in a ternary boolean logics system, (true || error) = true. So if P.Gt(null) were connected to a predicate that evaluated to `true` we would want to evaluate the whole value expression to `true`, not throw an ArgumentNullException up front. We defer this judgment until the top level of the boolean expression and then reduce `error` to `false` if it has not been eliminated by a connective.




-- 
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



[GitHub] [tinkerpop] FlorianHockmann commented on a change in pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
FlorianHockmann commented on a change in pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#discussion_r804526525



##########
File path: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/P.cs
##########
@@ -85,79 +85,79 @@ public P Or(P otherPredicate)
 
         public static P Between(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("between", value);
         }
 
         public static P Eq(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("eq", value);
         }
 
         public static P Gt(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;

Review comment:
       Hey Mike, sorry if this came off the wrong way! I didn't have much time on Wednesday to review this and therefore really only checked the .NET changes. So I didn't see all the work you put into comparability semantics, including handling of `null`.
   
   I just read through (most) of the added docs where you describe the improved semantics and checked the added feature tests. Now I understand why it makes sense to allow `null` here and I definitely welcome these changes! It's good to have a clear definition of how we handle infinity and `null`.
   This will also help with TINKERPOP-2348 where we want to clearly communicate to users of Gremlin.NET where `null` is allowed as an argument or a possible return value and where not. When I initially worked on that, I ran into the problem that I wasn't sure in a lot of places whether we actually want to accept `null` there or not. So, any clarifications we make on how we handle `null` in various places are definitely welcome 😄 




-- 
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



[GitHub] [tinkerpop] spmallette commented on a change in pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
spmallette commented on a change in pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#discussion_r797785016



##########
File path: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java
##########
@@ -181,24 +182,30 @@ protected String getSyntax(final TraversalOptionParent.Pick o) {
 
         @Override
         protected String getSyntax(final Number o) {

Review comment:
       if you go further down in this file you will find `LanguageTypeTranslator`  which is a `TypeTranslator` for `gremlin-language`. It's an extension of this one. I would think you need some changes there too so that we can generate inf/nan that matches the grammar.




-- 
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



[GitHub] [tinkerpop] mikepersonick commented on a change in pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
mikepersonick commented on a change in pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#discussion_r803720226



##########
File path: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/P.cs
##########
@@ -85,79 +85,79 @@ public P Or(P otherPredicate)
 
         public static P Between(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("between", value);
         }
 
         public static P Eq(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("eq", value);
         }
 
         public static P Gt(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;

Review comment:
       Florian, I've been working on a clearly defined semantics for `null` (and other values). This includes well defined behavior for comparisons against it. All described [here](https://github.com/apache/tinkerpop/blob/1b2ed0c1c3dfd54dfe9cf95bb124571f9849d8a0/docs/src/dev/provider/gremlin-semantics.asciidoc#comparability-equality-orderability-and-equivalence) and [here](https://github.com/apache/tinkerpop/blob/1b2ed0c1c3dfd54dfe9cf95bb124571f9849d8a0/docs/src/dev/provider/gremlin-semantics.asciidoc#nulltype). As such, `null` is a perfectly valid argument for the `P` predicates. It might be evaluated to an `error` once evaluated, but it should not be disallowed as an argument. .NET was the only language that was disallowing it up front.




-- 
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



[GitHub] [tinkerpop] krlawrence commented on a change in pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
krlawrence commented on a change in pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#discussion_r802734087



##########
File path: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java
##########
@@ -419,24 +426,30 @@ protected Script produceScript(final Vertex o) {
 
         @Override
         protected String getSyntax(final Number o) {
+            if (o instanceof Double || o instanceof Float) {
+                if (NumberHelper.isNaN(o))
+                    return "NaN";
+                if (NumberHelper.isPositiveInfinity(o))
+                    return "Infinity";
+                if (NumberHelper.isNegativeInfinity(o))
+                    return "-Infinity";
+
+                return o + (o instanceof Double ? "D" : "F");
+            }
             if (o instanceof Long)
                 return o + "L";
-            else if (o instanceof Double)
-                return o + "D";
-            else if (o instanceof Float)
-                return o + "F";
-            else if (o instanceof Integer)
+            if (o instanceof Integer)
                 return o + "I";
-            else if (o instanceof Byte)

Review comment:
       Just curious why the else's are removed - can "o" be more than one thing?




-- 
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



[GitHub] [tinkerpop] FlorianHockmann commented on a change in pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
FlorianHockmann commented on a change in pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#discussion_r802759384



##########
File path: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/P.cs
##########
@@ -85,79 +85,79 @@ public P Or(P otherPredicate)
 
         public static P Between(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("between", value);
         }
 
         public static P Eq(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;
             return new P("eq", value);
         }
 
         public static P Gt(params object[] args)
         {
-            var value = args.Length == 1 ? args[0] : args;
+            var value = args != null && args.Length == 1 ? args[0] : args;

Review comment:
       I've already noticed that we need to check `args` for `null` in some Gremlin steps while working on TINKERPOP-2518. What I'm wondering however: Do we really want to allow `null` everywhere, for every step and for all predicates?
   `P.Gt(null)` for example doesn't look valid to me. So, I think we should ideally check for `null` here and then throw an `ArgumentNullException`.
   
   But we need to agree on where we want to allow `null` and where not and then check that ideally already in the GLVs instead of throwing some null pointer exception on the server and return that to the user.
   
   @spmallette You've improved `null` handling in the last releases. What's your take on this?
   
   




-- 
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



[GitHub] [tinkerpop] spmallette merged pull request #1557: Inf/NaN Support for Parser/Gherkin and Comparability Feature Tests

Posted by GitBox <gi...@apache.org>.
spmallette merged pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557


   


-- 
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