[
https://issues.apache.org/jira/browse/TINKERPOP-1502?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15667449#comment-15667449
]
ASF GitHub Bot commented on TINKERPOP-1502:
-------------------------------------------
GitHub user okram opened a pull request:
https://github.com/apache/tinkerpop/pull/495
TINKERPOP-1502 & TINKERPOP-1482: Chained has-containers and AndP stringency
https://issues.apache.org/jira/browse/TINKERPOP-1502
https://issues.apache.org/jira/browse/TINKERPOP-1482
This ticket accomplishes various things around `GraphTraversal.has()`.
1.) A write-time optimization is provided where if a `HasStep` is the
traversal's end step, simply `HasStep.addHasContainer()`. This reduces the time
required by `InlineFilterStrategy` and provider strategies to fold in
`HasContainers` and remove `HasSteps`.
2.) `GraphTraversal.hasXXX()` was tweaked by @dkuppitz a release ago. He
used `Object` to determine numerous things like `String`, `P`, etc. This is
bad. We want the `GraphTraversal` API to be explicit so I added stricter
typing: e.g. `hasLabel(String,String...)` and `hasLabel(P<String>)`.
3.) `HasContainer.makeHasContainers()` was a TinkerPop hack trying to help
providers and it shouldn't be done. This method turned `AndP(x,y)` into
individual `x` and `y` predicates so providers didn't have to analyze `AndP`
trees. We shouldn't do this as the provider will be getting `AndP`
compositions elsewhere and will need to be able to handle it. I updated
`Neo4jGraphStep` and `TinkerGraphStep` to process `AndP` predicates accordingly
for index lookups. Added a note to upgrade docs for other providers --- simple
update if they want to use the old "break apart `AndP`"-model.
4.) Found a bug in `InlineFilterStrategy` around `outE().hasLabel()` where
`outE('knows').hasLabel('created')` was turned into `outE('knows','created')`
and thus, semantically incorrect.
Going to add a few more tests before VOTING.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1502
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/495.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 #495
----
commit 83f2ddb6629eab9c4dca8c4103dce60a3b8fc7a6
Author: Marko A. Rodriguez <okramma...@gmail.com>
Date: 2016-11-14T19:54:07Z
first push on 'left-append' of has() containers.
commit d471f2d22fe0e040504ab98a1735ad9a6edaa692
Author: Marko A. Rodriguez <okramma...@gmail.com>
Date: 2016-11-14T20:46:00Z
fixed up the hasXXX() methods to account of P and Object differently. Will
go through and create a TraversalHelper.addHasContainer() method which will
left append or right append depending on Traversal state. this will simplify
methods signficiantly.
commit 07f264241fa7a1124faac27a06df83a7449b75c9
Author: Marko A. Rodriguez <okramma...@gmail.com>
Date: 2016-11-14T22:44:40Z
added TraversalHelper.addHasContainer() which will either append a HasStep
with container or if the traverasl ends with a HasContainerHolder, fold the
container into the holder. This just makes the code in GraphTravesrsal cleaner
with less copy/paste.
commit e29e1c33fddf2ea245027fe91dec882b8ffdf4d7
Author: Marko A. Rodriguez <okramma...@gmail.com>
Date: 2016-11-15T15:27:24Z
found a bug in HasContainer.makeHasContainers() around AndP recurssion. No
biggie, just didn't yield ultimate optimization. Found a bug in
InlineFilterStrategy where hasLabel() should only back propagate into
VertexStep[edges] if the step doesn't already have edge labels. Removed
HasContainer.makeHasContainers() -- another ticket I'm putting into this
branch. GraphTraversal.has() is fixed up accordingly with left-fold
HasContainers and valid Object[] usage. Going to add a few more tests around
hasXXX() steps.
----
> Chained has()-steps should simply left-append HasContainers in Gremlin-Java.
> ----------------------------------------------------------------------------
>
> Key: TINKERPOP-1502
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1502
> Project: TinkerPop
> Issue Type: Improvement
> Components: process
> Affects Versions: 3.2.2
> Reporter: Marko A. Rodriguez
>
> In Gremlin-Java, {{g.V().has(a).has(b).has(c).out()}} is originally
> represented as {{[GraphStep,HasStep(a),HasStep(b),HasStep(c),VertexStep]}}.
> Ultimately, {{InlineFilterStrategy}} or most provider strategies will turn
> such {{HasStep}}-chains into {{[GraphStep,HasStep(a,b,c),VertexStep]}}. That
> is, strategies fold {{has()}}-steps "left" and delete "right" {{has()}}-steps
> and left propagates their labels (i.e. clock cycles). I think that
> {{GraphTraversal}} should simply do this:
> {code}
> public GraphTraversal has(whateves) {
> if(this.getEndStep() instanceof HasStep)
> this.getEndSte().addHasContainer(new HasContainer(whateves))
> else
> this.addStep(new HasStep(new HasContainer(whateves)));
> this.bytecode.addStep("has",whateves);
> return this;
> }
> {code}
> In essence, a "write time" optimization can be done. Given that chains of
> {{has()}}'s is super common, this can save significant clock-cycles in the
> long run of a production application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
