You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jena.apache.org by da...@apache.org on 2011/12/01 19:02:21 UTC
svn commit: r1209166 - in /incubator/jena/Jena2/ARQ/trunk/src:
main/java/com/hp/hpl/jena/sparql/algebra/OpAsQuery.java
test/java/com/hp/hpl/jena/sparql/algebra/OpAsQueryTest.java
Author: damian
Date: Thu Dec 1 18:02:20 2011
New Revision: 1209166
URL: http://svn.apache.org/viewvc?rev=1209166&view=rev
Log:
Fix JENA-166
Modified:
incubator/jena/Jena2/ARQ/trunk/src/main/java/com/hp/hpl/jena/sparql/algebra/OpAsQuery.java
incubator/jena/Jena2/ARQ/trunk/src/test/java/com/hp/hpl/jena/sparql/algebra/OpAsQueryTest.java
Modified: incubator/jena/Jena2/ARQ/trunk/src/main/java/com/hp/hpl/jena/sparql/algebra/OpAsQuery.java
URL: http://svn.apache.org/viewvc/incubator/jena/Jena2/ARQ/trunk/src/main/java/com/hp/hpl/jena/sparql/algebra/OpAsQuery.java?rev=1209166&r1=1209165&r2=1209166&view=diff
==============================================================================
--- incubator/jena/Jena2/ARQ/trunk/src/main/java/com/hp/hpl/jena/sparql/algebra/OpAsQuery.java (original)
+++ incubator/jena/Jena2/ARQ/trunk/src/main/java/com/hp/hpl/jena/sparql/algebra/OpAsQuery.java Thu Dec 1 18:02:20 2011
@@ -40,6 +40,9 @@ 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.expr.ExprTransformCopy;
+import com.hp.hpl.jena.sparql.expr.ExprTransformer;
+import com.hp.hpl.jena.sparql.expr.ExprVar;
import com.hp.hpl.jena.sparql.pfunction.PropFuncArg ;
import com.hp.hpl.jena.sparql.syntax.* ;
import com.hp.hpl.jena.sparql.util.graph.GraphList ;
@@ -383,62 +386,53 @@ public class OpAsQuery
@Override
public void visit(OpAssign opAssign)
{
- /**
- * Special case: group involves and internal assignment
- * e.g. (assign ((?.1 ?.0)) (group () ((?.0 (count)))
- * We attempt to intercept that here.
- */
- if (opAssign.getSubOp() instanceof OpGroup) {
- Map<Var, Var> subs = new HashMap<Var, Var>();
- for (Var v: opAssign.getVarExprList().getVars())
- {
- Expr exp = opAssign.getVarExprList().getExpr(v);
- if (exp.isVariable())
- subs.put(exp.asVar(), v) ;
- else
- throw new ARQNotImplemented("Expected simple assignment for group (OpAssign): "+exp) ;
- }
- visit((OpGroup) opAssign.getSubOp(), subs);
- return;
- }
-
- opAssign.getSubOp().visit(this) ;
+ opAssign.getSubOp().visit(this) ;
+
+ // Go through each var and get the assigned expression
for ( Var v : opAssign.getVarExprList().getVars() )
{
- Element elt = new ElementAssign(v, opAssign.getVarExprList().getExpr(v)) ;
- ElementGroup g = currentGroup() ;
- g.addElement(elt) ;
+ Expr e = opAssign.getVarExprList().getExpr(v);
+
+ // Substitute group aggregate expressions in for generated vars
+ SubExprForVar sefr = new SubExprForVar(varExpression);
+ Expr tr = ExprTransformer.transform(sefr, e);
+
+ // If in top level we defer assignment to SELECT section
+ // This also covers the GROUP recombine
+ // NOTE: this means we can't round trip top-level BINDs
+ if (inTopLevel()) {
+ varExpression.put(v, tr);
+ } else {
+ Element elt = new ElementAssign(v, e) ;
+ ElementGroup g = currentGroup() ;
+ g.addElement(elt) ;
+ }
}
}
@Override
public void visit(OpExtend opExtend)
{
- /**
- * Special case: group involves and internal assignment
- * e.g. (assign ((?.1 ?.0)) (group () ((?.0 (count)))
- * We attempt to intercept that here.
- */
- if (opExtend.getSubOp() instanceof OpGroup) {
- Map<Var, Var> subs = new HashMap<Var, Var>();
- for (Var v: opExtend.getVarExprList().getVars())
- {
- Expr exp = opExtend.getVarExprList().getExpr(v);
- if (exp.isVariable())
- subs.put(exp.asVar(), v) ;
- else
- throw new ARQNotImplemented("Expected simple assignment for group (OpExtend): "+exp) ;
- }
- visit((OpGroup) opExtend.getSubOp(), subs);
- return;
- }
-
opExtend.getSubOp().visit(this) ;
+
+ // Go through each var and get the assigned expression
for ( Var v : opExtend.getVarExprList().getVars() )
{
- Element elt = new ElementBind(v, opExtend.getVarExprList().getExpr(v)) ;
- ElementGroup g = currentGroup() ;
- g.addElement(elt) ;
+ Expr e = opExtend.getVarExprList().getExpr(v);
+
+ // Substitute group aggregate expressions in for generated vars
+ Expr tr = ExprTransformer.transform(new SubExprForVar(varExpression), e);
+
+ // If in top level we defer assignment to SELECT section
+ // This also covers the GROUP recombine
+ // NOTE: this means we can't round trip top-level BINDs
+ if (inTopLevel()) {
+ varExpression.put(v, tr);
+ } else {
+ Element elt = new ElementBind(v, tr) ;
+ ElementGroup g = currentGroup() ;
+ g.addElement(elt) ;
+ }
}
}
@@ -490,25 +484,18 @@ public class OpAsQuery
}
@Override
- @SuppressWarnings("unchecked")
- public void visit(OpGroup opGroup) {
- visit(opGroup, Collections.EMPTY_MAP);
- }
-
- // Specialised to cope with the preceeding assigns
- private void visit(OpGroup opGroup, Map<Var, Var> subs) {
+ public void visit(OpGroup opGroup) {
List<ExprAggregator> a = opGroup.getAggregators();
-
+
+ // Aggregators are broken up in the algebra, split between a
+ // group and an assignment (extend or assign) using a generated var.
+ // We record them here and insert later.
for (ExprAggregator ea : a) {
// Substitute generated var for actual
Var givenVar = ea.getAggVar().asVar();
- Var realVar = (subs.containsKey(givenVar))
- ? subs.get(givenVar)
- : givenVar;
// Copy aggregator across (?)
Expr myAggr = query.allocAggregate(ea.getAggregator());
- varExpression.put(realVar, myAggr);
- //query.addResultVar(realVar, myAggr);
+ varExpression.put(givenVar, myAggr);
}
VarExprList b = opGroup.getGroupVars();
@@ -575,5 +562,30 @@ public class OpAsQuery
}
private ElementGroup pop() { return stack.pop(); }
private void push(ElementGroup el) { stack.push(el); }
+ private boolean inTopLevel() { return stack.size() == 0; }
+ }
+
+ /**
+ * This class is used to take substitute an expressions for variables
+ * in another expression. It is used to stick grouping expressions back
+ * together.
+ */
+ public static class SubExprForVar extends ExprTransformCopy {
+ private final Map<Var, Expr> varExpr;
+ private boolean subOccurred = false;
+ public SubExprForVar(Map<Var, Expr> varExpr) {
+ this.varExpr = varExpr;
+ }
+
+ public boolean didChange() { return subOccurred; }
+
+ @Override
+ public Expr transform(ExprVar var) {
+ if (varExpr.containsKey(var.asVar())) {
+ subOccurred = true;
+ return varExpr.get(var.asVar()).deepCopy();
+ }
+ else return var.deepCopy();
+ }
}
}
Modified: incubator/jena/Jena2/ARQ/trunk/src/test/java/com/hp/hpl/jena/sparql/algebra/OpAsQueryTest.java
URL: http://svn.apache.org/viewvc/incubator/jena/Jena2/ARQ/trunk/src/test/java/com/hp/hpl/jena/sparql/algebra/OpAsQueryTest.java?rev=1209166&r1=1209165&r2=1209166&view=diff
==============================================================================
--- incubator/jena/Jena2/ARQ/trunk/src/test/java/com/hp/hpl/jena/sparql/algebra/OpAsQueryTest.java (original)
+++ incubator/jena/Jena2/ARQ/trunk/src/test/java/com/hp/hpl/jena/sparql/algebra/OpAsQueryTest.java Thu Dec 1 18:02:20 2011
@@ -57,11 +57,57 @@ public class OpAsQueryTest {
assertEquals(result[0], result[1]);
}
+ /* JENA-166 */
+ @Test
+ public void testGroupWithExpression() {
+ Object[] result = checkQuery("SELECT (sample(?a) + 1 AS ?c) {} GROUP BY ?x");
+ assertEquals(result[0], result[1]);
+ }
+
+ // The next two tests represent an loss of information
+ // I suspect the best solution is to let this pass and the other fail
+ @Test
+ public void testProject() {
+ Object[] result = checkQuery("SELECT (?x + 1 AS ?c) {}");
+ assertEquals(result[0], result[1]);
+ }
+
+ // Ambiguous, don't bother
+ public void testBind() {
+ Object[] result = checkQuery("SELECT ?c { BIND(?x + 1 AS ?c) }");
+ assertEquals(result[0], result[1]);
+ }
+
+ // This BIND is distinguisable, however
+ @Test
+ public void testNestedBind() {
+ Object[] result = checkQuery("SELECT ?c { { } UNION { BIND(?x + 1 AS ?c) } }");
+ assertEquals(result[0], result[1]);
+ }
+
+ @Test
+ public void testNestedProject() {
+ Object[] result = checkQuery("SELECT (?x + 1 AS ?c) { { } UNION { } }");
+ assertEquals(result[0], result[1]);
+ }
+
+ @Test
+ public void testGroupExpression() {
+ Object[] result = checkQuery("SELECT ?z { } GROUP BY (?x + ?y AS ?z)");
+ assertEquals(result[0], result[1]);
+ }
+
+ @Test
+ public void testNestedProjectWithGroup() {
+ Object[] result = checkQuery("SELECT (SAMPLE(?c) as ?s) { {} UNION {BIND(?x + 1 AS ?c)} } GROUP BY ?x");
+ assertEquals(result[0], result[1]);
+ }
+
public Object[] checkQuery(String query) {
Query orig = QueryFactory.create(query, Syntax.syntaxSPARQL_11);
- Op a = Algebra.compile(orig);
- Query got = OpAsQuery.asQuery(a);
- Object[] r = { a, Algebra.compile(got) };
+ Op toReconstruct = Algebra.compile(orig);
+ Query got = OpAsQuery.asQuery(toReconstruct);
+ Object[] r = { orig, got };
return r;
}
}