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 2012/06/30 18:04:30 UTC

svn commit: r1355755 - in /jena/trunk/jena-arq/src: main/java/com/hp/hpl/jena/sparql/algebra/ main/java/com/hp/hpl/jena/sparql/lang/ test/java/com/hp/hpl/jena/sparql/ test/java/com/hp/hpl/jena/sparql/algebra/ test/java/com/hp/hpl/jena/sparql/lang/

Author: andy
Date: Sat Jun 30 16:04:29 2012
New Revision: 1355755

URL: http://svn.apache.org/viewvc?rev=1355755&view=rev
Log:
SPARQL 1.1:
  Spec change to make BIND consistent between description and formal handling.
  BIND (and hence LET) act over TriplesBlock, not all the preceeding group elements.
  Changes to scope checking
  Changes to algebra generator



Added:
    jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/TestAlgebraTranslate.java
Modified:
    jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/AlgebraGenerator.java
    jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/lang/SyntaxVarScope.java
    jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/ARQTestSuite.java
    jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/TS_Algebra.java
    jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/lang/TestVarScope.java

Modified: jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/AlgebraGenerator.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/AlgebraGenerator.java?rev=1355755&r1=1355754&r2=1355755&view=diff
==============================================================================
--- jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/AlgebraGenerator.java (original)
+++ jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/AlgebraGenerator.java Sat Jun 30 16:04:29 2012
@@ -18,23 +18,20 @@
 
 package com.hp.hpl.jena.sparql.algebra;
 
-import java.util.ArrayList ;
-import java.util.Iterator ;
-import java.util.List ;
+import java.util.* ;
 
+import org.openjena.atlas.lib.Pair ;
 import org.openjena.atlas.logging.Log ;
 
 import com.hp.hpl.jena.graph.Node ;
+import com.hp.hpl.jena.graph.Triple ;
 import com.hp.hpl.jena.query.ARQ ;
 import com.hp.hpl.jena.query.Query ;
 import com.hp.hpl.jena.query.SortCondition ;
 import com.hp.hpl.jena.sparql.ARQInternalErrorException ;
 import com.hp.hpl.jena.sparql.algebra.op.* ;
 import com.hp.hpl.jena.sparql.algebra.optimize.TransformSimplify ;
-import com.hp.hpl.jena.sparql.core.BasicPattern ;
-import com.hp.hpl.jena.sparql.core.PathBlock ;
-import com.hp.hpl.jena.sparql.core.Var ;
-import com.hp.hpl.jena.sparql.core.VarExprList ;
+import com.hp.hpl.jena.sparql.core.* ;
 import com.hp.hpl.jena.sparql.engine.binding.Binding ;
 import com.hp.hpl.jena.sparql.expr.* ;
 import com.hp.hpl.jena.sparql.path.PathLib ;
@@ -44,13 +41,14 @@ import com.hp.hpl.jena.sparql.syntax.* ;
 import com.hp.hpl.jena.sparql.util.Context ;
 import com.hp.hpl.jena.sparql.util.Utils ;
 
