Author: andy
Date: Wed Sep 10 09:17:53 2014
New Revision: 1623948
URL: http://svn.apache.org/r1623948
Log:
Don't call the transformation on an already transformed subop.
Modified:
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformExtendCombine.java
jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestOptimizer.java
Modified:
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformExtendCombine.java
URL:
http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformExtendCombine.java?rev=1623948&r1=1623947&r2=1623948&view=diff
==============================================================================
---
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformExtendCombine.java
(original)
+++
jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformExtendCombine.java
Wed Sep 10 09:17:53 2014
@@ -18,11 +18,11 @@
package com.hp.hpl.jena.sparql.algebra.optimize;
-import com.hp.hpl.jena.sparql.algebra.Op;
-import com.hp.hpl.jena.sparql.algebra.TransformCopy;
-import com.hp.hpl.jena.sparql.algebra.Transformer;
-import com.hp.hpl.jena.sparql.algebra.op.OpAssign;
-import com.hp.hpl.jena.sparql.algebra.op.OpExtend;
+import com.hp.hpl.jena.sparql.algebra.Op ;
+import com.hp.hpl.jena.sparql.algebra.TransformCopy ;
+import com.hp.hpl.jena.sparql.algebra.op.OpAssign ;
+import com.hp.hpl.jena.sparql.algebra.op.OpExtend ;
+import com.hp.hpl.jena.sparql.core.VarExprList ;
/**
* An optimizer that aims to combine multiple extend clauses together.
@@ -45,7 +45,21 @@ public class TransformExtendCombine exte
@Override
public Op transform(OpAssign opAssign, Op subOp) {
if (subOp instanceof OpAssign) {
- return OpAssign.assign(Transformer.transform(this, subOp),
opAssign.getVarExprList());
+ // If a variable is assigned twice, don't do anything.
+ // (assign (?x 2) (assign (?x 1) op)) => leave alone.
+ // This is the safest option in a rare case.
+ // It would be OK if addAll does a replacement without checking
+ // but having it check and complain about duplicates adds
robustness.
+ // In OpExtend, it's actually illegal.
+
+ OpAssign x = (OpAssign)subOp ;
+ VarExprList outerVarExprList = opAssign.getVarExprList() ;
+ VarExprList innerVarExprList = x.getVarExprList() ;
+
+ Op r = OpAssign.assign(x.getSubOp(), innerVarExprList) ;
+ // This contains an "if already assigned" test.
+ r = OpAssign.assign(r, outerVarExprList) ;
+ return r ;
}
return super.transform(opAssign, subOp);
}
@@ -53,7 +67,15 @@ public class TransformExtendCombine exte
@Override
public Op transform(OpExtend opExtend, Op subOp) {
if (subOp instanceof OpExtend) {
- return OpExtend.extend(Transformer.transform(this, subOp),
opExtend.getVarExprList());
+ // The case of (extend (?x e1) (extend (?x e2) ...op...))
+ // is actually illegal in SPARQL. ?x must be a fresh variable.
+ OpExtend x = (OpExtend)subOp ;
+ VarExprList outerVarExprList = opExtend.getVarExprList() ;
+ VarExprList innerVarExprList = x.getVarExprList() ;
+ Op r = OpExtend.extend(x.getSubOp(), innerVarExprList) ;
+ // This contains an "if already assigned" test.
+ r = OpExtend.extend(r, outerVarExprList) ;
+ return r ;
}
return super.transform(opExtend, subOp);
}
Modified:
jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestOptimizer.java
URL:
http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestOptimizer.java?rev=1623948&r1=1623947&r2=1623948&view=diff
==============================================================================
---
jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestOptimizer.java
(original)
+++
jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestOptimizer.java
Wed Sep 10 09:17:53 2014
@@ -30,6 +30,7 @@ import com.hp.hpl.jena.sparql.core.Var ;
import com.hp.hpl.jena.sparql.core.VarExprList ;
import com.hp.hpl.jena.sparql.expr.ExprVar ;
import com.hp.hpl.jena.sparql.expr.nodevalue.NodeValueInteger ;
+import com.hp.hpl.jena.sparql.sse.SSE ;
public class TestOptimizer extends AbstractTestTransform
{
@@ -264,6 +265,28 @@ public class TestOptimizer extends Abstr
check(extend, new TransformExtendCombine(), opExpectedString);
}
+
+ @Test public void combine_extend_04()
+ {
+ String opString = StrUtils.strjoinNL
+ ("(extend ((?x 2))"
+ ," (extend ((?y 3))"
+ ," (distinct"
+ ," (extend ((?a 'A') (?b 'B'))"
+ ," (extend ((?c 'C'))"
+ ," (table unit)"
+ ," )))))"
+ );
+ String opExpectedString = StrUtils.strjoinNL
+ ("(extend ((?y 3) (?x 2))"
+ ," (distinct"
+ ," (extend ((?c 'C') (?a 'A') (?b 'B'))"
+ ," (table unit))))");
+
+ Op op = SSE.parseOp(opString) ;
+ check(op, new TransformExtendCombine(), opExpectedString);
+ }
+
@Test public void combine_assign_01()
{
@@ -301,4 +324,26 @@ public class TestOptimizer extends Abstr
check(assign, new TransformExtendCombine(), opExpectedString);
}
+
+ @Test public void combine_assign_04()
+ {
+ String opString = StrUtils.strjoinNL
+ ("(assign ((?x 2))"
+ ," (assign ((?y 3))"
+ ," (distinct"
+ ," (assign ((?a 'A') (?b 'B'))"
+ ," (assign ((?c 'C'))"
+ ," (table unit)"
+ ," )))))"
+ );
+ String opExpectedString = StrUtils.strjoinNL
+ ("(assign ((?y 3) (?x 2))"
+ ," (distinct"
+ ," (assign ((?c 'C') (?a 'A') (?b 'B'))"
+ ," (table unit))))");
+
+ Op op = SSE.parseOp(opString) ;
+ check(op, new TransformExtendCombine(), opExpectedString);
+ }
+
}
