[ 
https://issues.apache.org/jira/browse/TINKERPOP-1312?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15303051#comment-15303051
 ] 

ASF GitHub Bot commented on TINKERPOP-1312:
-------------------------------------------

Github user dkuppitz commented on a diff in the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/320#discussion_r64829983
  
    --- Diff: 
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/RangeByIsCountStrategyTest.java
 ---
    @@ -142,6 +142,7 @@ public void doTest(final Traversal traversal, final 
Traversal optimized) {
                         {__.out().count().is(without(2, 6, 4)), 
__.out().limit(6).count().is(without(2, 6, 4))},
                         {__.map(__.count().is(0)), 
__.map(__.limit(1).count().is(0))},
                         {__.flatMap(__.count().is(0)), 
__.flatMap(__.limit(1).count().is(0))},
    +                    {__.filter(__.count().is(0)), 
__.filter(__.not(__.identity()))},
    --- End diff --
    
    It is the same, but IMO it's not RangeByIsCountStrategy's job to optimize 
that.


> .count().is(0) is not properly optimized
> ----------------------------------------
>
>                 Key: TINKERPOP-1312
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-1312
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: process
>    Affects Versions: 3.2.0-incubating, 3.1.2-incubating
>            Reporter: Daniel Kuppitz
>            Assignee: Daniel Kuppitz
>             Fix For: 3.1.3, 3.2.1
>
>
> {{bla.count().is(0)}} gets optimized by {{RangeByIsCountStrategy}}, which 
> replaces it with {{bla.limit(1).count().is(0)}}. That's good, but we can do 
> even better by replacing it with {{__.not(bla)}}, which is a simple 
> {{.hasNext()}} instead of a {{RangeStep}} followed by a 
> {{ReducingBarrierStep}} ({{count()}}).
> Question is: should we do the replacement in {{RangeByIsCountStrategy}}? The 
> strategy will recognize the pattern, no matter if we use it for the 
> replacement or not; it's just that the strategy name is then no longer in 
> line with the the actual replacement (for this particular {{.count().is(0)}} 
> case) as it won't inject a {{RangeStep}}.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to