Author: andy
Date: Sat Jul 19 20:29:30 2014
New Revision: 1611957
URL: http://svn.apache.org/r1611957
Log:
JENA-734 : Don't move expressions involving unstable functions.
Modified:
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformFilterPlacement.java
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/Expr.java
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprLib.java
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprVisitorFunction.java
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprWalker.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=1611957&r1=1611956&r2=1611957&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
Sat Jul 19 20:29:30 2014
@@ -31,8 +31,7 @@ import com.hp.hpl.jena.sparql.algebra.Tr
import com.hp.hpl.jena.sparql.algebra.op.* ;
import com.hp.hpl.jena.sparql.core.BasicPattern ;
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.expr.* ;
import com.hp.hpl.jena.sparql.util.VarUtils ;
/**
@@ -100,11 +99,41 @@ public class TransformFilterPlacement ex
@Override
public Op transform(OpFilter opFilter, Op x) {
ExprList exprs = opFilter.getExprs() ;
+
+ // Extract any expressions with "nasty" cases (RAND, UUID, STRUUID and
BNODE)
+ // which are not true functions (they return a different value every
call so
+ // number of calls matters. NOW is safe (returns a fixed time point
for the whole
+ // query.
+
+ // Phase one - check to see if work needed.
+ ExprList exprs2 = null ;
+ for ( Expr expr : exprs ) {
+ if ( ! ExprLib.isStable(expr) ) {
+ if ( exprs2 == null )
+ exprs2 = new ExprList() ;
+ exprs2.add(expr) ;
+ }
+ }
+
+ // Phase 2 - if needed, split.
+ if ( exprs2 != null ) {
+ ExprList exprs1 = new ExprList() ;
+ for ( Expr expr : exprs ) {
+ // We are assuming fixup is rare.
+ if ( ExprLib.isStable(expr) )
+ exprs1.add(expr) ;
+ }
+ exprs = exprs1 ;
+ }
+
Placement placement = transform(exprs, x) ;
if ( placement == null )
// Didn't do anything.
return super.transform(opFilter, x) ;
Op op = buildFilter(placement) ;
+ if ( exprs2 != null )
+ // Add back the non-deterministic expressions
+ op = OpFilter.filter(exprs2, op );
return op ;
}
Modified:
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/Expr.java
URL:
http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/Expr.java?rev=1611957&r1=1611956&r2=1611957&view=diff
==============================================================================
--- jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/Expr.java
(original)
+++ jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/Expr.java Sat
Jul 19 20:29:30 2014
@@ -46,16 +46,11 @@ public interface Expr
*/
public boolean isSatisfied(Binding binding, FunctionEnv execCxt) ;
- /** Variables used by this expression - excludes variables scoped to
(NOT)EXISTS*/
+ /** Variables used by this expression - excludes variables scoped to
(NOT)EXISTS */
public Set<Var> getVarsMentioned() ;
- /** Variables used by this expression - excludes variables scoped to
(NOT)EXISTS*/
+ /** Variables used by this expression - excludes variables scoped to
(NOT)EXISTS */
public void varsMentioned(Collection<Var> acc) ;
- /** Return true iff this constraint is implemened by something in the expr
package
- * and hence is fully integrated or compatible with the visitors.
- * @return boolean
- */
-
/** Evaluate this expression against the binding
* @param binding
* @param env
Modified:
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprLib.java
URL:
http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprLib.java?rev=1611957&r1=1611956&r2=1611957&view=diff
==============================================================================
--- jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprLib.java
(original)
+++ jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprLib.java
Sat Jul 19 20:29:30 2014
@@ -124,5 +124,39 @@ public class ExprLib
throw new ARQInternalErrorException() ;
}
+ /** Some "functions" are non-deterministic (unstable) -
+ * calling them with the same arguments
+ * does not yields the same answer each time.
+ * Therefore how and when they are called
+ * matters.
+ *
+ * Functions: RAND, UUID, StrUUID, BNode
+ *
+ * NOW() is safe.
+ */
+ public static boolean isStable(Expr expr) {
+ try {
+ ExprWalker.walk(exprVisitorCheckForNonFunctions, expr) ;
+ return true ;
+ } catch ( ExprUnstable ex ) {
+ return false ;
+ }
+ }
+
+ private static ExprVisitor exprVisitorCheckForNonFunctions = new
ExprVisitorBase() {
+ @Override
+ public void visit(ExprFunction0 func) {
+ if ( func instanceof E_Random ||
+ func instanceof E_UUID ||
+ func instanceof E_StrUUID)
+ throw new ExprUnstable() ;
+ }
+ @Override
+ public void visit(ExprFunctionN func) {
+ if (func instanceof E_BNode )
+ throw new ExprUnstable() ;
+ }
+ } ;
+ private static class ExprUnstable extends ExprException {}
}
Modified:
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprVisitorFunction.java
URL:
http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprVisitorFunction.java?rev=1611957&r1=1611956&r2=1611957&view=diff
==============================================================================
---
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprVisitorFunction.java
(original)
+++
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprVisitorFunction.java
Sat Jul 19 20:29:30 2014
@@ -18,7 +18,7 @@
package com.hp.hpl.jena.sparql.expr;
-/** Convert all visit calls on the expressions in a call to a generic visit
operation for expession functions */
+/** Convert all visit calls on the expressions in a call to a generic visit
operation for expression functions */
public abstract class ExprVisitorFunction implements ExprVisitor
{
@Override
Modified:
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprWalker.java
URL:
http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprWalker.java?rev=1611957&r1=1611956&r2=1611957&view=diff
==============================================================================
---
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprWalker.java
(original)
+++
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprWalker.java
Sat Jul 19 20:29:30 2014
@@ -17,9 +17,8 @@
*/
package com.hp.hpl.jena.sparql.expr;
-// Walk the expression tree, bottom up.
-// NOT FINISHED
-public class ExprWalker //implements ExprVisitor
+/** Walk the expression tree, bottom up */
+public class ExprWalker
{
ExprVisitor visitor ;
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=1611957&r1=1611956&r2=1611957&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
Sat Jul 19 20:29:30 2014
@@ -71,8 +71,8 @@ public class TestTransformFilterPlacemen
@Test public void place_bgp_06() {
- test("(filter (isURI ?g) (quadpattern (?g ?s ?p ?o) ))",
- null) ;
+ testNoChange("(filter (isURI ?g) (quadpattern (?g ?s ?p ?o) ))") ;
+
}
@Test public void place_bgp_06a() {
@@ -154,15 +154,15 @@ public class TestTransformFilterPlacemen
@Test public void place_no_match_01() {
// Unbound
- test("(filter (= ?x ?unbound) (bgp (?s ?p ?x)))", null) ;
+ testNoChange("(filter (= ?x ?unbound) (bgp (?s ?p ?x)))") ;
}
@Test public void place_no_match_02() {
- test("(filter (= ?x ?unbound) (bgp (?s ?p ?x) (?s ?p ?x)))", null) ;
+ testNoChange("(filter (= ?x ?unbound) (bgp (?s ?p ?x) (?s ?p ?x)))") ;
}
@Test public void place_no_match_03() {
- test("(filter (= ?x ?unbound) (bgp (?s ?p ?x) (?s1 ?p1 ?XX)))", null) ;
+ testNoChange("(filter (= ?x ?unbound) (bgp (?s ?p ?x) (?s1 ?p1
?XX)))") ;
}
@Test public void place_sequence_01() {
@@ -210,13 +210,11 @@ public class TestTransformFilterPlacemen
}
@Test public void place_sequence_07() {
- test("(filter (= ?A 123) (sequence (bgp (?s ?p ?x)) (bgp (?s ?p ?z))
))",
- null) ;
+ testNoChange("(filter (= ?A 123) (sequence (bgp (?s ?p ?x)) (bgp (?s
?p ?z)) ))") ;
}
@Test public void place_sequence_08() {
- test("(sequence (bgp (?s ?p ?x)) (filter (= ?z 123) (bgp (?s ?p ?z)))
)",
- null) ;
+ testNoChange("(sequence (bgp (?s ?p ?x)) (filter (= ?z 123) (bgp (?s
?p ?z))) )") ;
}
@Test public void place_sequence_09() {
@@ -314,8 +312,7 @@ public class TestTransformFilterPlacemen
}
@Test public void place_project_02() {
- test("(filter (= ?x 123) (project (?s) (bgp (?s ?p ?x)) ))",
- null) ;
+ testNoChange("(filter (= ?x 123) (project (?s) (bgp (?s ?p ?x)) ))") ;
}
@Test public void place_project_03() {
@@ -597,12 +594,69 @@ public class TestTransformFilterPlacemen
) ;
test( in, out ) ;
}
+
+ @Test public void nondeterministic_functions_01() {
+ testNoChange("(filter (= ?x (rand)) (bgp (?s ?p ?x) (?s1 ?p1 ?x)))") ;
+ }
+ @Test public void nondeterministic_functions_02() {
+ testNoChange("(filter (= ?x (bnode)) (bgp (?s ?p ?x) (?s1 ?p1 ?x)))") ;
+ }
+
+ @Test public void nondeterministic_functions_03() {
+ testNoChange("(filter (= ?x (struuid)) (bgp (?s ?p ?x) (?s1 ?p1
?x)))") ;
+ }
+
+ @Test public void nondeterministic_functions_04() {
+ testNoChange("(filter (= ?x (uuid)) (bgp (?s ?p ?x) (?s1 ?p1 ?x)))") ;
+ }
+
+ // NOW() is safe.
+ @Test public void nondeterministic_functions_05() {
+ test("(filter (= ?x (now)) (bgp (?s ?p ?x) (?s1 ?p1 ?x)))",
+ "(sequence (filter (= ?x (now)) (bgp (?s ?p ?x) )) (bgp (?s1
?p1 ?x)) )") ;
+ }
+
+ @Test public void nondeterministic_functions_06() {
+ String in = StrUtils.strjoinNL
+ ("(filter ( (!= ?x ?s) (= ?x (rand)) )"
+ ," (bgp (?s ?p ?x) (?s1 ?p1 ?x))"
+ ,")") ;
+ String out = StrUtils.strjoinNL
+ ("(filter (= ?x (rand)) "
+ ," (sequence"
+ ," (filter (!= ?x ?s) (bgp (?s ?p ?x)))"
+ ," (bgp (?s1 ?p1 ?x))"
+ ,"))"
+ ) ;
+ test(in,out) ;
+ }
+
+ @Test public void nondeterministic_functions_07() {
+ String in = StrUtils.strjoinNL
+ ("(filter ( (!= ?x ?s) (|| ?x (rand)) )"
+ ," (bgp (?s ?p ?x) (?s1 ?p1 ?x))"
+ ,")") ;
+ String out = StrUtils.strjoinNL
+ ("(filter (|| ?x (rand)) "
+ ," (sequence"
+ ," (filter (!= ?x ?s) (bgp (?s ?p ?x)))"
+ ," (bgp (?s1 ?p1 ?x))"
+ ,"))"
+ ) ;
+ test(in,out) ;
+ }
+
+
+ public static void testNoChange(String input) {
+ test(input, null) ;
+ }
+
public static void test(String input, String output) {
test$(input, output, true) ;
}
- public static void testNoBGP(String input , String output ) {
+ public static void testNoBGP(String input , String output) {
test$(input, output, false) ;
}
