[jira] [Commented] (JENA-1519) OpWalkerVisitor should interact better with custom operators

2018-04-18 Thread ASF subversion and git services (JIRA) 

[ 
https://issues.apache.org/jira/browse/JENA-1519?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16442293#comment-16442293
 ] 

ASF subversion and git services commented on JENA-1519:
---

Commit 1af87896dc9e6803878769f5d9eef2a5578b1de7 in jena's branch 
refs/heads/master from [~andy.seaborne]
[ https://git-wip-us.apache.org/repos/asf?p=jena.git;h=1af8789 ]

JENA-1519: Move decision about effectiveOp into walker.


> OpWalkerVisitor should interact better with custom operators
> 
>
> Key: JENA-1519
> URL: https://issues.apache.org/jira/browse/JENA-1519
> Project: Apache Jena
>  Issue Type: Bug
>  Components: ARQ
>Affects Versions: Jena 3.6.0
>Reporter: Jeremy Coulon
>Priority: Major
>
> When visiting an OpExt, current implementation of OpWalkerVisitor is just 
> calling the visitor on opExt.effectiveOp().
> However if OpExt is replacing a sub-tree of the algebra instead of a single 
> leaf node, the sub-tree is ignored by walker.
> I found this bug when trying to execute a query with a MINUS operator with 
> either side of the operator being replaced by an custom operator.
> For example the following query is not working, if I transform both side of 
> MINUS with a custom operator:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (bgp (triple ?animal rdf:type ex:Animal))}}
> {{    (filter (|| (= ?type ex:Reptile) (= ?type ex:Insect))}}
> {{      (bgp (triple ?animal rdf:type ?type)}}
> which I transform into:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (op-ext )}}
> {{    (op-ext )))}}
> The reason is that MINUS is calling OpVars.visibleVars() which does not walk 
> into the subop of the filter.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (JENA-1519) OpWalkerVisitor should interact better with custom operators

2018-04-18 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/JENA-1519?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16442296#comment-16442296
 ] 

ASF GitHub Bot commented on JENA-1519:
--

Github user asfgit closed the pull request at:

https://github.com/apache/jena/pull/394


> OpWalkerVisitor should interact better with custom operators
> 
>
> Key: JENA-1519
> URL: https://issues.apache.org/jira/browse/JENA-1519
> Project: Apache Jena
>  Issue Type: Bug
>  Components: ARQ
>Affects Versions: Jena 3.6.0
>Reporter: Jeremy Coulon
>Priority: Major
>
> When visiting an OpExt, current implementation of OpWalkerVisitor is just 
> calling the visitor on opExt.effectiveOp().
> However if OpExt is replacing a sub-tree of the algebra instead of a single 
> leaf node, the sub-tree is ignored by walker.
> I found this bug when trying to execute a query with a MINUS operator with 
> either side of the operator being replaced by an custom operator.
> For example the following query is not working, if I transform both side of 
> MINUS with a custom operator:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (bgp (triple ?animal rdf:type ex:Animal))}}
> {{    (filter (|| (= ?type ex:Reptile) (= ?type ex:Insect))}}
> {{      (bgp (triple ?animal rdf:type ?type)}}
> which I transform into:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (op-ext )}}
> {{    (op-ext )))}}
> The reason is that MINUS is calling OpVars.visibleVars() which does not walk 
> into the subop of the filter.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (JENA-1519) OpWalkerVisitor should interact better with custom operators

2018-04-18 Thread ASF subversion and git services (JIRA) 

[ 
https://issues.apache.org/jira/browse/JENA-1519?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16442292#comment-16442292
 ] 

ASF subversion and git services commented on JENA-1519:
---

Commit 2a5577725073d04671cec0359f9597b54b1d1efd in jena's branch 
refs/heads/master from [~andy.seaborne]
[ https://git-wip-us.apache.org/repos/asf?p=jena.git;h=2a55777 ]

JENA-1519: Merge commit 'refs/pull/394/head' of https://github.com/apache/jena

This closes #394.


> OpWalkerVisitor should interact better with custom operators
> 
>
> Key: JENA-1519
> URL: https://issues.apache.org/jira/browse/JENA-1519
> Project: Apache Jena
>  Issue Type: Bug
>  Components: ARQ
>Affects Versions: Jena 3.6.0
>Reporter: Jeremy Coulon
>Priority: Major
>
> When visiting an OpExt, current implementation of OpWalkerVisitor is just 
> calling the visitor on opExt.effectiveOp().
> However if OpExt is replacing a sub-tree of the algebra instead of a single 
> leaf node, the sub-tree is ignored by walker.
> I found this bug when trying to execute a query with a MINUS operator with 
> either side of the operator being replaced by an custom operator.
> For example the following query is not working, if I transform both side of 
> MINUS with a custom operator:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (bgp (triple ?animal rdf:type ex:Animal))}}
> {{    (filter (|| (= ?type ex:Reptile) (= ?type ex:Insect))}}
> {{      (bgp (triple ?animal rdf:type ?type)}}
> which I transform into:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (op-ext )}}
> {{    (op-ext )))}}
> The reason is that MINUS is calling OpVars.visibleVars() which does not walk 
> into the subop of the filter.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (JENA-1519) OpWalkerVisitor should interact better with custom operators

