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

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

GitHub user okram opened a pull request:

    https://github.com/apache/incubator-tinkerpop/pull/267

    TINKERPOP-1219: Create a test case that ensures the provider's compilation 
of g.V(x) and g.V().hasId(x) is identical

    https://issues.apache.org/jira/browse/TINKERPOP-1219
    
    OLTP providers that have custom `GraphStep` implementations should have 
`g.V(1)` and `g.V().hasId(1)` compile to the same representation. Likewise for 
`g.V(1,2)` and `g.V().hasId(1,2)` and `g.V().has(T.id,within(1,2))`. This 
ensures that random-access databases are using not using linear scans with 
`g.V().hasId(…)`. I added `GraphStep.processHasContainerIds()` which makes it 
easy for providers to update their respective `XXXGraphStepStrategy` to 
`GraphStep.addIds()` is appropriate.
    
    I found a few `hashCode()`-bugs in the process and fixed them up. 
    
    CHANGELOG
    
    ```
    * Added `GraphStep.addIds()` which is useful for `HasContainer` "fold ins."
    * Added a static `GraphStep.processHashContainerIds()` helper for handling 
id-based `HasContainers`.
    * `GraphStep` implementations should have `g.V().hasId(x)` and `g.V(x)` 
compile equivalently. (*breaking*)
    ```
    
    UPDATE
    
    ```
    GraphStep Compilation Requirement
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
    OLTP graph providers that have a custom `GraphStep` implementation should 
ensure that `g.V().hasId(x)` and `g.V(x)` compile to the same representation. 
This ensures a consistent user experience around random access of elements 
based on ids (as opposed to potentially the former doing a linear scan). A 
static helper method called `GraphStep.processHasContainerIds()` has been 
added. `TinkerGraphStepStrategy` was updated as such:
    
    ```
    ((HasContainerHolder) 
currentStep).getHasContainers().forEach(tinkerGraphStep::addHasContainer);
    ```
    
    is now
    
    ```
    ((HasContainerHolder) currentStep).getHasContainers().forEach(hasContainer 
-> {
      if (!GraphStep.processHasContainerIds(tinkerGraphStep, hasContainer))
        tinkerGraphStep.addHasContainer(hasContainer);
    });
    ``` 

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

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

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

    https://github.com/apache/incubator-tinkerpop/pull/267.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 #267
    
----
commit 53ecadd022adf01b94fdbcfbe669c51427ebac35
Author: Marko A. Rodriguez <okramma...@gmail.com>
Date:   2016-03-16T15:11:22Z

    g.V(x) and g.V().hasId(x) should compile to the same representation for 
OLTP graph databases to ensure a consistent user experience and to ensure that 
linear scans are not being performend on the latter. A test case was added to 
ensure this in HasTest. TinkerGraphStepStrategy and Neo4jGraphStepStrategy had 
to be updated accordingly as they were, in fact, a big wacky in their 
compilation. Added GraphStep.addIds() to make easy to increase the id pool. 
Found a few hashCode() issues while testing and have fixed them up.

commit 90e5ca3f3b0d006d42894316db3c0839bd92c583
Author: Marko A. Rodriguez <okramma...@gmail.com>
Date:   2016-03-16T15:29:41Z

    Neo4jGraphStep needed a valid hashCode().

----


> Create a test case that ensures the provider's compilation of g.V(x) and 
> g.V().hasId(x) is identical
> ----------------------------------------------------------------------------------------------------
>
>                 Key: TINKERPOP-1219
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-1219
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: test-suite
>    Affects Versions: 3.1.1-incubating
>            Reporter: Mike Dias
>            Priority: Minor
>              Labels: test
>             Fix For: 3.1.2-incubating
>
>
> As discussed in this topic: 
> https://groups.google.com/forum/#!topic/gremlin-users/Kz2bOeAeqh4
> It will ensure the same behavior between g.V().hasId(id) vs. g.V(id) and 
> graph.vertices(id)



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

Reply via email to