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/01/21 16:55:45 UTC

[GitHub] [tinkerpop] lvca opened a new pull request #1552: Fixed issue on comparing BigDecimal with Double/Float infinite numbers

lvca opened a new pull request #1552:
URL: https://github.com/apache/tinkerpop/pull/1552


   Since there is no such concept of "Infinite" number in BigDecimal, a comparison between a BigDecimal and a Float.INFINITE or Double.INFINITE causes a parsing exception.
   
   Example:
   
   ```java
   Vertex alice = g.addV("person").property("hair", Double.POSITIVE_INFINITY ).next();
   Vertex bob = g.addV("person").property("hair", 500 ).next();
   String query =“g.V().has('hair',500.00)”;
   ```
   
   For more information: https://github.com/ArcadeData/arcadedb/issues/289


-- 
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 #1552: Fixed issue on comparing BigDecimal with Double/Float infinite numbers

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


   VOTE +1 - i can add CHANGELOG on merge.


-- 
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 #1552: Fixed issue on comparing BigDecimal with Double/Float infinite numbers

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



##########
File path: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java
##########
@@ -255,7 +255,17 @@
                 }
                 return bigDecimalValue(b);
             },
-            (a, b) -> bigDecimalValue(a).compareTo(bigDecimalValue(b)));
+            (a, b) -> {

Review comment:
       Can you add some test cases for 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] lvca commented on pull request #1552: Fixed issue on comparing BigDecimal with Double/Float infinite numbers

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


   @divijvaidya ok, all done.


-- 
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] lvca commented on a change in pull request #1552: Fixed issue on comparing BigDecimal with Double/Float infinite numbers

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



##########
File path: gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/CompareTest.java
##########
@@ -138,6 +138,84 @@
                 {Compare.gte, big2, big1, true},
                 {Compare.lte, big2, big1, false},
         }));
+
+        // Compare bigdecimal numbers with double infinite
+        final BigDecimal big500 = new BigDecimal("500.00");
+
+        final Double doubleInfinite = Double.POSITIVE_INFINITY;
+        testCases.addAll(Arrays.asList(new Object[][]{
+            // big500 - doubleInfinite
+            {Compare.eq, big500, doubleInfinite, false},
+            {Compare.neq, big500, doubleInfinite, true},
+            {Compare.gt, big500, doubleInfinite, false},
+            {Compare.lt, big500, doubleInfinite, true},
+            {Compare.gte, big500, doubleInfinite, false},
+            {Compare.lte, big500, doubleInfinite, true},
+            // doubleInfinite - big500
+            {Compare.eq, doubleInfinite, big500, false},
+            {Compare.neq, doubleInfinite, big500, true},
+            {Compare.gt, doubleInfinite, big500, true},
+            {Compare.lt, doubleInfinite, big500, false},
+            {Compare.gte, doubleInfinite, big500, true},
+            {Compare.lte, doubleInfinite, big500, false},
+        }));
+
+        // Compare bigdecimal numbers with float infinite
+        final Float floatInfinite = Float.POSITIVE_INFINITY;
+        testCases.addAll(Arrays.asList(new Object[][]{
+            // big500 - floatInfinite
+            {Compare.eq, big500, floatInfinite, false},
+            {Compare.neq, big500, floatInfinite, true},
+            {Compare.gt, big500, floatInfinite, false},
+            {Compare.lt, big500, floatInfinite, true},
+            {Compare.gte, big500, floatInfinite, false},
+            {Compare.lte, big500, floatInfinite, true},
+            // floatInfinite - big500
+            {Compare.eq, floatInfinite, big500, false},
+            {Compare.neq, floatInfinite, big500, true},
+            {Compare.gt, floatInfinite, big500, true},
+            {Compare.lt, floatInfinite, big500, false},
+            {Compare.gte, floatInfinite, big500, true},
+            {Compare.lte, floatInfinite, big500, false},
+        }));
+
+        final Double doubleNegativeInfinite = Double.NEGATIVE_INFINITY;
+        testCases.addAll(Arrays.asList(new Object[][]{
+            // big500 - doubleNegativeInfinite
+            {Compare.eq, big500, doubleNegativeInfinite, false},
+            {Compare.neq, big500, doubleNegativeInfinite, true},
+            {Compare.gt, big500, doubleNegativeInfinite, true},
+            {Compare.lt, big500, doubleNegativeInfinite, false},
+            {Compare.gte, big500, doubleNegativeInfinite, true},
+            {Compare.lte, big500, doubleNegativeInfinite, false},
+            // doubleNegativeInfinite - big500
+            {Compare.eq, doubleNegativeInfinite, big500, false},
+            {Compare.neq, doubleNegativeInfinite, big500, true},
+            {Compare.gt, doubleNegativeInfinite, big500, false},
+            {Compare.lt, doubleNegativeInfinite, big500, true},
+            {Compare.gte, doubleNegativeInfinite, big500, false},
+            {Compare.lte, doubleNegativeInfinite, big500, true},
+        }));
+//

Review comment:
       I guess I pushed between some tests. Fixing 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 pull request #1552: Fixed issue on comparing BigDecimal with Double/Float infinite numbers

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


   Looks good +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] lvca edited a comment on pull request #1552: Fixed issue on comparing BigDecimal with Double/Float infinite numbers