2018-04-17 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/JENA-1519?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16441017#comment-16441017
 ] 

ASF GitHub Bot commented on JENA-1519:
--

Github user afs commented on a diff in the pull request:

https://github.com/apache/jena/pull/394#discussion_r182114929
  
--- Diff: 
jena-arq/src/main/java/org/apache/jena/sparql/algebra/OpWalker.java ---
@@ -139,6 +139,7 @@ protected void visitN(OpN op) {
 @Override
 protected void visitExt(OpExt op) {
 before(op) ;
+super.visitExt(op);
--- End diff --

There are probably both cases. Its an extension point so it is hard to be 
categorical. Only the effective is walkable unless the OpExt is comprised of 
known Ops in which case it does not need to be an OpExt.

Let's leave it as per the PR - we might have to return to it but without a 
concrete alternative usage, it feels like slipping towards guessing.



> OpWalkerVisitor should interact better with custom operators
> 
>
> Key: JENA-1519
> URL: https://issues.apache.org/jira/browse/JENA-1519
> Project: Apache Jena
>  Issue Type: Bug
>  Components: ARQ
>Affects Versions: Jena 3.6.0
>Reporter: Jeremy Coulon
>Priority: Major
>
> When visiting an OpExt, current implementation of OpWalkerVisitor is just 
> calling the visitor on opExt.effectiveOp().
> However if OpExt is replacing a sub-tree of the algebra instead of a single 
> leaf node, the sub-tree is ignored by walker.
> I found this bug when trying to execute a query with a MINUS operator with 
> either side of the operator being replaced by an custom operator.
> For example the following query is not working, if I transform both side of 
> MINUS with a custom operator:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (bgp (triple ?animal rdf:type ex:Animal))}}
> {{    (filter (|| (= ?type ex:Reptile) (= ?type ex:Insect))}}
> {{      (bgp (triple ?animal rdf:type ?type)}}
> which I transform into:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (op-ext )}}
> {{    (op-ext )))}}
> The reason is that MINUS is calling OpVars.visibleVars() which does not walk 
> into the subop of the filter.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (JENA-1519) OpWalkerVisitor should interact better with custom operators

2018-04-13 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/JENA-1519?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16437366#comment-16437366
 ] 

ASF GitHub Bot commented on JENA-1519:
--

Github user jeremy-coulon commented on a diff in the pull request:

https://github.com/apache/jena/pull/394#discussion_r181407237
  
--- Diff: 
jena-arq/src/main/java/org/apache/jena/sparql/algebra/OpWalker.java ---
@@ -139,6 +139,7 @@ protected void visitN(OpN op) {
 @Override
 protected void visitExt(OpExt op) {
 before(op) ;
+super.visitExt(op);
--- End diff --

Visiting the real OpExt is not useful for me but I can't say for other 
people.


> OpWalkerVisitor should interact better with custom operators
> 
>
> Key: JENA-1519
> URL: https://issues.apache.org/jira/browse/JENA-1519
> Project: Apache Jena
>  Issue Type: Bug
>  Components: ARQ
>Affects Versions: Jena 3.6.0
>Reporter: Jeremy Coulon
>Priority: Major
>
> When visiting an OpExt, current implementation of OpWalkerVisitor is just 
> calling the visitor on opExt.effectiveOp().
> However if OpExt is replacing a sub-tree of the algebra instead of a single 
> leaf node, the sub-tree is ignored by walker.
> I found this bug when trying to execute a query with a MINUS operator with 
> either side of the operator being replaced by an custom operator.
> For example the following query is not working, if I transform both side of 
> MINUS with a custom operator:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (bgp (triple ?animal rdf:type ex:Animal))}}
> {{    (filter (|| (= ?type ex:Reptile) (= ?type ex:Insect))}}
> {{      (bgp (triple ?animal rdf:type ?type)}}
> which I transform into:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (op-ext )}}
> {{    (op-ext )))}}
> The reason is that MINUS is calling OpVars.visibleVars() which does not walk 
> into the subop of the filter.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (JENA-1519) OpWalkerVisitor should interact better with custom operators

