jena-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [2/4] jena git commit: JENA-874 : Don't put unsatisifed filters inside distinct/reduced.
Date Sat, 31 Jan 2015 13:15:29 GMT
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 <andy@apache.org>
Authored: Sat Jan 31 13:09:02 2015 +0000
Committer: Andy Seaborne <andy@apache.org>
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))"


Mime
View raw message