[GitHub] tinkerpop pull request #714: TINKERPOP-1782 RangeByIsCountStrategy doesn't h...

2017-09-26 Thread asfgit 
Github user asfgit closed the pull request at:

https://github.com/apache/tinkerpop/pull/714


---

[GitHub] tinkerpop pull request #714: TINKERPOP-1782 RangeByIsCountStrategy doesn't h...

2017-09-14 Thread dkuppitz 
Github user dkuppitz commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/714#discussion_r138940544
  
--- Diff: 
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/RangeByIsCountStrategyTest.java
 ---
@@ -99,6 +99,9 @@
 {__.and(__.out().count().is(0), __.in().count().is(1)), 
__.and(__.not(__.out()), __.in().limit(2).count().is(1))},
 {__.and(__.out().count().is(1), __.in().count().is(0)), 
__.and(__.out().limit(2).count().is(1), __.not(__.in()))},
 {__.or(__.out().count().is(0), __.in().count().is(0)), 
__.or(__.not(__.out()), __.not(__.in()))},
+{__.path().filter(__.count().is(gte(0.5))).limit(5), 
__.path().identity().limit(5)}, // unfortunately we can't just remove the 
filter step
--- End diff --

It breaks other parts of the code when you attempt to modify the parent 
traversal, in particular when you remove one or more steps. I don't remember 
where the exception was thrown, but it was an `IndexOutOfBoundsException`. So 
apparently there's a `for` loop with an upper bound that is only initialized 
once (which is good, I wouldn't like to recount the steps in each iteration).


---

[GitHub] tinkerpop pull request #714: TINKERPOP-1782 RangeByIsCountStrategy doesn't h...

2017-09-13 Thread okram 
Github user okram commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/714#discussion_r138748892
  
--- Diff: 
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/RangeByIsCountStrategyTest.java
 ---
@@ -99,6 +99,9 @@
 {__.and(__.out().count().is(0), __.in().count().is(1)), 
__.and(__.not(__.out()), __.in().limit(2).count().is(1))},
 {__.and(__.out().count().is(1), __.in().count().is(0)), 
__.and(__.out().limit(2).count().is(1), __.not(__.in()))},
 {__.or(__.out().count().is(0), __.in().count().is(0)), 
__.or(__.not(__.out()), __.not(__.in()))},
+{__.path().filter(__.count().is(gte(0.5))).limit(5), 
__.path().identity().limit(5)}, // unfortunately we can't just remove the 
filter step
--- End diff --

Why can't you remove the `filter()` step? That is, why the `identity()` 
step replacement?


---

[GitHub] tinkerpop pull request #714: TINKERPOP-1782 RangeByIsCountStrategy doesn't h...

2017-09-13 Thread dkuppitz 
GitHub user dkuppitz opened a pull request:

https://github.com/apache/tinkerpop/pull/714

TINKERPOP-1782 RangeByIsCountStrategy doesn't handle floating point numbers 
properly

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

Fixed a bug in `RangeByIsCountStrategy` that led to unexpected behaviors 
when predicates were used with floating point numbers.

VOTE: +1

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/tinkerpop TINKERPOP-1782

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/714.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #714


commit 8c26a33658a69b9d9e2295662e063345eca2ae76
Author: Daniel Kuppitz 
Date:   2017-09-13T21:08:07Z

Fixed a bug in `RangeByIsCountStrategy` that led to unexpected behaviors 
when predicates were used with floating point numbers.




---