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().
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
