Repository: jena
Updated Branches:
  refs/heads/master a30a04ab1 -> c7a5820a6


JENA-809 : Copy input VarExprList as it may be modified.

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

Branch: refs/heads/master
Commit: c7a5820a61b1faa9285ec788391e699282618cac
Parents: a30a04a
Author: Andy Seaborne <a...@apache.org>
Authored: Sun Nov 2 16:34:08 2014 +0000
Committer: Andy Seaborne <a...@apache.org>
Committed: Sun Nov 2 16:34:08 2014 +0000

----------------------------------------------------------------------
 .../jena/sparql/algebra/optimize/Optimize.java   |  2 +-
 .../algebra/optimize/TransformExtendCombine.java |  9 ++++-----
 .../algebra/optimize/AbstractTestTransform.java  | 16 +++++++++-------
 .../sparql/algebra/optimize/TestOptimizer.java   | 19 ++++++++++++++++++-
 4 files changed, 32 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/jena/blob/c7a5820a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/Optimize.java
----------------------------------------------------------------------
diff --git 
a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/Optimize.java 
b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/Optimize.java
index 9ac4e49..150e122 100644
--- 
a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/Optimize.java
+++ 
b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/Optimize.java
@@ -247,7 +247,7 @@ public class Optimize implements Rewrite
         
         // Merge (extend) and (assign) stacks
         if ( context.isTrueOrUndef(ARQ.optMergeExtends) )
-            op = apply("Merge BGPs", new TransformExtendCombine(), op) ;
+            op = apply("Combine BIND/LET", new TransformExtendCombine(), op) ;
         
         // Mark
         if ( false )

http://git-wip-us.apache.org/repos/asf/jena/blob/c7a5820a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformExtendCombine.java
----------------------------------------------------------------------
diff --git 
a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformExtendCombine.java
 
b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformExtendCombine.java
index 444ed9e..1fab784 100644
--- 
a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformExtendCombine.java
+++ 
b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformExtendCombine.java
@@ -56,10 +56,10 @@ public class TransformExtendCombine extends TransformCopy {
 
             OpAssign x = (OpAssign)subOp ;
             VarExprList outerVarExprList = opAssign.getVarExprList() ;
-            VarExprList innerVarExprList = x.getVarExprList() ;
+            VarExprList innerVarExprList = new VarExprList(x.getVarExprList()) 
;    // Copy
             
             Op r = OpAssign.assign(x.getSubOp(), innerVarExprList) ;
-            // This contains an "if already assigned" test.
+            // This contains an "if already (assign)" test.
             r = OpAssign.assign(r, outerVarExprList) ;
             return r ;
         }
@@ -73,10 +73,9 @@ public class TransformExtendCombine extends TransformCopy {
             // is actually illegal in SPARQL.  ?x must be a fresh variable.
             OpExtend x = (OpExtend)subOp ;
             VarExprList outerVarExprList = opExtend.getVarExprList() ;
-            VarExprList innerVarExprList = x.getVarExprList() ;
-            // XXX Go back to "create"
+            VarExprList innerVarExprList = new VarExprList(x.getVarExprList()) 
;    // Copy
             Op r = OpExtend.extend(x.getSubOp(), innerVarExprList) ;
-            // This contains an "if already assigned" test.
+            // This contains an "if already (extend)" test.
             r = OpExtend.extend(r, outerVarExprList) ;
             return r ;
         }

http://git-wip-us.apache.org/repos/asf/jena/blob/c7a5820a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/AbstractTestTransform.java
----------------------------------------------------------------------
diff --git 
a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/AbstractTestTransform.java
 
