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