2018-04-13 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/JENA-1519?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16437326#comment-16437326
 ] 

ASF GitHub Bot commented on JENA-1519:
--

Github user afs commented on a diff in the pull request:

https://github.com/apache/jena/pull/394#discussion_r181393545
  
--- Diff: 
jena-arq/src/main/java/org/apache/jena/sparql/algebra/OpWalker.java ---
@@ -139,6 +139,7 @@ protected void visitN(OpN op) {
 @Override
 protected void visitExt(OpExt op) {
 before(op) ;
+super.visitExt(op);
--- End diff --

Not sure about this. It is visiting both the effective op and the real 
OpExt. Shoudn't it be the visitor deciding that? ie. visiting 
`op.effectiveOp()` is required?


> OpWalkerVisitor should interact better with custom operators
> 
>
> Key: JENA-1519
> URL: https://issues.apache.org/jira/browse/JENA-1519
> Project: Apache Jena
>  Issue Type: Bug
>  Components: ARQ
>Affects Versions: Jena 3.6.0
>Reporter: Jeremy Coulon
>Priority: Major
>
> When visiting an OpExt, current implementation of OpWalkerVisitor is just 
> calling the visitor on opExt.effectiveOp().
> However if OpExt is replacing a sub-tree of the algebra instead of a single 
> leaf node, the sub-tree is ignored by walker.
> I found this bug when trying to execute a query with a MINUS operator with 
> either side of the operator being replaced by an custom operator.
> For example the following query is not working, if I transform both side of 
> MINUS with a custom operator:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (bgp (triple ?animal rdf:type ex:Animal))}}
> {{    (filter (|| (= ?type ex:Reptile) (= ?type ex:Insect))}}
> {{      (bgp (triple ?animal rdf:type ?type)}}
> which I transform into:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (op-ext )}}
> {{    (op-ext )))}}
> The reason is that MINUS is calling OpVars.visibleVars() which does not walk 
> into the subop of the filter.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (JENA-1519) OpWalkerVisitor should interact better with custom operators

2018-04-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/JENA-1519?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16432459#comment-16432459
 ] 

ASF GitHub Bot commented on JENA-1519:
--

GitHub user jeremy-coulon opened a pull request:

https://github.com/apache/jena/pull/394

JENA-1519: OpWalkerVisitor with custom operators



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

$ git pull https://github.com/jeremy-coulon/jena bug-opext-jira-1519

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

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


commit 74529be4f7da2ce1a16c5542df073a1911859751
Author: Jeremy Coulon 
Date:   2018-04-10T15:19:29Z

Add UT showing the problem

commit 9c2bb1c183a70fafa1007857823e91aca9a1f18f
Author: Jeremy Coulon 
Date:   2018-04-10T15:20:08Z

Fix OpWalkerVisitor for OpExt




> OpWalkerVisitor should interact better with custom operators
> 
>
> Key: JENA-1519
> URL: https://issues.apache.org/jira/browse/JENA-1519
> Project: Apache Jena
>  Issue Type: Bug
>  Components: ARQ
>Affects Versions: Jena 3.6.0
>Reporter: Jeremy Coulon
>Priority: Major
>
> When visiting an OpExt, current implementation of OpWalkerVisitor is just 
> calling the visitor on opExt.effectiveOp().
> However if OpExt is replacing a sub-tree of the algebra instead of a single 
> leaf node, the sub-tree is ignored by walker.
> I found this bug when trying to execute a query with a MINUS operator with 
> either side of the operator being replaced by an custom operator.
> For example the following query is not working, if I transform both side of 
> MINUS with a custom operator:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (bgp (triple ?animal rdf:type ex:Animal))}}
> {{    (filter (|| (= ?type ex:Reptile) (= ?type ex:Insect))}}
> {{      (bgp (triple ?animal rdf:type ?type)}}
> which I transform into:
> {{(project (?animal)}}
> {{  (minus}}
> {{    (op-ext )}}
> {{    (op-ext )))}}
> The reason is that MINUS is calling OpVars.visibleVars() which does not walk 
> into the subop of the filter.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)