You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jena.apache.org by an...@apache.org on 2015/01/31 14:15:28 UTC

[1/4] jena git commit: Clean formatting, fix typos.

Repository: jena
Updated Branches:
  refs/heads/master b7999c6e5 -> 4701d2dd6


Clean formatting, fix typos.

Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/58e0e6ce
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/58e0e6ce
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/58e0e6ce

Branch: refs/heads/master
Commit: 58e0e6cee9ac3225f2411092d7a114cf0f35222d
Parents: 2b79b0e
Author: Andy Seaborne <an...@apache.org>
Authored: Sat Jan 31 12:45:14 2015 +0000
Committer: Andy Seaborne <an...@apache.org>
Committed: Sat Jan 31 12:45:14 2015 +0000

----------------------------------------------------------------------
 .../optimize/TransformFilterPlacement.java      | 41 ++++++++------------
 1 file changed, 17 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/jena/blob/58e0e6ce/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 612b8b7..19f02ad 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
@@ -18,10 +18,7 @@ z * Licensed to the Apache Software Foundation (ASF) under one
 
 package com.hp.hpl.jena.sparql.algebra.optimize ;
 
-import java.util.Collection ;
-import java.util.Iterator ;
-import java.util.List ;
-import java.util.Set ;
+import java.util.* ;
 
 import org.apache.jena.atlas.lib.CollectionUtils ;
 import org.apache.jena.atlas.lib.DS ;
