[
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 <[email protected]>
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 <[email protected]>
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)