b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/AbstractTestTransform.java
index 4fb7940..cffec7f 100644
--- 
a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/AbstractTestTransform.java
+++ 
b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/AbstractTestTransform.java
@@ -66,7 +66,9 @@ public abstract class AbstractTestTransform extends BaseTest {
         queryString = "PREFIX : <http://example/>\n" + queryString ;
         Query query = QueryFactory.create(queryString) ;
         Op opQuery = Algebra.compile(query) ;
+        Op op1 = Algebra.compile(query) ;   // Safe copy
         check(opQuery, opExpectedString) ;
+        assertEquals("Modification of input during optimization", op1, 
opQuery) ;
     }
 
     private static void check(Op opToOptimize, String opExpectedString) {
@@ -84,17 +86,17 @@ public abstract class AbstractTestTransform extends 
BaseTest {
     
     public static void checkAlgebra(String algString, Transform 
additionalOptimizer, String opExpectedString) {
         Op algebra = SSE.parseOp(algString) ;
-        algebra = Algebra.optimize(algebra) ;
-        algebra = Transformer.transform(additionalOptimizer, algebra);
+        Op algebra1 = SSE.parseOp(algString) ;  // Safe copy
+        Op optimized = Algebra.optimize(algebra) ;
+        if ( additionalOptimizer != null )
+            optimized = Transformer.transform(additionalOptimizer, optimized);
         Op opExpected = SSE.parseOp(opExpectedString != null ? 
opExpectedString : algString);
-        assertEquals(opExpected, algebra) ;
+        assertEquals(opExpected, optimized) ;
+        assertEquals("Modification of input during optimization", algebra1, 
algebra) ;
     }
 
     public static void checkAlgebra(String algString, String opExpectedString) 
{
-        Op algebra = SSE.parseOp(algString) ;
-        algebra = Algebra.optimize(algebra) ;
-        Op opExpected = SSE.parseOp(opExpectedString != null ? 
opExpectedString : algString);
-        assertEquals(opExpected, algebra) ;
+        checkAlgebra(algString, null, opExpectedString); 
     }
 
 }

http://git-wip-us.apache.org/repos/asf/jena/blob/c7a5820a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestOptimizer.java
----------------------------------------------------------------------
diff --git 
a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestOptimizer.java
 
b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestOptimizer.java
index fdf57b7..2b71ea4 100644
--- 
a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestOptimizer.java
+++ 
b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestOptimizer.java
@@ -38,7 +38,7 @@ public class TestOptimizer extends AbstractTestTransform
     // A lot of the optimizer is tested by using the scripted queries.
     // Theer are many tests of individual transforms.
     
-    @Test public void slice_order_to_topn_01()
+    @Test public void slice_order_to_topn_01() 
     {
         assertTrue(ARQ.isTrueOrUndef(ARQ.optTopNSorting)) ;
         String queryString = "SELECT * { ?s ?p ?o } ORDER BY ?p ?o LIMIT 42"  
;  
@@ -287,6 +287,14 @@ public class TestOptimizer extends AbstractTestTransform
         check(op, new TransformExtendCombine(), opExpectedString);
     }
 
+    @Test public void combine_extend_05()
+    {
+        // JENA-809 : check no changes to input.
+        String x = "(project (?x) (extend ((?bar 2)) (extend ((?foo 1)) (table 
unit))))" ;
+        String y = "(project (?x) (extend ((?foo 1) (?bar 2)) (table unit)))" ;
+        AbstractTestTransform.checkAlgebra(x, new TransformExtendCombine(), y);
+    }
+
         
     @Test public void combine_assign_01()
     {
@@ -345,5 +353,14 @@ public class TestOptimizer extends AbstractTestTransform
         Op op = SSE.parseOp(opString) ;
         check(op, new TransformExtendCombine(), opExpectedString);
     }
+    
+    @Test public void combine_assign_05()
+    {
+        // JENA-809 : check no changes to input.
+        String x = "(project (?x) (assign ((?bar 2)) (assign ((?foo 1)) (table 
unit))))" ;
+        String y = "(project (?x) (assign ((?foo 1) (?bar 2)) (table unit)))" ;
+        AbstractTestTransform.checkAlgebra(x, new TransformExtendCombine(), y);
+    }
+
 
 }

Reply via email to