You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by Claude Warren <cl...@xenei.com> on 2013/11/25 08:03:23 UTC
Re: svn commit: r1545008 - in /jena/trunk/jena-arq/src:
main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java
test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java
Andy,
My test with the bound was removed and I don't see one with the bound
included. I think the bound was what triggered the issue in the case I was
attempting to fix, because the bound creates a new triple block.
Do you think this is not important to test or am I missing a test that
applies in this case?
Claude
On Sun, Nov 24, 2013 at 4:27 PM, <an...@apache.org> wrote:
> Author: andy
> Date: Sun Nov 24 16:27:13 2013
> New Revision: 1545008
>
> URL: http://svn.apache.org/r1545008
> Log:
> More general fix for JENA-595.
>
> Modified:
>
> jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java
>
> jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java
>
> Modified:
> jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java
> URL:
> http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java?rev=1545008&r1=1545007&r2=1545008&view=diff
>
> ==============================================================================
> ---
> jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java
> (original)
> +++
> jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java
> Sun Nov 24 16:27:13 2013
> @@ -426,26 +426,19 @@ public class TransformFilterPlacement ex
>
> // And try down the expressions
> Placement p = transform(pushed, opSub) ;
> -
> +
> if ( p == null ) {
> - Op op1 = OpFilter.filter(pushed, opSub) ;
> - Op op2 = input.copy(op1) ; //, input.getVarExprList()) ;
> //OpExtend.extend(op1, input.getVarExprList()) ;
> - return result(op2, unpushed) ;
> - }
> - // handle the case where the filter is being to a surrounding
> sequence or other bgp container
> - // and may apply to the assignation variables.
> - // in this case disjoint var sets indicate the expression is
> unused.
> - pushed = new ExprList() ;
> - for ( Expr expr : p.unplaced ) {
> - Set<Var> exprVars = expr.getVarsMentioned() ;
> - if ( disjoint(vars1, exprVars) )
> - unpushed.add(expr);
> - else
> - pushed.add(expr) ;
> + // Couldn't place an filter expressions. Do nothing.
> + return null ;
> }
> - Op op1 = OpFilter.filter(pushed, p.op) ;
> - Op op2 = input.copy(op1) ;
> - return result(op2, unpushed) ;
> +
> + if ( ! p.unplaced.isEmpty() )
> + // Some placed, not all.
> + // Pass back out all untouched expressions.
> + unpushed.addAll(p.unplaced) ;
> + Op op1 = input.copy(p.op) ;
> +
> + return result(op1, unpushed) ;
> }
>
> private Placement placeProject(ExprList exprs, OpProject input) {
>
> Modified:
> jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java
> URL:
> http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java?rev=1545008&r1=1545007&r2=1545008&view=diff
>
> ==============================================================================
> ---
> jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java
> (original)
> +++
> jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java
> Sun Nov 24 16:27:13 2013
> @@ -124,15 +124,6 @@ public class TestTransformFilterPlacemen
> null) ;
> }
>
> - @Test public void place_sequence_with_bind() {
> - test("(filter (= ?foo 1) " +
> - "(sequence " +
> - "(extend " +
> - " ((?bound (if ?v_binder 'Y' 'N')))
> " +
> - " (bgp (triple ?foob <
> http://example.com/binding> ?v_binder)))" +
> - "(bgp (triple ?foob <
> http://www.w3.org/2000/01/rdf-schema#label> ?foo))))", null );
> - }
> -
> // Join : one sided push.
> @Test public void place_join_01() {
> test("(filter (= ?x 123) (join (bgp (?s ?p ?x)) (bgp (?s ?p ?z))
> ))",
> @@ -232,27 +223,49 @@ public class TestTransformFilterPlacemen
> "(extend ((?z 123)) (filter (= ?x 123) (bgp (?s ?p ?x)) ))")
> ;
> }
>
> - @Test public void place_extend_02() { // Blocked
> + @Test public void place_extend_02() {
> + test("(filter ((= ?x1 123) (= ?x2 456)) (extend (?z 789) (bgp (?s
> ?p ?x1)) ))",
> + "(filter (= ?x2 456) (extend (?z 789) (filter (= ?x1 123)
> (bgp (?s ?p ?x1)) )))") ;
> + }
> +
> + @Test public void place_extend_03() { // Blocked
> test("(filter (= ?x 123) (extend ((?x 123)) (bgp (?s ?p ?z)) ))",
> null) ;
> }
>
> - @Test public void place_extend_03() {
> + @Test public void place_extend_04() {
> test("(filter (= ?x 123) (extend ((?x1 123)) (filter (< ?x 456)
> (bgp (?s ?p ?x) (?s ?p ?z))) ))",
> "(extend (?x1 123) (sequence (filter ((= ?x 123) (< ?x 456))
> (bgp (?s ?p ?x))) (bgp (?s ?p ?z))) )") ;
> }
>
> + @Test public void place_extend_05() {
> + // Filter further out than one place.
> + test("(filter (= ?z 1) (sequence (extend (?x1 123) (bgp (?s ?p
> ?x))) (bgp (?s ?p ?z))))",
> + null) ;
> + }
> +
> + @Test public void place_extend_06() {
> + // Filter further out than one place.
> + test("(filter (= ?z 1) (join (extend (?x1 123) (bgp (?s ?p ?x)))
> (bgp (?s ?p ?z))))" ,
> + "(join (extend (?x1 123) (bgp (?s ?p ?x))) (filter (= ?z 1)
> (bgp (?s ?p ?z))) )") ;
> + }
> +
> @Test public void place_assign_01() {
> test("(filter (= ?x 123) (assign ((?z 123)) (bgp (?s ?p ?x)) ))",
> "(assign ((?z 123)) (filter (= ?x 123) (bgp (?s ?p ?x)) ))")
> ;
> }
>
> - @Test public void place_assign_02() { // Blocked
> + @Test public void place_assign_02() {
> + test("(filter ((= ?x1 123) (= ?x2 456)) (assign (?z 789) (bgp (?s
> ?p ?x1)) ))",
> + "(filter (= ?x2 456) (assign (?z 789) (filter (= ?x1 123)
> (bgp (?s ?p ?x1)) )))") ;
> + }
> +
> + @Test public void place_assign_03() { // Blocked
> test("(filter (= ?x 123) (assign ((?x 123)) (bgp (?s ?p ?z)) ))",
> null) ;
> }
>
> - @Test public void place_assign_03() {
> + @Test public void place_assign_04() {
> // Caution - OpFilter equality is sensitive to the order of
> expressions
> test("(filter (= ?x 123) (assign ((?x1 123)) (filter (< ?x 456)
> (bgp (?s ?p ?x) (?s ?p ?z))) ))",
> "(assign (?x1 123) (sequence (filter ((= ?x 123) (< ?x 456))
> (bgp (?s ?p ?x))) (bgp (?s ?p ?z))) )") ;
>
>
>
--
I like: Like Like - The likeliest place on the web<http://like-like.xenei.com>
LinkedIn: http://www.linkedin.com/in/claudewarren
Re: svn commit: r1545008 - in /jena/trunk/jena-arq/src:
main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java
test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java
Posted by Claude Warren <cl...@xenei.com>.
Thank you for the in depth explanation. I see that the specific case I
couldn't find in the tests is in fact covered. Thank you.
On Mon, Nov 25, 2013 at 10:08 AM, Andy Seaborne <an...@apache.org> wrote:
> On 25/11/13 07:03, Claude Warren wrote:
>
>> Andy,
>>
>> My test with the bound was removed and I don't see one with the bound
>> included. I think the bound was what triggered the issue in the case I
>> was
>> attempting to fix, because the bound creates a new triple block.
>>
>> Do you think this is not important to test or am I missing a test that
>> applies in this case?
>>
>
> Claude,
>
> Your fixed worked in some cases but not other - you make have seen it as a
> programming bug when in fact it was a design error.
>
> I put in several tests to cover the initial situation of your test case
> and also other situations that I could see arising which were related but
> not part of the test case. I hope there is now full coverage. I did
> simpify tests, taking out long URIs that only add chars to the test but
> don't change the thing being tested. Ditto filter expression (if) which is
> not relevant here - I wanted to leave behind a focused set of tests and I
> feel that reducing them down to the core issue means that someone looking
> at them later will find it easier. That's the style of the test class.
>
> If you want to add back your test, please do so. The essence is covered
> by place_extend_05, one of several tests I added or modified.
>
> The core issue was that the expressions that were potential candidates for
> pushing down into the expression below (extend). It is not to do with
> (sequence), although place_extend_05 covers that, nor the nature of the
> FILTER expression (if).
>
> It is wrong to put in the
>
> OpFilter.filter(pushed, ...)
>
> wrapper in either of the two code routes after
>
>
> Placement p = transform(pushed, opSub) ;
>
> If all potential candidate expressions are not processed, that returns
> null, nothing was processed and the correct result is to return null
> indicating no processing done.
>
> The other case is when some, but not all expressions are successfully
> pushed inwards. This is a new situation outside the test cases.
>
> The unprocessed expressions need to be left outside the (extend) just like
> the "unpushed" collection.
>
> At the top of the method we have:
>
> for ( Expr expr : exprs ) {
>
> Set<Var> exprVars = expr.getVarsMentioned() ;
> if ( disjoint(vars1, exprVars) )
> pushed.add(expr);
> else
> unpushed.add(expr) ;
> }
>
> where if disjoint an expr goes into 'pushed' and into 'unpushed' otherwise.
>
> You added after the recursive step:
>
> pushed = new ExprList() ;
>
> for ( Expr expr : p.unplaced ) {
> Set<Var> exprVars = expr.getVarsMentioned() ;
> if ( disjoint(vars1, exprVars) )
> unpushed.add(expr);
> else
> pushed.add(expr) ;
> }
>
> which has the 'pushed' and 'unpush' swapped.
>
> Note that 'p.unplaced' can only contain elements from the initial 'pushed'
> - by swapping the arms of the "if", you can see everything from p.unplaced
> must be added to "unpushed" and "pushed" is now always empty.
>
> In effect, it copies p.unplaced into unpushed.
>
> OpFilter.filter(pushed, p.op) tests whether the expr list is empty and
> does not add a filter is if it oiis, returning p.op.
>
> That can all be reduced to the simpler code.
>
> The if ( ! p.unplaced.isEmpty() ) test is strictly unnecessary. Just
> 'unpushed.addAll(p.unplaced) ; ' would do.
>
> Andy
>
>
>
>> Claude
>>
>>
>> On Sun, Nov 24, 2013 at 4:27 PM, <an...@apache.org> wrote:
>>
>> Author: andy
>>> Date: Sun Nov 24 16:27:13 2013
>>> New Revision: 1545008
>>>
>>> URL: http://svn.apache.org/r1545008
>>> Log:
>>> More general fix for JENA-595.
>>>
>>> Modified:
>>>
>>> jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/
>>> algebra/optimize/TransformFilterPlacement.java
>>>
>>> jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/
>>> algebra/optimize/TestTransformFilterPlacement.java
>>>
>>> Modified:
>>> jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/
>>> algebra/optimize/TransformFilterPlacement.java
>>> URL:
>>> http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/main/
>>> java/com/hp/hpl/jena/sparql/algebra/optimize/
>>> TransformFilterPlacement.java?rev=1545008&r1=1545007&r2=
>>> 1545008&view=diff
>>>
>>> ============================================================
>>> ==================
>>> ---
>>> jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/
>>> algebra/optimize/TransformFilterPlacement.java
>>> (original)
>>> +++
>>> jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/
>>> algebra/optimize/TransformFilterPlacement.java
>>> Sun Nov 24 16:27:13 2013
>>> @@ -426,26 +426,19 @@ public class TransformFilterPlacement ex
>>>
>>> // And try down the expressions
>>> Placement p = transform(pushed, opSub) ;
>>> -
>>> +
>>> if ( p == null ) {
>>> - Op op1 = OpFilter.filter(pushed, opSub) ;
>>> - Op op2 = input.copy(op1) ; //, input.getVarExprList()) ;
>>> //OpExtend.extend(op1, input.getVarExprList()) ;
>>> - return result(op2, unpushed) ;
>>> - }
>>> - // handle the case where the filter is being to a surrounding
>>> sequence or other bgp container
>>> - // and may apply to the assignation variables.
>>> - // in this case disjoint var sets indicate the expression is
>>> unused.
>>> - pushed = new ExprList() ;
>>> - for ( Expr expr : p.unplaced ) {
>>> - Set<Var> exprVars = expr.getVarsMentioned() ;
>>> - if ( disjoint(vars1, exprVars) )
>>> - unpushed.add(expr);
>>> - else
>>> - pushed.add(expr) ;
>>> + // Couldn't place an filter expressions. Do nothing.
>>> + return null ;
>>> }
>>> - Op op1 = OpFilter.filter(pushed, p.op) ;
>>> - Op op2 = input.copy(op1) ;
>>> - return result(op2, unpushed) ;
>>> +
>>> + if ( ! p.unplaced.isEmpty() )
>>> + // Some placed, not all.
>>> + // Pass back out all untouched expressions.
>>> + unpushed.addAll(p.unplaced) ;
>>> + Op op1 = input.copy(p.op) ;
>>> +
>>> + return result(op1, unpushed) ;
>>> }
>>>
>>> private Placement placeProject(ExprList exprs, OpProject input) {
>>>
>>> Modified:
>>> jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/
>>> algebra/optimize/TestTransformFilterPlacement.java
>>> URL:
>>> http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/test/
>>> java/com/hp/hpl/jena/sparql/algebra/optimize/
>>> TestTransformFilterPlacement.java?rev=1545008&r1=1545007&
>>> r2=1545008&view=diff
>>>
>>> ============================================================
>>> ==================
>>> ---
>>> jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/
>>> algebra/optimize/TestTransformFilterPlacement.java
>>> (original)
>>> +++
>>> jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/
>>> algebra/optimize/TestTransformFilterPlacement.java
>>> Sun Nov 24 16:27:13 2013
>>> @@ -124,15 +124,6 @@ public class TestTransformFilterPlacemen
>>> null) ;
>>> }
>>>
>>> - @Test public void place_sequence_with_bind() {
>>> - test("(filter (= ?foo 1) " +
>>> - "(sequence " +
>>> - "(extend " +
>>> - " ((?bound (if ?v_binder 'Y' 'N')))
>>> " +
>>> - " (bgp (triple ?foob <
>>> http://example.com/binding> ?v_binder)))" +
>>> - "(bgp (triple ?foob <
>>> http://www.w3.org/2000/01/rdf-schema#label> ?foo))))", null );
>>> - }
>>> -
>>> // Join : one sided push.
>>> @Test public void place_join_01() {
>>> test("(filter (= ?x 123) (join (bgp (?s ?p ?x)) (bgp (?s ?p
>>> ?z))
>>> ))",
>>> @@ -232,27 +223,49 @@ public class TestTransformFilterPlacemen
>>> "(extend ((?z 123)) (filter (= ?x 123) (bgp (?s ?p ?x))
>>> ))")
>>> ;
>>> }
>>>
>>> - @Test public void place_extend_02() { // Blocked
>>> + @Test public void place_extend_02() {
>>> + test("(filter ((= ?x1 123) (= ?x2 456)) (extend (?z 789) (bgp
>>> (?s
>>> ?p ?x1)) ))",
>>> + "(filter (= ?x2 456) (extend (?z 789) (filter (= ?x1 123)
>>> (bgp (?s ?p ?x1)) )))") ;
>>> + }
>>> +
>>> + @Test public void place_extend_03() { // Blocked
>>> test("(filter (= ?x 123) (extend ((?x 123)) (bgp (?s ?p ?z))
>>> ))",
>>> null) ;
>>> }
>>>
>>> - @Test public void place_extend_03() {
>>> + @Test public void place_extend_04() {
>>> test("(filter (= ?x 123) (extend ((?x1 123)) (filter (< ?x 456)
>>> (bgp (?s ?p ?x) (?s ?p ?z))) ))",
>>> "(extend (?x1 123) (sequence (filter ((= ?x 123) (< ?x
>>> 456))
>>> (bgp (?s ?p ?x))) (bgp (?s ?p ?z))) )") ;
>>> }
>>>
>>> + @Test public void place_extend_05() {
>>> + // Filter further out than one place.
>>> + test("(filter (= ?z 1) (sequence (extend (?x1 123) (bgp (?s ?p
>>> ?x))) (bgp (?s ?p ?z))))",
>>> + null) ;
>>> + }
>>> +
>>> + @Test public void place_extend_06() {
>>> + // Filter further out than one place.
>>> + test("(filter (= ?z 1) (join (extend (?x1 123) (bgp (?s ?p ?x)))
>>> (bgp (?s ?p ?z))))" ,
>>> + "(join (extend (?x1 123) (bgp (?s ?p ?x))) (filter (= ?z 1)
>>> (bgp (?s ?p ?z))) )") ;
>>> + }
>>> +
>>> @Test public void place_assign_01() {
>>> test("(filter (= ?x 123) (assign ((?z 123)) (bgp (?s ?p ?x))
>>> ))",
>>> "(assign ((?z 123)) (filter (= ?x 123) (bgp (?s ?p ?x))
>>> ))")
>>> ;
>>> }
>>>
>>> - @Test public void place_assign_02() { // Blocked
>>> + @Test public void place_assign_02() {
>>> + test("(filter ((= ?x1 123) (= ?x2 456)) (assign (?z 789) (bgp
>>> (?s
>>> ?p ?x1)) ))",
>>> + "(filter (= ?x2 456) (assign (?z 789) (filter (= ?x1 123)
>>> (bgp (?s ?p ?x1)) )))") ;
>>> + }
>>> +
>>> + @Test public void place_assign_03() { // Blocked
>>> test("(filter (= ?x 123) (assign ((?x 123)) (bgp (?s ?p ?z))
>>> ))",
>>> null) ;
>>> }
>>>
>>> - @Test public void place_assign_03() {
>>> + @Test public void place_assign_04() {
>>> // Caution - OpFilter equality is sensitive to the order of
>>> expressions
>>> test("(filter (= ?x 123) (assign ((?x1 123)) (filter (< ?x 456)
>>> (bgp (?s ?p ?x) (?s ?p ?z))) ))",
>>> "(assign (?x1 123) (sequence (filter ((= ?x 123) (< ?x
>>> 456))
>>> (bgp (?s ?p ?x))) (bgp (?s ?p ?z))) )") ;
>>>
>>>
>>>
>>>
>>
>>
>
--
I like: Like Like - The likeliest place on the web<http://like-like.xenei.com>
LinkedIn: http://www.linkedin.com/in/claudewarren
Re: svn commit: r1545008 - in /jena/trunk/jena-arq/src: main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java
test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java
Posted by Andy Seaborne <an...@apache.org>.
On 25/11/13 07:03, Claude Warren wrote:
> Andy,
>
> My test with the bound was removed and I don't see one with the bound
> included. I think the bound was what triggered the issue in the case I was
> attempting to fix, because the bound creates a new triple block.
>
> Do you think this is not important to test or am I missing a test that
> applies in this case?
Claude,
Your fixed worked in some cases but not other - you make have seen it as
a programming bug when in fact it was a design error.
I put in several tests to cover the initial situation of your test case
and also other situations that I could see arising which were related
but not part of the test case. I hope there is now full coverage. I
did simpify tests, taking out long URIs that only add chars to the test
but don't change the thing being tested. Ditto filter expression (if)
which is not relevant here - I wanted to leave behind a focused set of
tests and I feel that reducing them down to the core issue means that
someone looking at them later will find it easier. That's the style of
the test class.
If you want to add back your test, please do so. The essence is covered
by place_extend_05, one of several tests I added or modified.
The core issue was that the expressions that were potential candidates
for pushing down into the expression below (extend). It is not to do
with (sequence), although place_extend_05 covers that, nor the nature of
the FILTER expression (if).
It is wrong to put in the
OpFilter.filter(pushed, ...)
wrapper in either of the two code routes after
Placement p = transform(pushed, opSub) ;
If all potential candidate expressions are not processed, that returns
null, nothing was processed and the correct result is to return null
indicating no processing done.
The other case is when some, but not all expressions are successfully
pushed inwards. This is a new situation outside the test cases.
The unprocessed expressions need to be left outside the (extend) just
like the "unpushed" collection.
At the top of the method we have:
for ( Expr expr : exprs ) {
Set<Var> exprVars = expr.getVarsMentioned() ;
if ( disjoint(vars1, exprVars) )
pushed.add(expr);
else
unpushed.add(expr) ;
}
where if disjoint an expr goes into 'pushed' and into 'unpushed' otherwise.
You added after the recursive step:
pushed = new ExprList() ;
for ( Expr expr : p.unplaced ) {
Set<Var> exprVars = expr.getVarsMentioned() ;
if ( disjoint(vars1, exprVars) )
unpushed.add(expr);
else
pushed.add(expr) ;
}
which has the 'pushed' and 'unpush' swapped.
Note that 'p.unplaced' can only contain elements from the initial
'pushed' - by swapping the arms of the "if", you can see everything from
p.unplaced must be added to "unpushed" and "pushed" is now always empty.
In effect, it copies p.unplaced into unpushed.
OpFilter.filter(pushed, p.op) tests whether the expr list is empty and
does not add a filter is if it oiis, returning p.op.
That can all be reduced to the simpler code.
The if ( ! p.unplaced.isEmpty() ) test is strictly unnecessary. Just
'unpushed.addAll(p.unplaced) ; ' would do.
Andy
>
> Claude
>
>
> On Sun, Nov 24, 2013 at 4:27 PM, <an...@apache.org> wrote:
>
>> Author: andy
>> Date: Sun Nov 24 16:27:13 2013
>> New Revision: 1545008
>>
>> URL: http://svn.apache.org/r1545008
>> Log:
>> More general fix for JENA-595.
>>
>> Modified:
>>
>> jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java
>>
>> jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java
>>
>> Modified:
>> jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java
>> URL:
>> http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java?rev=1545008&r1=1545007&r2=1545008&view=diff
>>
>> ==============================================================================
>> ---
>> jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java
>> (original)
>> +++
>> jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java
>> Sun Nov 24 16:27:13 2013
>> @@ -426,26 +426,19 @@ public class TransformFilterPlacement ex
>>
>> // And try down the expressions
>> Placement p = transform(pushed, opSub) ;
>> -
>> +
>> if ( p == null ) {
>> - Op op1 = OpFilter.filter(pushed, opSub) ;
>> - Op op2 = input.copy(op1) ; //, input.getVarExprList()) ;
>> //OpExtend.extend(op1, input.getVarExprList()) ;
>> - return result(op2, unpushed) ;
>> - }
>> - // handle the case where the filter is being to a surrounding
>> sequence or other bgp container
>> - // and may apply to the assignation variables.
>> - // in this case disjoint var sets indicate the expression is
>> unused.
>> - pushed = new ExprList() ;
>> - for ( Expr expr : p.unplaced ) {
>> - Set<Var> exprVars = expr.getVarsMentioned() ;
>> - if ( disjoint(vars1, exprVars) )
>> - unpushed.add(expr);
>> - else
>> - pushed.add(expr) ;
>> + // Couldn't place an filter expressions. Do nothing.
>> + return null ;
>> }
>> - Op op1 = OpFilter.filter(pushed, p.op) ;
>> - Op op2 = input.copy(op1) ;
>> - return result(op2, unpushed) ;
>> +
>> + if ( ! p.unplaced.isEmpty() )
>> + // Some placed, not all.
>> + // Pass back out all untouched expressions.
>> + unpushed.addAll(p.unplaced) ;
>> + Op op1 = input.copy(p.op) ;
>> +
>> + return result(op1, unpushed) ;
>> }
>>
>> private Placement placeProject(ExprList exprs, OpProject input) {
>>
>> Modified:
>> jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java
>> URL:
>> http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java?rev=1545008&r1=1545007&r2=1545008&view=diff
>>
>> ==============================================================================
>> ---
>> jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java
>> (original)
>> +++
>> jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java
>> Sun Nov 24 16:27:13 2013
>> @@ -124,15 +124,6 @@ public class TestTransformFilterPlacemen
>> null) ;
>> }
>>
>> - @Test public void place_sequence_with_bind() {
>> - test("(filter (= ?foo 1) " +
>> - "(sequence " +
>> - "(extend " +
>> - " ((?bound (if ?v_binder 'Y' 'N')))
>> " +
>> - " (bgp (triple ?foob <
>> http://example.com/binding> ?v_binder)))" +
>> - "(bgp (triple ?foob <
>> http://www.w3.org/2000/01/rdf-schema#label> ?foo))))", null );
>> - }
>> -
>> // Join : one sided push.
>> @Test public void place_join_01() {
>> test("(filter (= ?x 123) (join (bgp (?s ?p ?x)) (bgp (?s ?p ?z))
>> ))",
>> @@ -232,27 +223,49 @@ public class TestTransformFilterPlacemen
>> "(extend ((?z 123)) (filter (= ?x 123) (bgp (?s ?p ?x)) ))")
>> ;
>> }
>>
>> - @Test public void place_extend_02() { // Blocked
>> + @Test public void place_extend_02() {
>> + test("(filter ((= ?x1 123) (= ?x2 456)) (extend (?z 789) (bgp (?s
>> ?p ?x1)) ))",
>> + "(filter (= ?x2 456) (extend (?z 789) (filter (= ?x1 123)
>> (bgp (?s ?p ?x1)) )))") ;
>> + }
>> +
>> + @Test public void place_extend_03() { // Blocked
>> test("(filter (= ?x 123) (extend ((?x 123)) (bgp (?s ?p ?z)) ))",
>> null) ;
>> }
>>
>> - @Test public void place_extend_03() {
>> + @Test public void place_extend_04() {
>> test("(filter (= ?x 123) (extend ((?x1 123)) (filter (< ?x 456)
>> (bgp (?s ?p ?x) (?s ?p ?z))) ))",
>> "(extend (?x1 123) (sequence (filter ((= ?x 123) (< ?x 456))
>> (bgp (?s ?p ?x))) (bgp (?s ?p ?z))) )") ;
>> }
>>
>> + @Test public void place_extend_05() {
>> + // Filter further out than one place.
>> + test("(filter (= ?z 1) (sequence (extend (?x1 123) (bgp (?s ?p
>> ?x))) (bgp (?s ?p ?z))))",
>> + null) ;
>> + }
>> +
>> + @Test public void place_extend_06() {
>> + // Filter further out than one place.
>> + test("(filter (= ?z 1) (join (extend (?x1 123) (bgp (?s ?p ?x)))
>> (bgp (?s ?p ?z))))" ,
>> + "(join (extend (?x1 123) (bgp (?s ?p ?x))) (filter (= ?z 1)
>> (bgp (?s ?p ?z))) )") ;
>> + }
>> +
>> @Test public void place_assign_01() {
>> test("(filter (= ?x 123) (assign ((?z 123)) (bgp (?s ?p ?x)) ))",
>> "(assign ((?z 123)) (filter (= ?x 123) (bgp (?s ?p ?x)) ))")
>> ;
>> }
>>
>> - @Test public void place_assign_02() { // Blocked
>> + @Test public void place_assign_02() {
>> + test("(filter ((= ?x1 123) (= ?x2 456)) (assign (?z 789) (bgp (?s
>> ?p ?x1)) ))",
>> + "(filter (= ?x2 456) (assign (?z 789) (filter (= ?x1 123)
>> (bgp (?s ?p ?x1)) )))") ;
>> + }
>> +
>> + @Test public void place_assign_03() { // Blocked
>> test("(filter (= ?x 123) (assign ((?x 123)) (bgp (?s ?p ?z)) ))",
>> null) ;
>> }
>>
>> - @Test public void place_assign_03() {
>> + @Test public void place_assign_04() {
>> // Caution - OpFilter equality is sensitive to the order of
>> expressions
>> test("(filter (= ?x 123) (assign ((?x1 123)) (filter (< ?x 456)
>> (bgp (?s ?p ?x) (?s ?p ?z))) ))",
>> "(assign (?x1 123) (sequence (filter ((= ?x 123) (< ?x 456))
>> (bgp (?s ?p ?x))) (bgp (?s ?p ?z))) )") ;
>>
>>
>>
>
>