[jira] [Commented] (TINKERPOP-1681) Multiple hasId's are or'd into GraphStep

[ASF GitHub Bot (JIRA)]

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

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

Github user asfgit closed the pull request at:

https://github.com/apache/tinkerpop/pull/617


> Multiple hasId's are or'd into GraphStep
> 
>
> Key: TINKERPOP-1681
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1681
> Project: TinkerPop
>  Issue Type: Bug
>  Components: process
>Affects Versions: 3.2.4
>Reporter: Daniel Kuppitz
>Assignee: Daniel Kuppitz
>
> From the dev mailing list:
> {noformat}
> @Test
> public void testHasId() {
> final TinkerGraph graph = TinkerGraph.open();
> Vertex a = graph.addVertex(T.label, "A");
> Vertex b = graph.addVertex(T.label, "B");
> List vertices = 
> graph.traversal().V().hasId(a.id()).hasId(b.id()).toList();
> Assert.assertTrue(vertices.isEmpty());
> }
> {noformat}
> The test fails as the both vertices are returned.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (TINKERPOP-1681) Multiple hasId's are or'd into GraphStep

[ASF GitHub Bot (JIRA)]

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

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

Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/617
  
Needs an entry to CHANGELOG, but all tests pass with `docker/build.sh -t -n 
-i`

VOTE +1


> Multiple hasId's are or'd into GraphStep
> 
>
> Key: TINKERPOP-1681
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1681
> Project: TinkerPop
>  Issue Type: Bug
>  Components: process
>Affects Versions: 3.2.4
>Reporter: Daniel Kuppitz
>Assignee: Daniel Kuppitz
>
> From the dev mailing list:
> {noformat}
> @Test
> public void testHasId() {
> final TinkerGraph graph = TinkerGraph.open();
> Vertex a = graph.addVertex(T.label, "A");
> Vertex b = graph.addVertex(T.label, "B");
> List vertices = 
> graph.traversal().V().hasId(a.id()).hasId(b.id()).toList();
> Assert.assertTrue(vertices.isEmpty());
> }
> {noformat}
> The test fails as the both vertices are returned.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (TINKERPOP-1681) Multiple hasId's are or'd into GraphStep

[ASF GitHub Bot (JIRA)]

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

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

Github user okram commented on the issue:

https://github.com/apache/tinkerpop/pull/617
  
Seems right to me. Just to be certain `x.y.z` are ANDs and before, for 
`hasLabel()` and `hasId()` they were being OR'd. Test cases make sense.

VOTE +1.


> Multiple hasId's are or'd into GraphStep
> 
>
> Key: TINKERPOP-1681
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1681
> Project: TinkerPop
>  Issue Type: Bug
>  Components: process
>Affects Versions: 3.2.4
>Reporter: Daniel Kuppitz
>Assignee: Daniel Kuppitz
>
> From the dev mailing list:
> {noformat}
> @Test
> public void testHasId() {
> final TinkerGraph graph = TinkerGraph.open();
> Vertex a = graph.addVertex(T.label, "A");
> Vertex b = graph.addVertex(T.label, "B");
> List vertices = 
> graph.traversal().V().hasId(a.id()).hasId(b.id()).toList();
> Assert.assertTrue(vertices.isEmpty());
> }
> {noformat}
> The test fails as the both vertices are returned.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (TINKERPOP-1681) Multiple hasId's are or'd into GraphStep

[ASF GitHub Bot (JIRA)]

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

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

GitHub user dkuppitz opened a pull request:

https://github.com/apache/tinkerpop/pull/617

TINKERPOP-1681 Multiple hasId's are or'd into GraphStep

https://issues.apache.org/jira/browse/TINKERPOP-1681

Fixed folding of multiple `hasId()`'s into `GraphStep`.

VOTE: +1

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

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

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

https://github.com/apache/tinkerpop/pull/617.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 #617


commit 063118e9d7b65cff82e74c5db47e64524a833e92
Author: Daniel Kuppitz 
Date:   2017-05-28T13:41:34Z

Fixed folding of multiple `hasId()` into `GraphStep`.




> Multiple hasId's are or'd into GraphStep
> 
>
> Key: TINKERPOP-1681
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1681
> Project: TinkerPop
>  Issue Type: Bug
>  Components: process
>Affects Versions: 3.2.4
>Reporter: Daniel Kuppitz
>Assignee: Daniel Kuppitz
>
> From the dev mailing list:
> {noformat}
> @Test
> public void testHasId() {
> final TinkerGraph graph = TinkerGraph.open();
> Vertex a = graph.addVertex(T.label, "A");
> Vertex b = graph.addVertex(T.label, "B");
> List vertices = 
> graph.traversal().V().hasId(a.id()).hasId(b.id()).toList();
> Assert.assertTrue(vertices.isEmpty());
> }
> {noformat}
> The test fails as the both vertices are returned.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (TINKERPOP-1681) Multiple hasId's are or'd into GraphStep