+// Develop modifications outside of main codebase.Table
 public class AlgebraGenerator 
 {
     // Fixed filter position means leave exactly where it is syntactically (illegal SPARQL)
     // Helpful only to write exactly what you mean and test the full query compiler.
     private boolean fixedFilterPosition = false ;
     private Context context ;
-    private int subQueryDepth = 0 ;
+    private final int subQueryDepth ;
     
     // simplifyInAlgebraGeneration=true is the alternative reading of
     // the DAWG Algebra translation algorithm. 
@@ -63,14 +61,18 @@ public class AlgebraGenerator 
     static final private boolean simplifyTooEarlyInAlgebraGeneration = false ;  // False is the correct setting. 
 
     public AlgebraGenerator(Context context)
-    { 
-        if ( context == null )
-            context = ARQ.getContext().copy() ;
-        this.context = context ;
+    {
+        this (context != null ? context : ARQ.getContext().copy(), 0) ;
     }
     
     public AlgebraGenerator() { this(null) ; } 
     
+    private AlgebraGenerator(Context context, int depth)
+    {
+        this.context = context ;
+        this.subQueryDepth = depth ;
+    }
+    
     //-- Public operations.  Do not call recursively (call compileElement).
     // These operations apply the simplification step which is done, once, at the end.
     
@@ -101,12 +103,12 @@ public class AlgebraGenerator 
     // This is the operation to call for recursive application.
     protected Op compileElement(Element elt)
     {
-        if ( elt instanceof ElementUnion )
-            return compileElementUnion((ElementUnion)elt) ;
-      
         if ( elt instanceof ElementGroup )
             return compileElementGroup((ElementGroup)elt) ;
       
+        if ( elt instanceof ElementUnion )
+            return compileElementUnion((ElementUnion)elt) ;
+      
         if ( elt instanceof ElementNamedGraph )
             return compileElementGraph((ElementNamedGraph)elt) ; 
       
@@ -128,6 +130,9 @@ public class AlgebraGenerator 
         if ( elt instanceof ElementSubQuery )
             return compileElementSubquery((ElementSubQuery)elt) ; 
         
+        if ( elt instanceof ElementData )
+            return compileElementData((ElementData)elt) ; 
+
         if ( elt == null )
             return OpNull.create() ;
 
@@ -135,58 +140,49 @@ public class AlgebraGenerator 
         return null ;
     }
     
-    protected Op compileElementUnion(ElementUnion el)
-    { 
-        Op current = null ;
-        
-        for ( Element subElt: el.getElements() )
-        {
-            Op op = compileElement(subElt) ;
-            current = union(current, op) ;
-        }
-        return current ;
-    }
-    
     //Produce the algebra for a single group.
     //<a href="http://www.w3.org/TR/rdf-sparql-query/#sparqlQuery">Translation to the SPARQL Algebra</a>
     //
     // Step : (URI resolving and triple pattern syntax forms) was done during parsing
-    // Step : (Paths) e.g. simple links become triple patterns. [finalizeSyntax]
-    // Step : (BGPs) Merge BGPs   [finalizeSyntax]
-    // Step : (BIND/LET) Associate with BGP
+    // Step : Collection FILTERS [prepareGroup]
+    // Step : (Paths) e.g. simple links become triple patterns. Done later in [compileOneInGroup]
+    // Step : (BGPs) Merge PathBlocks - these are SPARQL 1.1's TriplesBlock   [prepareGroup]
+    // Step : (BIND/LET) Associate with BGP [??]
     // Step : (Groups and unions) Was done during parsing to get ElementUnion.
-    // Step : (GRAPH) Done in this code.
-    // Step : (Filter extraction and OPTIONAL) Done in this code
+    // Step : Graph Patterns [compileOneInGroup]
+    // Step : Filters [here]
     // Simplification: Done later 
-    // If simplicifation is done now, it changes OPTIONAL { { ?x :p ?w . FILTER(?w>23) } } because it removes the
+    // If simplification is done now, it changes OPTIONAL { { ?x :p ?w . FILTER(?w>23) } } because it removes the
     //   (join Z (filter...)) that in turn stops the filter getting moved into the LeftJoin.  
     //   It need a depth of 2 or more {{ }} for this to happen. 
     
     protected Op compileElementGroup(ElementGroup groupElt)
     {
+        Pair<List<Expr>, List<Element>> pair = prepareGroup(groupElt) ;
+        List<Expr> filters = pair.getLeft() ;
+        List<Element> groupElts = pair.getRight() ;
+
+        // Compile the consolidated group elements.
+        // "current" is the completed part only - there may be thing pushed into the accumulator.
         Op current = OpTable.unit() ;
+        Deque<Op> acc = new ArrayDeque<Op>() ;
         
-        // First: get all filters, merge adjacent BGPs. This includes BGP-FILTER-BGP
-        // This is done in finalizeSyntax after which the new ElementGroup is in
-        // the right order w.r.t. BGPs and filters. 
-        // 
-        // This is a delay from parsing time so a printed query
-        // keeps filters where the query author put them.
-        
-        List<Element> groupElts = finalizeSyntax(groupElt) ;
-
-        // Processing assignments is combined into the whole group processing.
-        // This includes a small amount of undoing some of the conversion work
-        // but means that the translation after finalizeSyntax is a single pass.
-        
-        // Compile the consolidated group elements.
-        // Assumes the filters have been moved to end.
         for (Iterator<Element> iter = groupElts.listIterator() ; iter.hasNext() ; )
         {
             Element elt = iter.next() ;
-            current = compileOneInGroup(elt, current) ;
+            if ( elt != null )
+                current = compileOneInGroup(elt, current, acc) ;
+        }
+        
+        // Deal with any remaining ops.
+        current = joinOpAcc(current, acc) ;
+        
+        if ( filters != null )
+        {
+            // Put filters round the group.
+            for ( Expr expr : filters )
+                current = OpFilter.filter(expr, current) ;
         }
-            
         return current ;
     }
 
@@ -196,130 +192,150 @@ public class AlgebraGenerator 
      * Return a list of elements with any filters at the end. 
      */
     
-    private List<Element> finalizeSyntax(ElementGroup groupElt)
+    private Pair<List<Expr>, List<Element>> prepareGroup(ElementGroup groupElt)
     {
-        if ( fixedFilterPosition )
-            // Illegal SPARQL
-            return groupElt.getElements() ;
-        
         List<Element> groupElts = new ArrayList<Element>() ;
-        BasicPattern prevBGP = null ;
-        List<ElementFilter> filters = null ;
-        PathBlock prevPathBlock = null ;
+        
+        PathBlock currentBGP = null ;
+        PathBlock currentPathBlock = null ;
+        List<Expr> filters = null ;
         
         for (Element elt : groupElt.getElements() )
         {
-            if ( elt instanceof ElementFilter )
+            if ( ! fixedFilterPosition && elt instanceof ElementFilter )
             {
+                // For fixed position filters, drop through to general element processing.
+                // It's also illegal SPARQL - filters operate over the whole group.
                 ElementFilter f = (ElementFilter)elt ;
                 if ( filters == null )
-                    filters = new ArrayList<ElementFilter>() ;
-                filters.add(f) ;
+                    filters = new ArrayList<Expr>() ;
+                filters.add(f.getExpr()) ;
                 // Collect filters but do not place them yet.
                 continue ;
             }
 
-            // Rather ugly code that combines blocks. 
+            // The parser does not generate ElementTriplesBlock (SPARQL 1.1) 
+            // but SPARQL 1.0 does and also we cope for programmtically built queries
             
             if ( elt instanceof ElementTriplesBlock )
             {
-                if ( prevPathBlock != null )
-                    throw new ARQInternalErrorException("Mixed ElementTriplesBlock and ElementPathBlock (case 1)") ;
-                
                 ElementTriplesBlock etb = (ElementTriplesBlock)elt ;
 
-                if ( prevBGP != null )
+                if ( currentPathBlock == null )
                 {
-                    // Previous was an ElementTriplesBlock.
-                    // Merge because they were adjacent in a group
-                    // in syntax, so it must have been BGP, Filter, BGP.
-                    // Or someone constructed a non-serializable query. 
-                    prevBGP.addAll(etb.getPattern()) ;
-                    continue ;
+                    ElementPathBlock etb2 = new ElementPathBlock() ;
+                    currentPathBlock = etb2.getPattern() ;
+                    groupElts.add(etb2) ;
                 }
-                // New BGP.
-                // Copy - so that any later mergings do not change the original query. 
-
-                ElementTriplesBlock etb2 = new ElementTriplesBlock() ;
-                etb2.getPattern().addAll(etb.getPattern()) ;
-                prevBGP = etb2.getPattern() ;
-                groupElts.add(etb2) ;
+                
+                for ( Triple t : etb.getPattern())
+                    currentPathBlock.add(new TriplePath(t)) ;
                 continue ;
             }
             
-            // TIDY UP - grr this is duplication.
-            // Can't mix ElementTriplesBlock and ElementPathBlock (which subsumes ElementTriplesBlock)
+            // To PathLib
+            
             if ( elt instanceof ElementPathBlock )
             {
-                if ( prevBGP != null )
-                    throw new ARQInternalErrorException("Mixed ElementTriplesBlock and ElementPathBlock (case 2)") ;
-                
                 ElementPathBlock epb = (ElementPathBlock)elt ;
-                if ( prevPathBlock != null )
+                // Done later.  remove when definitely OK to delay.
+//                for ( TriplePath p : epb.getPattern() )
+//                {
+//                    if ( p.isTriple() )
+//                    {}
+//                    
+//                    // Do the top level conversion.
+//                    Path path = p.getPath() ;
+//                    if ( path instanceof P_Link )
+//                    {}
+//                    else if ( path instanceof P_Inverse ) 
+//                    {
+//                        // and inverse of a link.
+//                    }
+//                    else if ( path instanceof P_Seq )
+//                    {
+//                        P_Seq seq = (P_Seq)path ;
+//                        
+//                    }
+//                    else
+//                    {}
+//                }
+                
+                
+                if ( currentPathBlock == null )
                 {
-                    prevPathBlock.addAll(epb.getPattern()) ;
-                    continue ;
+                    ElementPathBlock etb2 = new ElementPathBlock() ;
+                    currentPathBlock = etb2.getPattern() ;
+                    groupElts.add(etb2) ;
                 }
-                
-                ElementPathBlock epb2 = new ElementPathBlock() ;
-                epb2.getPattern().addAll(epb.getPattern()) ;
-                prevPathBlock = epb2.getPattern() ;
-                groupElts.add(epb2) ;
+
+                currentPathBlock.addAll(epb.getPattern()) ;
                 continue ;
             }
             
-            // Not BGP or Filter.
+            // else
+            
+            // Not BGP, path or filters.
             // Clear any BGP-related triple accumulators.
-            prevBGP = null ;
-            prevPathBlock = null ;
+            currentPathBlock = null ;
             // Add this element
             groupElts.add(elt) ;
         }
-        //End of group - put in any accumulated filters
+        return Pair.create(filters, groupElts) ;
+    }
+    
+    /** Flush the op accumulator - and clear it */
+    private void accumulate(Deque<Op> acc, Op op) { acc.addLast(op) ; }
+
+    /** Accumulate stored ops, return unit if none. */
+    private Op popAccumulated(Deque<Op> acc)
+    {
+        if ( acc.size() == 0 )
+            return OpTable.unit() ; 
         
-        if ( filters != null )
-            groupElts.addAll(filters) ;
-        return groupElts ;
+        Op joined = null ;
+        // First first to last.
+        for ( Op op : acc )
+            joined = OpJoin.create(joined,op) ;
+        acc.clear() ;
+        return joined ; 
     }
     
-    private Op compileOneInGroup(Element elt, Op current)
+    /** Join stored ops to the current state */
+    private Op joinOpAcc(Op current, Deque<Op> acc)
     {
+        if ( acc.size() == 0 ) return current ;
+        Op joined = current ;
+        // First first to last.
+        for ( Op op : acc )
+            joined = OpJoin.create(joined,op) ;
+        acc.clear() ;
+        return joined ; 
+    }
+    
+    private Op compileOneInGroup(Element elt, Op current, Deque<Op> acc)
+    {
+        // PUSH
         // Replace triple patterns by OpBGP (i.e. SPARQL translation step 1)
         if ( elt instanceof ElementTriplesBlock )
         {
+            // Should not happen.
             ElementTriplesBlock etb = (ElementTriplesBlock)elt ;
             Op op = compileBasicPattern(etb.getPattern()) ;
-            return join(current, op) ;
+            accumulate(acc, op) ;
+            return current ;
         }
         
+        // PUSH
         if ( elt instanceof ElementPathBlock )
         {
             ElementPathBlock epb = (ElementPathBlock)elt ;
             Op op = compilePathBlock(epb.getPattern()) ;
-            return join(current, op) ;
-        }
-        
-        // Filters were collected together by finalizeSyntax.
-        // So they are in the right place.
-        if ( elt instanceof ElementFilter )
-        {
-            ElementFilter f = (ElementFilter)elt ;
-            return OpFilter.filter(f.getExpr(), current) ;
-        }
-    
-        if ( elt instanceof ElementOptional )
-        {
-            ElementOptional eltOpt = (ElementOptional)elt ;
-            return compileElementOptional(eltOpt, current) ;
-        }
-        
-        if ( elt instanceof ElementSubQuery )
-        {
-            ElementSubQuery elQuery = (ElementSubQuery)elt ;
-            Op op = compileElementSubquery(elQuery) ;
-            return join(current, op) ;
+            accumulate(acc, op) ;
+            return current ;
         }
         
+        // POP
         if ( elt instanceof ElementAssign )
         {
             // This step and the similar BIND step needs to access the preceeding 
@@ -327,17 +343,60 @@ public class AlgebraGenerator 
             // That might 'current', or in the left side of a join.
             // If not a BGP, insert a empty one.  
             
+            Op op = popAccumulated(acc) ;
             ElementAssign assign = (ElementAssign)elt ;
-            Op op = OpAssign.assign(current, assign.getVar(), assign.getExpr()) ;
-            return op ;
+            Op opAssign = OpAssign.assign(op, assign.getVar(), assign.getExpr()) ;
+            accumulate(acc, opAssign) ;
+            return current ;
         }
         
+        // POP
         if ( elt instanceof ElementBind )
         {
+            Op op = popAccumulated(acc) ;
             ElementBind bind = (ElementBind)elt ;
-            Op op = OpExtend.extend(current, bind.getVar(), bind.getExpr()) ;
+            Op opExtend = OpExtend.extend(op, bind.getVar(), bind.getExpr()) ;
+            accumulate(acc, opExtend) ;
+            return current ;
+        }
+
+        // Flush.
+        current = joinOpAcc(current, acc) ; 
+        
+        if ( elt instanceof ElementOptional )
+        {
+            ElementOptional eltOpt = (ElementOptional)elt ;
+            return compileElementOptional(eltOpt, current) ;
+        }
+        
+//        if ( elt instanceof ElementSubQuery )
+//        {
+//            ElementSubQuery elQuery = (ElementSubQuery)elt ;
+//            Op op = compileElementSubquery(elQuery) ;
+//            //TODO Plain join
+//            return join(current, op) ;
+//        }
+        
+        if ( elt instanceof ElementMinus )
+        {
+            ElementMinus elt2 = (ElementMinus)elt ;
+            Op op = compileElementMinus(current, elt2) ;
             return op ;
         }
+
+        // All elements that simply "join" into the algebra.
+        if ( elt instanceof ElementGroup || 
+             elt instanceof ElementNamedGraph ||
+             elt instanceof ElementService ||
+             elt instanceof ElementFetch ||
+             elt instanceof ElementUnion || 
+             elt instanceof ElementSubQuery  ||
+             elt instanceof ElementData
+            )
+        {
+            Op op = compileElement(elt) ;
+            return join(current, op) ;
+        }
         
         if ( elt instanceof ElementExists )
         {
@@ -353,21 +412,14 @@ public class AlgebraGenerator 
             return op ;
         }
         
-        if ( elt instanceof ElementMinus )
-        {
-            ElementMinus elt2 = (ElementMinus)elt ;
-            Op op = compileElementMinus(current, elt2) ;
-            return op ;
-        }
-
-        if ( elt instanceof ElementData)
+        // Filters were collected together by prepareGroup
+        // This only handels filters left in place by some magic. 
+        if ( elt instanceof ElementFilter )
         {
-            // Accumulate, like filters.
-            ElementData elt2 = (ElementData)elt ;
-            Op op = compileElementData(current, elt2) ;
-            return op ;
+            ElementFilter f = (ElementFilter)elt ;
+            return OpFilter.filter(f.getExpr(), current) ;
         }
-        
+    
 //        // SPARQL 1.1 UNION -- did not make SPARQL 
 //        if ( elt instanceof ElementUnion )
 //        {
@@ -379,23 +431,23 @@ public class AlgebraGenerator 
 //            }
 //        }
         
-        // All other elements: compile the element and then join on to the current group expression.
-        if ( elt instanceof ElementGroup || 
-             elt instanceof ElementNamedGraph ||
-             elt instanceof ElementService ||
-             elt instanceof ElementFetch ||
-             elt instanceof ElementUnion )
-        {
-            Op op = compileElement(elt) ;
-            return join(current, op) ;
-        }
-
-        
         
         broken("compile/Element not recognized: "+Utils.className(elt)) ;
         return null ;
     }
 
+    private Op compileElementUnion(ElementUnion el)
+    { 
+        Op current = null ;
+        
+        for ( Element subElt: el.getElements() )
+        {
+            Op op = compileElement(subElt) ;
+            current = union(current, op) ;
+        }
+        return current ;
+    }
+
     private Op compileElementNotExists(Op current, ElementNotExists elt2)
     {
         Op op = compile(elt2.getElement()) ;    // "compile", not "compileElement" -- do simpliifcation  
@@ -418,10 +470,9 @@ public class AlgebraGenerator 
         return opMinus ;
     }
 
-    private Op compileElementData(Op current, ElementData elt2)
+    private Op compileElementData(ElementData elt)
     {
-        OpTable opTable = OpTable.create(elt2.getTable()) ;
-        return OpJoin.create(current, opTable) ;
+        return OpTable.create(elt.getTable()) ;
     }
 
     private Op compileElementUnion(Op current, ElementUnion elt2)
@@ -500,10 +551,8 @@ public class AlgebraGenerator 
 
     protected Op compileElementSubquery(ElementSubQuery eltSubQuery)
     {
-        subQueryDepth++ ;
-        Op sub = this.compile(eltSubQuery.getQuery()) ;
-        subQueryDepth-- ;
-        return sub ;
+        AlgebraGenerator gen = new AlgebraGenerator(context, subQueryDepth+1) ;
+        return gen.compile(eltSubQuery.getQuery()) ;
     }
     
     /** Compile query modifiers */
@@ -632,22 +681,10 @@ public class AlgebraGenerator 
 
     // -------- 
     
-    protected Op join(Op current, Op newOp)
+    private static Op join(Op current, Op newOp)
     { 
-//        if ( current instanceof OpBGP && newOp instanceof OpBGP )
-//        {
-//            OpBGP opBGP = (OpBGP)current ;
-//            opBGP.getPattern().addAll( ((OpBGP)newOp).getPattern() ) ;
-//            return current ;
-//        }
-        
         if ( simplifyTooEarlyInAlgebraGeneration && applySimplification )
-        {
-            if ( OpJoin.isJoinIdentify(current) )
-                return newOp ;
-            if ( OpJoin.isJoinIdentify(newOp) )
-                return current ;
-        }
+            return OpJoin.createReduce(current, newOp) ;
         
         return OpJoin.create(current, newOp) ;
     }

Modified: jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/lang/SyntaxVarScope.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/lang/SyntaxVarScope.java?rev=1355755&r1=1355754&r2=1355755&view=diff
==============================================================================
--- jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/lang/SyntaxVarScope.java (original)
+++ jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/sparql/lang/SyntaxVarScope.java Sat Jun 30 16:04:29 2012
@@ -237,46 +237,90 @@ public class SyntaxVarScope
     
     public static class ScopeChecker extends ElementVisitorBase
     {
-        private Collection<Var> accScope = new HashSet<Var>() ;
         public ScopeChecker() {}
         
         @Override
         public void visit(ElementGroup el)
         {
-            // There are two kinds of elements: ones that accumulate variables
-            // across the group and ones that isolate a subexpression to be joined
-            // Scoped: UNION, Group, (MINUS), SERVICE, SubSELECT
-            // Accumulating: BGPs, paths, BIND, LET, OPTIONAL.
-            // FILTER: may involve an EXISTS (not checked currently) 
+            // BIND scope rules
+            // BIND acts over the immediately preceeding ElementPathBlock, ElementTriplesBlock
+            // and any ElementBind, ElementAssign.
             
-            // Accumulate:
-            //   This BGP - used by EleemntBind.
-            //   All elements of this group - total aggregation.
-
-            accScope.clear() ;
-            
-            for ( Element e : el.getElements() )
+            for ( int i = 0 ; i < el.getElements().size() ; i++ )
             {
+                Element e = el.getElements().get(i) ;
                 // Tests.
                 if ( e instanceof ElementBind )
+                {
+                    Collection<Var> accScope = calcScopeImmediate(el.getElements(), i) ;
                     check(accScope, (ElementBind)e) ;
-                else if ( e instanceof ElementService )
+                }
+                
+                if ( e instanceof ElementService )
+                {
+                    Collection<Var> accScope = calcScopeAll(el.getElements(), i) ;
                     check(accScope, (ElementService)e) ;
-                // if joined in, the scope protects 
-                if ( ! joinedInGroup(e) )
-                    PatternVars.vars(accScope, e) ;
+                }
             }
         }
 
-        private static boolean joinedInGroup(Element e)
+        private static boolean scoped(Element e)
         {
+            // See AlgebraGenerator.compileOneInGroup
             return e instanceof ElementGroup ||
-                e instanceof ElementUnion ||
-                //e instanceof ElementOptional ||
-                e instanceof ElementService ||
-                e instanceof ElementSubQuery ;
+                   e instanceof ElementUnion ||
+                   //e instanceof ElementOptional ||
+                   e instanceof ElementService ||
+                   e instanceof ElementSubQuery || 
+                   e instanceof ElementNamedGraph ||
+                   e instanceof ElementFetch ||
+                   e instanceof ElementData ;
         }
+
         
+        private static Collection<Var> calcScopeAll(List<Element> elements, int idx)
+        {
+            return calcScope(elements, 0, idx) ;
+        }
+        
+        private static Collection<Var> calcScopeImmediate(List<Element> elements, int idx)
+        {
+            // Work backwards.
+            Collection<Var> accScope = new HashSet<Var>() ;
+            int start = idx ;
+            
+            for ( int i = idx-1 ; i >= 0 ; i-- )
+            {
+                start = i ;
+                Element e = elements.get(i) ;
+                if ( ! ( e instanceof ElementTriplesBlock ||
+                         e instanceof ElementPathBlock ||
+                         e instanceof ElementBind ||
+                         e instanceof ElementAssign ) ) 
+                    break ;
+
+                // Include.
+                // it does not matter that we work backwards.
+                PatternVars.vars(accScope, e) ;
+            }
+         
+            return accScope ;
+        }
+
+        /** Calculate scope, working forwards */
+        private static Collection<Var> calcScope(List<Element> elements, int start, int finish)
+        {
+            Collection<Var> accScope = new HashSet<Var>() ;
+            for ( int i = start ; i < finish ; i++ )
+            {
+                Element e = elements.get(i) ;
+                if ( ! scoped(e) )
+                    PatternVars.vars(accScope, e) ;
+            }
+            return accScope ;
+        }
+
+
         // Inside filters.
         
         private static void check(Collection<Var> scope, ElementBind el)

Modified: jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/ARQTestSuite.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/ARQTestSuite.java?rev=1355755&r1=1355754&r2=1355755&view=diff
==============================================================================
--- jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/ARQTestSuite.java (original)
+++ jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/ARQTestSuite.java Sat Jun 30 16:04:29 2012
@@ -125,7 +125,6 @@ public class ARQTestSuite extends TestSu
         ts.addTest(TS_SSE.suite()) ;
 
         ts.addTest(new JUnit4TestAdapter(TS_Graph.class)) ;
-        ts.addTest(new JUnit4TestAdapter(TS_Lang.class)) ;
         ts.addTest(new JUnit4TestAdapter(TS_Solver.class)) ;
         ts.addTest(new JUnit4TestAdapter(TS_Engine.class)) ; 
 

Modified: jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/TS_Algebra.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/TS_Algebra.java?rev=1355755&r1=1355754&r2=1355755&view=diff
==============================================================================
--- jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/TS_Algebra.java (original)
+++ jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/TS_Algebra.java Sat Jun 30 16:04:29 2012
@@ -31,6 +31,7 @@ import org.junit.runners.Suite ;
 @RunWith(Suite.class)
 @Suite.SuiteClasses( {
     TestVarFinder.class
+    , TestAlgebraTranslate.class
     , TestClassify.class
     , TestFilterTransform.class
     , TestTransformQuads.class

Added: jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/TestAlgebraTranslate.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/TestAlgebraTranslate.java?rev=1355755&view=auto
==============================================================================
--- jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/TestAlgebraTranslate.java (added)
+++ jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/TestAlgebraTranslate.java Sat Jun 30 16:04:29 2012
@@ -0,0 +1,144 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.hp.hpl.jena.sparql.algebra;
+
+import com.hp.hpl.jena.query.Query ;
+import com.hp.hpl.jena.query.QueryFactory ;
+import com.hp.hpl.jena.query.Syntax ;
+import com.hp.hpl.jena.sparql.algebra.Op ;
+import com.hp.hpl.jena.sparql.sse.SSE ;
+
+import org.junit.Test ;
+import org.openjena.atlas.junit.BaseTest ;
+import org.openjena.atlas.lib.StrUtils ;
+
+/** Test translation of syntax to algebra - no otpimization */ 
+public class TestAlgebraTranslate extends BaseTest 
+{
+    @Test public void translate_01() { test("?s ?p ?o", "(bgp (triple ?s ?p ?o))") ; }
+    
+    @Test public void translate_02() { test("?s ?p ?o2 . BIND(?v+1 AS ?v1)", 
+                                            "(extend [(?v1 (+ ?v 1))]",
+                                            "   (bgp [triple ?s ?p ?o2]))"
+                                            ) ; }
+    
+    @Test public void translate_03() { test("?s ?p ?o2 . LET(?v1 := ?v+1) LET(?v2 := ?v+2)",
+                                            "(assign ((?v1 (+ ?v 1)) (?v2 (+ ?v 2)))", 
+                                            "  (bgp (triple ?s ?p ?o2)))"
+                                            ) ; }
+
+    @Test public void translate_04() { test("?s ?p ?o2 . BIND(?v+1 AS ?v1) BIND(?v + 2 AS ?v2)", 
+                                            "(extend ((?v1 (+ ?v 1)) (?v2 (+ ?v 2)))", 
+                                            "  (bgp (triple ?s ?p ?o2)))"
+                                            ) ; }
+    
+    
+    @Test public void translate_05() { test("?s ?p ?o2 BIND(?v+1 AS ?v1) LET(?v2 := ?v+2)",
+                                            "(assign ((?v2 (+ ?v 2)))", 
+                                            "  (extend ((?v1 (+ ?v 1)))", 
+                                            "    (bgp (triple ?s ?p ?o2))))"
+                                            ) ; }
+
+    @Test public void translate_06() { test("?s ?p ?o2 LET(?v2 := ?v+2) BIND(?v+1 AS ?v1)",
+                                            "(extend ((?v1 (+ ?v 1)))", 
+                                            "  (assign ((?v2 (+ ?v 2)))", 
+                                            "    (bgp (triple ?s ?p ?o2))))"
+                                            ) ; }
+
+    @Test public void translate_07() { test("{ ?s ?p ?o1 . } BIND(?v+1 AS ?v1)",
+                                            "(join", 
+                                            "  (bgp (triple ?s ?p ?o1))", 
+                                            "  (extend ((?v1 (+ ?v 1)))", 
+                                            "    (table unit)))" 
+                                            ) ; }
+    
+    @Test public void translate_08() { test("BIND(5 AS ?v1)", "(extend ((?v1 5)) [table unit])") ; } 
+
+    @Test public void translate_09() { test("{ ?s ?p ?o1 . } ?s ?p ?o2 . BIND(?v+1 AS ?v1)",
+                                            "(join", 
+                                            "  (bgp (triple ?s ?p ?o1))", 
+                                            "  (extend ((?v1 (+ ?v 1)))", 
+                                            "    (bgp (triple ?s ?p ?o2))))"
+                                            ) ; }
+    
+    @Test public void translate_10() { test("?s ?p ?o2 . ?s ?p ?o3 . BIND(?v+1 AS ?v1)",
+                                            "(extend ((?v1 (+ ?v 1)))",
+                                            "   [bgp (triple ?s ?p ?o2) (triple ?s ?p ?o3)])"
+                                            ) ; } 
+    
+    
+    @Test public void translate_11() { test("{ SELECT * {?s ?p ?o2}} BIND(?o+1 AS ?v1)",
+                                            "(join",
+                                            "   [bgp (triple ?s ?p ?o2)]",
+                                            "   [extend ((?v1 (+ ?o 1)))",
+                                            "     (table unit)",
+                                            "   ])"
+                                            ) ; } 
+    
+    @Test public void translate_20() { test("?s1 ?p ?o . ?s2 ?p ?o OPTIONAL { ?s ?p3 ?o3 . ?s ?p4 ?o4 }",
+                                            "(leftjoin",
+                                            "  [bgp (?s1 ?p ?o) (?s2 ?p ?o)]",
+                                            "  [bgp (?s ?p3 ?o3) (?s ?p4 ?o4)] )") ; }
+                                            
+    @Test public void translate_21() { test("?s1 ?p ?o . ?s2 ?p ?o BIND (99 AS ?z) OPTIONAL { ?s ?p3 ?o3 . ?s ?p4 ?o4 }",
+                                            "(leftjoin",
+                                            "  (extend ((?z 99))[bgp (?s1 ?p ?o) (?s2 ?p ?o)])",
+                                            "  [bgp (?s ?p3 ?o3) (?s ?p4 ?o4)] )") ; }
+                                            
+    @Test public void translate_22() { test("BIND (99 AS ?z) OPTIONAL { ?s ?p3 ?o3 . ?s ?p4 ?o4 }",
+                                            "(leftjoin",
+                                            "  (extend ((?z 99))[table unit])",
+                                            "  [bgp (?s ?p3 ?o3) (?s ?p4 ?o4)] )") ; }
+
+    @Test public void translate_23() { test("OPTIONAL { BIND (99 AS ?z)}",
+                                            "(leftjoin",
+                                            "  [table unit]",
+                                            "  [extend ((?z 99)) (table unit)] )" ) ; }
+    
+    // Helper.  Prints the test result (check it!)
+    private static void test(String qs)
+    {
+        qs = "SELECT * {\n"+qs+"\n}" ;
+        Query query = QueryFactory.create(qs, Syntax.syntaxARQ) ;
+        AlgebraGenerator gen = new AlgebraGenerator() ;
+        Op opActual = gen.compile(query) ;
+        String x = opActual.toString() ;
+        x = x.replaceAll("\n$", "") ;
+        x = x.replace("\n", "\", \n\"") ;
+        System.out.print('"') ;
+        System.out.print(x) ;
+        System.out.println('"') ;
+        System.out.println() ;
+    }
+    
+    
+    private static void test(String qs, String... y)
+    {
+        qs = "SELECT * {\n"+qs+"\n}" ;
+        Query query = QueryFactory.create(qs, Syntax.syntaxARQ) ;
+
+        String opStr = StrUtils.strjoinNL(y) ;
+        Op opExpected = SSE.parseOp(opStr) ;
+        AlgebraGenerator gen = new AlgebraGenerator() ;
+        Op opActual = gen.compile(query) ;
+        assertEquals(opExpected, opActual) ;
+    }
+
+}
+

Modified: jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/lang/TestVarScope.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/lang/TestVarScope.java?rev=1355755&r1=1355754&r2=1355755&view=diff
==============================================================================
--- jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/lang/TestVarScope.java (original)
+++ jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/sparql/lang/TestVarScope.java Sat Jun 30 16:04:29 2012
@@ -72,17 +72,13 @@ public class TestVarScope extends BaseTe
     
     @Test public void scope_26() { scope("SELECT * { ?s ?p ?o OPTIONAL{?s ?p2 ?o2} BIND(?o2+5 AS ?z) }") ; }
 
-    @Test(expected=QueryException.class)
-    public void scope_27() { scope("SELECT * { ?s ?p ?o OPTIONAL{?s ?p2 ?o2} BIND(5 AS ?o2) }") ; }
+    @Test public void scope_27() { scope("SELECT * { ?s ?p ?o OPTIONAL{?s ?p2 ?o2} BIND(5 AS ?o2) }") ; }
 
-    @Test
-    public void scope_28() { scope("SELECT * { { ?s ?p ?o OPTIONAL{?s ?p2 ?o2} } BIND(?o+5 AS ?o2) }") ; }
+    @Test public void scope_28() { scope("SELECT * { { ?s ?p ?o OPTIONAL{?s ?p2 ?o2} } BIND(?o+5 AS ?o2) }") ; }
 
-    @Test(expected=QueryException.class)
-    public void scope_29() { scope("SELECT * { ?s ?p ?o OPTIONAL{?s ?p2 ?o2} BIND(5 AS ?o) }") ; }
+    @Test public void scope_29() { scope("SELECT * { ?s ?p ?o OPTIONAL{?s ?p2 ?o2} BIND(5 AS ?o) }") ; }
 
-    @Test
-    public void scope_30() { scope("SELECT * { { ?s ?p ?o } OPTIONAL{?s ?p2 ?o2} BIND(5 AS ?o) }") ; }
+    @Test public void scope_30() { scope("SELECT * { { ?s ?p ?o } OPTIONAL{?s ?p2 ?o2} BIND(5 AS ?o) }") ; }
     
     @Test
     public void scope_34() { scope("SELECT * { { ?s ?p ?o } UNION {?s ?p2 ?o2} BIND(5 AS ?o) }") ; }