@@ -187,11 +184,11 @@ public class TransformFilterPlacement extends TransformCopy {
 //            placement = noChangePlacement ;
 //        }
 //        else if ( input instanceof OpSlice ) {
-//            // Not sure what he best choice is here.
+//            // Not sure what the best choice is here.
 //            placement = noChangePlacement ;
 //        }
 //        else if ( input instanceof OpTopN ) {
-//            // Not sure what he best choice is here.
+//            // Not sure what the best choice is here.
 //            placement = noChangePlacement ;
 //        }
 
@@ -524,16 +521,15 @@ public class TransformFilterPlacement extends TransformCopy {
         Op right = input.getRight() ;
         Placement pRight = transform(exprs, right) ;
         
-        // If it's placed in neitehr arm it should be passed back out for placement.
+        // If it's placed in neither arm it should be passed back out for placement.
         //
         // If it's done in both arms, then expression can be left pushed in
         // and not passed back out for placement.
         
         // If it is done in one arm and not the other, then it can be left pushed
         // in but needs to be redone for the other arm as if it were no placed at all.
-        
+
         // A filter applied twice is safe.
-        // Placement = null => nothing done => unplaced. 
         
         ExprList exprs2 = null ;
         
@@ -560,7 +556,6 @@ public class TransformFilterPlacement extends TransformCopy {
             exprs2.add(expr) ;
         }
         
-        
         Op newLeft = (pLeft == null ) ? left : pLeft.op ;
         Op newRight = (pRight == null ) ? right : pRight.op ;
         if ( exprs2 == null )
@@ -580,8 +575,8 @@ public class TransformFilterPlacement extends TransformCopy {
     }
     
     /* Complete processing for an Op1. 
-     * Having split expressions into pushed an dunpsuehd at thispoint,
-     * try to push "psuehd" down further into the subOp.
+     * Having split expressions into pushed and unpushed at this point,
+     * try to push "pushed" down further into the subOp.
      */  
     private Placement processSubOp1(ExprList pushed, ExprList unpushed, Op1 input) {
         Op opSub = input.getSubOp() ;
@@ -596,13 +591,13 @@ public class TransformFilterPlacement extends TransformCopy {
             Op op2 = input.copy(op1) ;
             return result(op2, unpushed) ;
         }
-        // Did make changes below.  Add filter for these (which includes the "pushed" at this level,
-        // now in the p.op or left in p.unplaced.
+        // We did make changes below.  Add filter for these (which includes the 
+        // "pushed" at this level, now in the p.op or left in p.unplaced.
         Op op_a = OpFilter.filter(subPlacement.unplaced, subPlacement.op) ;
         op_a =  input.copy(op_a) ;
-       return result(op_a, unpushed) ;
+        return result(op_a, unpushed) ;
     }
-
+    
     private Placement processExtendAssign(ExprList exprs, OpExtendAssign input) {
         // We assume that each (extend) and (assign) is in simple form - always one 
         // assignment.  We cope with the general form (multiple assignments)
@@ -647,8 +642,8 @@ public class TransformFilterPlacement extends TransformCopy {
         // (filter (project ...)) ===> (project (filter ...)) 
         return processSubOp1(pushed, unpushed, input) ;
     }
-    
-    // For a  modifer without expressions (distinct, reduced), we push filters at least inside
+
+    // 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.
     private Placement placeDistinctReduced(ExprList exprs, OpDistinctReduced input) {
         Op subOp = input.getSubOp() ;
@@ -716,13 +711,11 @@ public class TransformFilterPlacement extends TransformCopy {
         if ( exprs == null || exprs.isEmpty() )
             return op ;
 
-        for ( Expr expr : exprs )
-        {
-            if ( op == null )
-            {
-                op = OpTable.unit();
+        for ( Expr expr : exprs ) {
+            if ( op == null ) {
+                op = OpTable.unit() ;
             }
-            op = OpFilter.filter( expr, op );
+            op = OpFilter.filter(expr, op) ;
         }
         return op ;
     }


[4/4] jena git commit: Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/jena

Posted by an...@apache.org.
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/jena


Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/4701d2dd
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/4701d2dd
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/4701d2dd

Branch: refs/heads/master
Commit: 4701d2dd66d80637a1a2f2e388af355c57ff7bc9
Parents: e2ec1f7 b7999c6
Author: Andy Seaborne <an...@apache.org>
Authored: Sat Jan 31 13:14:51 2015 +0000
Committer: Andy Seaborne <an...@apache.org>
Committed: Sat Jan 31 13:14:51 2015 +0000

----------------------------------------------------------------------
 .../test/java/org/apache/jena/propertytable/graph/GraphCSVTest.java | 1 -
 1 file changed, 1 deletion(-)
----------------------------------------------------------------------



[3/4] jena git commit: Place filters around extend/assign when introduced variables allow it.

Posted by an...@apache.org.
Place filters around extend/assign when introduced variables allow it.

JENA-875


Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/e2ec1f74
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/e2ec1f74
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/e2ec1f74

Branch: refs/heads/master
Commit: e2ec1f745fcc83fcfb832315b7cf6fc1309ee084
Parents: 25d5ea4
Author: Andy Seaborne <an...@apache.org>
Authored: Sat Jan 31 13:13:52 2015 +0000
Committer: Andy Seaborne <an...@apache.org>
Committed: Sat Jan 31 13:13:52 2015 +0000

----------------------------------------------------------------------
 .../optimize/TransformFilterPlacement.java      | 50 +++++++++++++-------
 .../optimize/TestTransformFilterPlacement.java  | 19 ++++----
 2 files changed, 45 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/jena/blob/e2ec1f74/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 2e4687f..0f43fa2 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
@@ -599,30 +599,48 @@ public class TransformFilterPlacement extends TransformCopy {
     }
     
     private Placement processExtendAssign(ExprList exprs, OpExtendAssign input) {
-        // We assume that each (extend) and (assign) is in simple form - always one 
-        // assignment.  We cope with the general form (multiple assignments)
-        // but do not attempt reordering of assignments.
+        // We assume that each (extend) and (assign) is usually in simple form -
+        // always one assignment. We cope with the general form (multiple
+        // assignments) but do not attempt reordering of assignments.
+
+        // There are three cases:
+        // 1 - expressions that can be pushed into the subop.
+        // 2 - expressions that are covered when the extend/assign has applied.
+        // 3 - "unpushed" : expressions that are not covered even at the outermost level.
+        
         List<Var> vars1 = input.getVarExprList().getVars() ;
-        ExprList pushed = new ExprList() ;
-        ExprList unpushed = new ExprList() ;
         Op subOp = input.getSubOp() ;
+        
+        // Case 1 : Do as much inner placement as possible.
+        ExprList remaining = exprs ;
+        Placement p = transform(exprs, input.getSubOp()) ;
+        if ( p != null ) {
+            subOp = p.op ;
+            remaining = p.unplaced ;
+        }
+        
+        // Case 2 : wrapping
+        // Case 3 : unplaced
+        
+        // Variables in subop and introduced by (extend)/(assign)
         Set<Var> subVars = OpVars.fixedVars(subOp) ;
+        subVars.addAll(input.getVarExprList().getVars()) ;
         
-        for ( Expr expr : exprs ) {
+        ExprList wrapping = new ExprList() ; 
+        ExprList unplaced = new ExprList() ;
+            
+        for ( Expr expr : remaining ) {
             Set<Var> exprVars = expr.getVarsMentioned() ;
-            if ( disjoint(vars1, exprVars) && subVars.containsAll(exprVars) ) 
-                pushed.add(expr);
+            if ( subVars.containsAll(exprVars) )
+                wrapping.add(expr) ;
             else
-                unpushed.add(expr) ;
+                unplaced.add(expr) ;
         }
-                
-        if ( pushed.isEmpty() )
-            return resultNoChange(input) ;
         
-        // (filter ... (extend ... ))
-        //   ===>
-        // (extend ... (filter ... ))
-        return processSubOp1(pushed, unpushed, input) ;
+        Op result = input.copy(subOp) ;
+        if ( ! wrapping.isEmpty() )
+            result = OpFilter.filter(wrapping, result) ;
+        return result(result, unplaced) ; 
     }
 
     private Placement placeProject(ExprList exprs, OpProject input) {

http://git-wip-us.apache.org/repos/asf/jena/blob/e2ec1f74/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 8ef081a..d1826d5 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
@@ -334,8 +334,7 @@ public class TestTransformFilterPlacement extends BaseTest { //extends AbstractT
     // Same outcome as pre JENA-874 for different reasons.
     @Test public void place_distinct_03() {
         test("(filter (= ?x 123) (distinct (extend ((?x 123)) (bgp (?s ?p ?o)) )))",
-             null) ;
-            //"(distinct (filter (= ?x 123) (extend ((?x 123)) (bgp (?s ?p ?o)) )))") ;
+             "(distinct (filter (= ?x 123) (extend ((?x 123)) (bgp (?s ?p ?o)) )))") ;
     }
 
     @Test public void place_distinct_04() {
@@ -354,12 +353,16 @@ public class TestTransformFilterPlacement extends BaseTest { //extends AbstractT
              null) ;
     }
     
+    @Test public void place_reduced_04() {
+        test("(filter ((= ?o 456) (= ?z 987)) (reduced (bgp (?s ?p ?o) )))",
+             "(filter (= ?z 987) (reduced (filter (= ?o 456) (bgp (?s ?p ?o) ))))") ;
+    }
+
     // 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) (reduced (extend ((?x 123)) (bgp (?s ?p ?o)) )))",
-             null ) ;
-            //"(reduced (filter (= ?x 123) (extend ((?x 123)) (bgp (?s ?p ?o)) )))") ;
+             "(reduced (filter (= ?x 123) (extend ((?x 123)) (bgp (?s ?p ?o)) )))") ;
     }
     
     @Test public void place_extend_01() {
@@ -435,8 +438,8 @@ public class TestTransformFilterPlacement extends BaseTest { //extends AbstractT
             ,"  (extend ((?w 2))"
             ,"    (filter (= ?s 'S')"
             ,"      (extend ((?s 1))"
-            ,"        (filter (= ?a 'A')"
-            ,"          (distinct"
+            ,"        (distinct"
+            ,"          (filter (= ?a 'A')"
             ,"            (extend ((?a 2))"
             ,"              (filter (= ?b 'B')"
             ,"                (extend ((?b 1))"
@@ -529,8 +532,8 @@ public class TestTransformFilterPlacement extends BaseTest { //extends AbstractT
             ,"  (assign ((?w 2))"
             ,"    (filter (= ?s 'S')"
             ,"      (assign ((?s 1))"
-            ,"        (filter (= ?a 'A')"
-            ,"          (distinct"
+            ,"        (distinct"
+            ,"          (filter (= ?a 'A')"
             ,"            (assign ((?a 2))"
             ,"              (filter (= ?b 'B')"
             ,"                (assign ((?b 1))"


[2/4] jena git commit: JENA-874 : Don't put unsatisifed filters inside distinct/reduced.

Posted by an...@apache.org.
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 <an...@apache.org>
Authored: Sat Jan 31 13:09:02 2015 +0000
Committer: Andy Seaborne <an...@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))"