[Daniel Kuppitz (JIRA)]

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

Daniel Kuppitz commented on TINKERPOP-1681:
---

Yea, I've added your 2 test cases.

See: 
https://github.com/apache/tinkerpop/commit/1856f79ba2558a30d56c8d08f26138dbd29cc370

{{not(identity())}} is out of question, as it would be a bad rewrite. Another 
thing that came to my mind was {{g.inject([] as Object[])}}, but, well, 
{{inject()}} always looks so hacky.

> Multiple hasId's are or'd into GraphStep
> 
>
> Key: TINKERPOP-1681
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1681
> Project: TinkerPop
>  Issue Type: Bug
>  Components: process
>Affects Versions: 3.2.4
>Reporter: Daniel Kuppitz
>Assignee: Daniel Kuppitz
>
> From the dev mailing list:
> {noformat}
> @Test
> public void testHasId() {
> final TinkerGraph graph = TinkerGraph.open();
> Vertex a = graph.addVertex(T.label, "A");
> Vertex b = graph.addVertex(T.label, "B");
> List vertices = 
> graph.traversal().V().hasId(a.id()).hasId(b.id()).toList();
> Assert.assertTrue(vertices.isEmpty());
> }
> {noformat}
> The test fails as the both vertices are returned.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (TINKERPOP-1681) Multiple hasId's are or'd into GraphStep

[pieter martin (JIRA)]

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

pieter martin commented on TINKERPOP-1681:
--

Just to check does your fix also work for,

{noformat}
final TinkerGraph graph = TinkerGraph.open();
Vertex a = graph.addVertex(T.label, "A");
Vertex b = graph.addVertex(T.label, "B");
List vertices = graph.traversal().V(a.id()).hasId(b.id()).toList();
Assert.assertTrue(vertices.isEmpty());
{noformat}

Regarding {{g.V().not(identity())}} it for sure is bad as {{not(identity())}} 
is not the kind of thing that Sqlg at least, even tries to optimize.

> Multiple hasId's are or'd into GraphStep
> 
>
> Key: TINKERPOP-1681
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1681
> Project: TinkerPop
>  Issue Type: Bug
>  Components: process
>Affects Versions: 3.2.4
>Reporter: Daniel Kuppitz
>Assignee: Daniel Kuppitz
>
> From the dev mailing list:
> {noformat}
> @Test
> public void testHasId() {
> final TinkerGraph graph = TinkerGraph.open();
> Vertex a = graph.addVertex(T.label, "A");
> Vertex b = graph.addVertex(T.label, "B");
> List vertices = 
> graph.traversal().V().hasId(a.id()).hasId(b.id()).toList();
> Assert.assertTrue(vertices.isEmpty());
> }
> {noformat}
> The test fails as the both vertices are returned.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (TINKERPOP-1681) Multiple hasId's are or'd into GraphStep

[Daniel Kuppitz (JIRA)]

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

Daniel Kuppitz commented on TINKERPOP-1681:
---

I have a fix locally (the only folds the first set of ids into GraphStep), but 
I wonder if we should be smarter. We could take the intersection of multiple 
{{hasId}}'s, so that

{noformat}
g.V().hasId(within(1,2)).hasId(within(2,3))
{noformat}

... becomes:

{noformat}
g.V().hasId(2) // or g.V().hasId(within(2))
{noformat}

But what if the intersection is empty?

{noformat}
g.V().hasId(within(1,2)).hasId(3)
{noformat}

... could be converted into something like this:

{noformat}
g.V().not(identity())
{noformat}

However, that turns the query into a full-scan query, which is not acceptable 
IMO. Other suggestions?

> Multiple hasId's are or'd into GraphStep
> 
>
> Key: TINKERPOP-1681
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1681
> Project: TinkerPop
>  Issue Type: Bug
>  Components: process
>Affects Versions: 3.2.4
>Reporter: Daniel Kuppitz
>Assignee: Daniel Kuppitz
>
> From the dev mailing list:
> {noformat}
> @Test
> public void testHasId() {
> final TinkerGraph graph = TinkerGraph.open();
> Vertex a = graph.addVertex(T.label, "A");
> Vertex b = graph.addVertex(T.label, "B");
> List vertices = 
> graph.traversal().V().hasId(a.id()).hasId(b.id()).toList();
> Assert.assertTrue(vertices.isEmpty());
> }
> {noformat}
> The test fails as the both vertices are returned.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)