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, <[email protected]> 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

Reply via email to