You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jena.apache.org by rv...@apache.org on 2014/01/27 21:59:40 UTC
svn commit: r1561847 - 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
Author: rvesse
Date: Mon Jan 27 20:59:40 2014
New Revision: 1561847
URL: http://svn.apache.org/r1561847
Log:
Fix for JENA-628, push filters down around BGPs even if pushing into BGPs themselves is disabled by user configuration
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=1561847&r1=1561846&r2=1561847&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 Mon Jan 27 20:59:40 2014
@@ -36,6 +36,7 @@ import com.hp.hpl.jena.sparql.core.Basic
import com.hp.hpl.jena.sparql.core.Var ;
import com.hp.hpl.jena.sparql.expr.Expr ;
import com.hp.hpl.jena.sparql.expr.ExprList ;
+import com.hp.hpl.jena.sparql.syntax.PatternVars;
import com.hp.hpl.jena.sparql.util.VarUtils ;
/**
@@ -70,7 +71,7 @@ public class TransformFilterPlacement ex
return placement == noChangePlacement ;
}
- public static Op transform(ExprList exprs, BasicPattern bgp) {
+ private Op transform(ExprList exprs, BasicPattern bgp) {
Placement placement = placeBGP(exprs, bgp) ;
Op op = ( placement == null ) ? new OpBGP(bgp) : placement.op ;
if ( placement != null )
@@ -78,7 +79,7 @@ public class TransformFilterPlacement ex
return op ;
}
- public static Op transform(ExprList exprs, Node graphNode, BasicPattern bgp) {
+ private Op transform(ExprList exprs, Node graphNode, BasicPattern bgp) {
Placement placement = placeQuadPattern(exprs, graphNode, bgp) ;
Op op = ( placement == null ) ? new OpQuadPattern(graphNode, bgp) : placement.op ;
if ( placement != null )
@@ -116,12 +117,10 @@ public class TransformFilterPlacement ex
Placement placement = null ;
if ( input instanceof OpBGP ) {
- if ( includeBGPs )
- placement = placeBGP(exprs, (OpBGP)input) ;
+ placement = placeBGP(exprs, (OpBGP)input) ;
}
else if ( input instanceof OpQuadPattern ) {
- if ( includeBGPs )
- placement = placeQuadPattern(exprs, (OpQuadPattern)input) ;
+ placement = placeQuadPattern(exprs, (OpQuadPattern)input) ;
}
else if ( input instanceof OpSequence )
placement = placeSequence(exprs, (OpSequence)input) ;
@@ -165,11 +164,33 @@ public class TransformFilterPlacement ex
return p ;
}
- private static Placement placeBGP(ExprList exprs, OpBGP x) {
+ private Placement placeBGP(ExprList exprs, OpBGP x) {
return placeBGP(exprs, x.getPattern()) ;
}
- private static Placement placeBGP(ExprList exprsIn, BasicPattern pattern) {
+ private Placement placeBGP(ExprList exprsIn, BasicPattern pattern) {
+ if (!includeBGPs) {
+ // Can't push into the BGP in this case but still may be able to place the filter around this BGP if all necessary variables are in-scope
+ Set<Var> vs = DS.set();
+ VarUtils.addVars(vs, pattern);
+ ExprList pushed = new ExprList();
+ ExprList unpushed = new ExprList();
+ for (Expr e : exprsIn) {
+ Set<Var> eVars = e.getVarsMentioned();
+ if (vs.containsAll(eVars)) {
+ pushed.add(e);
+ } else {
+ unpushed.add(e);
+ }
+ }
+
+ // Can't push anything into a filter around this BGP
+ if (pushed.size() == 0) return null;
+
+ // Safe to place some conditions around the BGP
+ return new Placement(OpFilter.filter(pushed, new OpBGP(pattern)), unpushed);
+ }
+
ExprList exprs = new ExprList(exprsIn) ;
Set<Var> patternVarsScope = DS.set() ;
// Any filters that depend on no variables.
@@ -222,7 +243,31 @@ public class TransformFilterPlacement ex
return placeQuadPattern(exprs, pattern.getGraphNode(), pattern.getBasicPattern()) ;
}
- private static Placement placeQuadPattern(ExprList exprsIn, Node graphNode, BasicPattern pattern) {
+ private Placement placeQuadPattern(ExprList exprsIn, Node graphNode, BasicPattern pattern) {
+ if (!includeBGPs) {
+ // Can't push into the quadpattern in this case but still may be able to place the filter around this quadpattern if all necessary variables are in-scope
+ Set<Var> vs = DS.set();
+ VarUtils.addVars(vs, pattern);
+ if (Var.isVar(graphNode))
+ vs.add(Var.alloc(graphNode));
+ ExprList pushed = new ExprList();
+ ExprList unpushed = new ExprList();
+ for (Expr e : exprsIn) {
+ Set<Var> eVars = e.getVarsMentioned();
+ if (vs.containsAll(eVars)) {
+ pushed.add(e);
+ } else {
+ unpushed.add(e);
+ }
+ }
+
+ // Can't push anything into a filter around this quadpattern
+ if (pushed.size() == 0) return null;
+
+ // Safe to place some conditions around the quadpattern
+ return new Placement(OpFilter.filter(pushed, new OpQuadPattern(graphNode, pattern)), unpushed);
+ }
+
ExprList exprs = new ExprList(exprsIn) ;
Set<Var> patternVarsScope = DS.set() ;
// Any filters that depend on no variables.
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=1561847&r1=1561846&r2=1561847&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 Mon Jan 27 20:59:40 2014
@@ -93,7 +93,7 @@ public class TestTransformFilterPlacemen
@Test public void place_sequence_04a() {
testNoBGP("(filter (= ?x 123) (sequence (bgp (?s ?p ?x1)) (bgp (?s ?p ?x)) (bgp (?s ?p ?x2)) ))",
- "(sequence (filter (= ?x 123) (sequence (bgp (?s ?p ?x1)) (bgp (?s ?p ?x)))) (bgp (?s ?p ?x2)) )") ;
+ "(sequence (bgp (?s ?p ?x1)) (filter (= ?x 123) (bgp (?s ?p ?x))) (bgp (?s ?p ?x2)))") ;
}
@Test public void place_sequence_05() {
@@ -103,7 +103,7 @@ public class TestTransformFilterPlacemen
@Test public void place_sequence_05a() {
testNoBGP("(filter (= ?x 123) (sequence (bgp (?s ?p ?x) (?s ?p ?x1)) (bgp (?s ?p ?x2)) ))",
- "(sequence (filter (= ?x 123) (bgp (?s ?p ?x) (?s ?p ?x1))) (bgp (?s ?p ?x2)) )") ;
+ "(sequence (filter (= ?x 123) (bgp (?s ?p ?x) (?s ?p ?x1))) (bgp (?s ?p ?x2)))") ;
}
@@ -124,6 +124,16 @@ public class TestTransformFilterPlacemen
null) ;
}
+ @Test public void place_sequence_09() {
+ test("(filter (= ?x 123) (sequence (bgp (?s ?p ?x1) (?s ?p ?x)) (bgp (?s ?p ?x2)) ))",
+ "(sequence (filter (= ?x 123) (bgp (?s ?p ?x1) (?s ?p ?x))) (bgp (?s ?p ?x2)) )") ;
+ }
+
+ @Test public void place_sequence_09a() {
+ testNoBGP("(filter (= ?x 123) (sequence (bgp (?s ?p ?x1) (?s ?p ?x)) (bgp (?s ?p ?x2)) ))",
+ "(sequence (filter (= ?x 123) (bgp (?s ?p ?x1) (?s ?p ?x))) (bgp (?s ?p ?x2)) )") ;
+ }
+
// Join : one sided push.
@Test public void place_join_01() {
test("(filter (= ?x 123) (join (bgp (?s ?p ?x)) (bgp (?s ?p ?z)) ))",
@@ -270,6 +280,23 @@ public class TestTransformFilterPlacemen
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))) )") ;
}
+
+ @Test public void place_assign_05() {
+ // Even with No BGP we can still wrap a BGP without breaking it
+ testNoBGP("(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_06() {
+ test("(filter (= ?x 123) (assign ((?z 123)) (bgp (?s ?p ?x) (?s ?p ?x1) )))",
+ "(assign ((?z 123)) (sequence (filter (= ?x 123) (bgp (?s ?p ?x)) ) (bgp (?s ?p ?x1)) ) )") ;
+ }
+
+ @Test public void place_assign_06a() {
+ // With No BGP we won't break up the BGP but we will still push the filter down
+ testNoBGP("(filter (= ?x 123) (assign ((?z 123)) (bgp (?s ?p ?x) (?s ?p ?x1) )))",
+ "(assign ((?z 123)) (filter (= ?x 123) (bgp (?s ?p ?x) (?s ?p ?x1)) ) )") ;
+ }
@Test public void place_filter_01() {
test("(filter (= ?x 123) (filter (= ?y 456) (bgp (?s ?p ?x) (?s ?p ?y)) ))" ,