Posted by GitBox <gi...@apache.org>.
lvca edited a comment on pull request #1552:
URL: https://github.com/apache/tinkerpop/pull/1552#issuecomment-1019767149


   @mikepersonick Updated the PR with the test cases. I also covered the case for negative infinite for both float and double numbers.


-- 
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] lvca commented on pull request #1552: Fixed issue on comparing BigDecimal with Double/Float infinite numbers

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


   @mikepersonick Update the PR with the test cases. I also covered the case for negative infinite for both float and double numbers.


-- 
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 #1552: Fixed issue on comparing BigDecimal with Double/Float infinite numbers

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


   


-- 
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] divijvaidya commented on pull request #1552: Fixed issue on comparing BigDecimal with Double/Float infinite numbers

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


   Thank you for the PR Luca. Looks good. We just need to fix some minor things before we can merge:
   1. Please remove the commented out part (or un-comment those tests)
   2. Please add a note to `CHANGELOG.asciidoc` about the bug fix.
   
   Could you please address the above and submit a commit? 


-- 
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 #1552: Fixed issue on comparing BigDecimal with Double/Float infinite numbers

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



##########
File path: gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/CompareTest.java
##########
@@ -138,6 +138,84 @@
                 {Compare.gte, big2, big1, true},
                 {Compare.lte, big2, big1, false},
         }));
+
+        // Compare bigdecimal numbers with double infinite
+        final BigDecimal big500 = new BigDecimal("500.00");
+
+        final Double doubleInfinite = Double.POSITIVE_INFINITY;
+        testCases.addAll(Arrays.asList(new Object[][]{
+            // big500 - doubleInfinite
+            {Compare.eq, big500, doubleInfinite, false},
+            {Compare.neq, big500, doubleInfinite, true},
+            {Compare.gt, big500, doubleInfinite, false},
+            {Compare.lt, big500, doubleInfinite, true},
+            {Compare.gte, big500, doubleInfinite, false},
+            {Compare.lte, big500, doubleInfinite, true},
+            // doubleInfinite - big500
+            {Compare.eq, doubleInfinite, big500, false},
+            {Compare.neq, doubleInfinite, big500, true},
+            {Compare.gt, doubleInfinite, big500, true},
+            {Compare.lt, doubleInfinite, big500, false},
+            {Compare.gte, doubleInfinite, big500, true},
+            {Compare.lte, doubleInfinite, big500, false},
+        }));
+
+        // Compare bigdecimal numbers with float infinite
+        final Float floatInfinite = Float.POSITIVE_INFINITY;
+        testCases.addAll(Arrays.asList(new Object[][]{
+            // big500 - floatInfinite
+            {Compare.eq, big500, floatInfinite, false},
+            {Compare.neq, big500, floatInfinite, true},
+            {Compare.gt, big500, floatInfinite, false},
+            {Compare.lt, big500, floatInfinite, true},
+            {Compare.gte, big500, floatInfinite, false},
+            {Compare.lte, big500, floatInfinite, true},
+            // floatInfinite - big500
+            {Compare.eq, floatInfinite, big500, false},
+            {Compare.neq, floatInfinite, big500, true},
+            {Compare.gt, floatInfinite, big500, true},
+            {Compare.lt, floatInfinite, big500, false},
+            {Compare.gte, floatInfinite, big500, true},
+            {Compare.lte, floatInfinite, big500, false},
+        }));
+
+        final Double doubleNegativeInfinite = Double.NEGATIVE_INFINITY;
+        testCases.addAll(Arrays.asList(new Object[][]{
+            // big500 - doubleNegativeInfinite
+            {Compare.eq, big500, doubleNegativeInfinite, false},
+            {Compare.neq, big500, doubleNegativeInfinite, true},
+            {Compare.gt, big500, doubleNegativeInfinite, true},
+            {Compare.lt, big500, doubleNegativeInfinite, false},
+            {Compare.gte, big500, doubleNegativeInfinite, true},
+            {Compare.lte, big500, doubleNegativeInfinite, false},
+            // doubleNegativeInfinite - big500
+            {Compare.eq, doubleNegativeInfinite, big500, false},
+            {Compare.neq, doubleNegativeInfinite, big500, true},
+            {Compare.gt, doubleNegativeInfinite, big500, false},
+            {Compare.lt, doubleNegativeInfinite, big500, true},
+            {Compare.gte, doubleNegativeInfinite, big500, false},
+            {Compare.lte, doubleNegativeInfinite, big500, true},
+        }));
+//

Review comment:
       Why commented out?




-- 
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] divijvaidya commented on pull request #1552: Fixed issue on comparing BigDecimal with Double/Float infinite numbers

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


   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 #1552: Fixed issue on comparing BigDecimal with Double/Float infinite numbers

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



##########
File path: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java
##########
@@ -255,7 +255,17 @@
                 }
                 return bigDecimalValue(b);
             },
-            (a, b) -> bigDecimalValue(a).compareTo(bigDecimalValue(b)));
+            (a, b) -> {

Review comment:
       https://github.com/apache/tinkerpop/blob/3.5-dev/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/CompareTest.java#L118




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