JENA-874 : Don't put unsatisifed filters inside distinct/reduced. If placed inside distinct/reduced, they are not available to be correctly placed, for example, working on a sequence.
Project: http://git-wip-us.apache.org/repos/asf/jena/repo Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/25d5ea45 Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/25d5ea45 Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/25d5ea45 Branch: refs/heads/master Commit: 25d5ea45dfa717f182f8773eea227aef470876f6 Parents: 58e0e6c Author: Andy Seaborne <[email protected]> Authored: Sat Jan 31 13:09:02 2015 +0000 Committer: Andy Seaborne <[email protected]> Committed: Sat Jan 31 13:09:02 2015 +0000 ---------------------------------------------------------------------- .../optimize/TransformFilterPlacement.java | 33 +++++++++-------- .../optimize/TestTransformFilterPlacement.java | 37 +++++++++++++------- 2 files changed, 43 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/jena/blob/25d5ea45/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java ---------------------------------------------------------------------- diff --git a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java index 19f02ad..2e4687f 100644 --- a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java +++ b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java @@ -643,24 +643,29 @@ public class TransformFilterPlacement extends TransformCopy { return processSubOp1(pushed, unpushed, input) ; } - // For a modifier without expressions (distinct, reduced), we push filters at least inside - // the modifier itself because does not affect scope. It may enable parallel execution. + // For a modifier without expressions (distinct, reduced), we could + // push that inside the modifier if that were all there was. But the + // expressions may be processed elsewhere in the overall algebra. + // Putting them inside the modifier would lock them here as they don't + // get returned in the Placement as "unplaced." + + // This is the cause of JENA-874. + private Placement placeDistinctReduced(ExprList exprs, OpDistinctReduced input) { Op subOp = input.getSubOp() ; Placement p = transform(exprs, subOp) ; - -// if ( p == null ) -// // If push in IFF it makes a difference further in. -// return resultNoChange(input) ; - - // Always push in. - // This is safe even if the filter contains vars not defined by the subOp - // OpDistinctReduced has the same scope inside and outside. - Op op = ( p == null ) - ? OpFilter.filter(exprs, subOp) - : OpFilter.filter(p.unplaced, p.op) ; + + if ( p == null ) + // No effect - we do not manage to make a change. + return resultNoChange(input) ; + + // Rebuild. + // We managed to place at least some expressions. + Op op = p.op ; + // Put back distinct/reduced op = input.copy(op) ; - return result(op, emptyList) ; + // Return with unplaced filters. + return result(op, p.unplaced) ; } private Placement placeTable(ExprList exprs, OpTable input) { http://git-wip-us.apache.org/repos/asf/jena/blob/25d5ea45/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java ---------------------------------------------------------------------- diff --git a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java index 33c2fc6..8ef081a 100644 --- a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java +++ b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformFilterPlacement.java @@ -327,14 +327,23 @@ public class TestTransformFilterPlacement extends BaseTest { //extends AbstractT @Test public void place_distinct_02() { test("(filter (= ?x 123) (distinct (bgp (?s ?p ?o)) ))", - "(distinct (filter (= ?x 123) (bgp (?s ?p ?o)) ))") ; + null) ; } - + + // Breaks for JENA-874 fix but correct (again) when JENA-875 applied. + // Same outcome as pre JENA-874 for different reasons. @Test public void place_distinct_03() { - test("(filter (= ?x 123) (reduced (extend ((?x 123)) (bgp (?s ?p ?o)) )))", - "(reduced (filter (= ?x 123) (extend ((?x 123)) (bgp (?s ?p ?o)) )))") ; + test("(filter (= ?x 123) (distinct (extend ((?x 123)) (bgp (?s ?p ?o)) )))", + null) ; + //"(distinct (filter (= ?x 123) (extend ((?x 123)) (bgp (?s ?p ?o)) )))") ; } + @Test public void place_distinct_04() { + test("(filter ((= ?o 456) (= ?z 987)) (distinct (bgp (?s ?p ?o) )))", + "(filter (= ?z 987) (distinct (filter (= ?o 456) (bgp (?s ?p ?o) ))))") ; + } + + @Test public void place_reduced_01() { test("(filter (= ?x 123) (reduced (bgp (?s ?p ?x)) ))", "(reduced (filter (= ?x 123) (bgp (?s ?p ?x)) ))") ; @@ -342,15 +351,17 @@ public class TestTransformFilterPlacement extends BaseTest { //extends AbstractT @Test public void place_reduced_02() { test("(filter (= ?x 123) (reduced (bgp (?s ?p ?o)) ))", - "(reduced (filter (= ?x 123) (bgp (?s ?p ?o)) ))") ; + null) ; } + // Breaks for JENA-874 fix but correct (again) when JENA-875 applied. + // Same outcome as pre JENA-874 for different reasons. @Test public void place_reduced_03() { - test("(filter (= ?x 123) (distinct (extend ((?x 123)) (bgp (?s ?p ?o)) )))", - "(distinct (filter (= ?x 123) (extend ((?x 123)) (bgp (?s ?p ?o)) )))") ; + test("(filter (= ?x 123) (reduced (extend ((?x 123)) (bgp (?s ?p ?o)) )))", + null ) ; + //"(reduced (filter (= ?x 123) (extend ((?x 123)) (bgp (?s ?p ?o)) )))") ; } - - + @Test public void place_extend_01() { test("(filter (= ?x 123) (extend ((?z 123)) (bgp (?s ?p ?x)) ))", "(extend ((?z 123)) (filter (= ?x 123) (bgp (?s ?p ?x)) ))") ; @@ -424,8 +435,8 @@ public class TestTransformFilterPlacement extends BaseTest { //extends AbstractT ," (extend ((?w 2))" ," (filter (= ?s 'S')" ," (extend ((?s 1))" - ," (distinct" - ," (filter (= ?a 'A')" + ," (filter (= ?a 'A')" + ," (distinct" ," (extend ((?a 2))" ," (filter (= ?b 'B')" ," (extend ((?b 1))" @@ -518,8 +529,8 @@ public class TestTransformFilterPlacement extends BaseTest { //extends AbstractT ," (assign ((?w 2))" ," (filter (= ?s 'S')" ," (assign ((?s 1))" - ," (distinct" - ," (filter (= ?a 'A')" + ," (filter (= ?a 'A')" + ," (distinct" ," (assign ((?a 2))" ," (filter (= ?b 'B')" ," (assign ((?b 1))"
