You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jena.apache.org by rv...@apache.org on 2015/07/08 16:11:59 UTC
[1/7] jena git commit: Don't inline assignments in some unsafe cases
(JENA-780)
Repository: jena
Updated Branches:
refs/heads/jena2 7e2c9527f -> 9bc5aa972
refs/heads/master d0d7664ae -> 833bc7049
Don't inline assignments in some unsafe cases (JENA-780)
There are some cases where inlining is unsafe around moving an
assignment from within an n-ary expression e.g. join, leftjoin, union,
minus etc because the assignments value could be dependent on the branch
of the query it occurs in and moving it outside the branching operator
could change the semantics of its evaluation.
This commit disables inlining in those cases and adds extra test cases
to ensure that those cases are not subject to inlining.
Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/166b31fe
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/166b31fe
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/166b31fe
Branch: refs/heads/jena2
Commit: 166b31fe093c181012ae81d3ae2ca2d534640710
Parents: 7e2c952
Author: Rob Vesse <rv...@apache.org>
Authored: Tue Jul 7 17:28:14 2015 +0100
Committer: Rob Vesse <rv...@apache.org>
Committed: Tue Jul 7 17:28:14 2015 +0100
----------------------------------------------------------------------
.../optimize/TransformEliminateAssignments.java | 32 ++++++++++
.../algebra/optimize/VariableUsageVisitor.java | 8 ++-
.../TestTransformEliminateAssignments.java | 65 +++++++++++++++++++-
3 files changed, 99 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/jena/blob/166b31fe/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
index e984c07..7044cc9 100644
--- a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
+++ b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
@@ -38,9 +38,13 @@ import com.hp.hpl.jena.sparql.algebra.op.OpExt;
import com.hp.hpl.jena.sparql.algebra.op.OpExtend;
import com.hp.hpl.jena.sparql.algebra.op.OpFilter;
import com.hp.hpl.jena.sparql.algebra.op.OpGroup;
+import com.hp.hpl.jena.sparql.algebra.op.OpJoin;
+import com.hp.hpl.jena.sparql.algebra.op.OpLeftJoin;
+import com.hp.hpl.jena.sparql.algebra.op.OpMinus;
import com.hp.hpl.jena.sparql.algebra.op.OpOrder;
import com.hp.hpl.jena.sparql.algebra.op.OpProject;
import com.hp.hpl.jena.sparql.algebra.op.OpTopN;
+import com.hp.hpl.jena.sparql.algebra.op.OpUnion;
import com.hp.hpl.jena.sparql.core.Var;
import com.hp.hpl.jena.sparql.core.VarExprList;
import com.hp.hpl.jena.sparql.expr.Expr;
@@ -532,5 +536,33 @@ public class TransformEliminateAssignments extends TransformCopy {
this.tracker.decrementDepth();
}
+ @Override
+ public void visit(OpUnion opUnion) {
+ unsafe();
+ }
+
+ @Override
+ public void visit(OpLeftJoin opLeftJoin) {
+ // TODO Technically if the assignment is single use and comes from
+ // the LHS we could keep it but for now we don't try and do this
+ unsafe();
+ }
+
+ @Override
+ public void visit(OpMinus opMinus) {
+ unsafe();
+ }
+
+ @Override
+ public void visit(OpJoin opJoin) {
+ unsafe();
+ }
+
+ private void unsafe() {
+ // Throw out any assignments because if they would be eligible their
+ // values can't be bound in every branch of the union and thus
+ // inlining could change the semantics
+ tracker.getAssignments().clear();
+ }
}
}
http://git-wip-us.apache.org/repos/asf/jena/blob/166b31fe/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/VariableUsageVisitor.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/VariableUsageVisitor.java b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/VariableUsageVisitor.java
index 664c7b0..3c34161 100644
--- a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/VariableUsageVisitor.java
+++ b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/VariableUsageVisitor.java
@@ -64,7 +64,7 @@ public abstract class VariableUsageVisitor extends OpVisitorBase {
protected abstract void action(Var var);
protected abstract void action(String var);
-
+
@Override
public void visit(OpBGP opBGP) {
Collection<Var> vars = new ArrayList<>();
@@ -115,8 +115,10 @@ public abstract class VariableUsageVisitor extends OpVisitorBase {
@Override
public void visit(OpLeftJoin opLeftJoin) {
Collection<Var> vars = new ArrayList<>();
- for (Expr expr : opLeftJoin.getExprs().getList()) {
- ExprVars.varsMentioned(vars, expr);
+ if (opLeftJoin.getExprs() != null) {
+ for (Expr expr : opLeftJoin.getExprs().getList()) {
+ ExprVars.varsMentioned(vars, expr);
+ }
}
action(vars);
}
http://git-wip-us.apache.org/repos/asf/jena/blob/166b31fe/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
index fa16c94..4f3a1a4 100644
--- a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
+++ b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
@@ -145,7 +145,7 @@ public class TestTransformEliminateAssignments {
" (table unit)))");
//@formatter:on
}
-
+
@Test
public void extend_02() {
// Assigned variable used only once can substitute expression for the
@@ -161,7 +161,7 @@ public class TestTransformEliminateAssignments {
" (table unit)))");
//@formatter:on
}
-
+
@Test
public void extend_03() {
// Assigned variable used only once can substitute expression for the
@@ -379,7 +379,66 @@ public class TestTransformEliminateAssignments {
" (bgp (triple ?x ?y ?z)))))");
//@formatter:on
}
-
+
+ @Test
+ public void ineligible_05() {
+ // Don't inline if we'd move it from within a n-ary operator as doing so
+ // may change the queries semantics
+ //@formatter:off
+ testNoChange("(project (?s)",
+ " (filter (exprlist ?x)",
+ " (union",
+ " (bgp (triple ?s ?p ?o))",
+ " (extend (?x true)",
+ " (table unit)))))");
+ //@formatter:on
+ }
+
+ @Test
+ public void ineligible_06() {
+ // Don't inline if we'd move it from within a n-ary operator as doing so
+ // may change the queries semantics
+ //@formatter:off
+ testNoChange("(project (?s)",
+ " (filter (exprlist ?x)",
+ " (leftjoin",
+ " (bgp (triple ?s ?p ?o))",
+ " (extend (?x true)",
+ " (table unit)))))");
+ //@formatter:on
+ }
+
+ @Test
+ public void ineligible_07() {
+ // Don't inline if we'd move it from within a n-ary operator as doing so
+ // may change the queries semantics
+ //@formatter:off
+ testNoChange("(project (?s)",
+ " (filter (exprlist ?x)",
+ " (minus",
+ " (bgp (triple ?s ?p ?o))",
+ " (extend (?x true)",
+ " (table unit)))))");
+ //@formatter:on
+ }
+
+ @Test
+ public void ineligible_08() {
+ // Don't inline if we'd move it from within a n-ary operator as doing so
+ // may change the queries semantics
+ // Technically a trivial case like this is probably safe to inline but
+ // the reality is that the assignment expression may depend on the
+ // behaviour of one branch and so it is safer to not inline
+ //@formatter:off
+ testNoChange("(project (?s)",
+ " (filter (exprlist ?x)",
+ " (join",
+ " (bgp (triple ?s ?p ?o))",
+ " (extend (?x true)",
+ " (table unit)))))");
+ //@formatter:on
+ }
+
@Test
public void no_merge_01() {
// We should not merge extends
[2/7] jena git commit: Add additional test cases for inlining
(JENA-780)
Posted by rv...@apache.org.
Add additional test cases for inlining (JENA-780)
Adds some more test cases which checks behaviour with inlining through
projections which is possible if the assignment is projected and we're
in a stack of unary operators
Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/a1886481
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/a1886481
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/a1886481
Branch: refs/heads/jena2
Commit: a18864815440586daeb7470154d6a2a6efb4827e
Parents: 166b31f
Author: Rob Vesse <rv...@apache.org>
Authored: Tue Jul 7 17:47:08 2015 +0100
Committer: Rob Vesse <rv...@apache.org>
Committed: Tue Jul 7 17:47:08 2015 +0100
----------------------------------------------------------------------
.../optimize/TransformRemoveAssignment.java | 16 ++++++
.../TestTransformEliminateAssignments.java | 52 ++++++++++++++++++++
2 files changed, 68 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/jena/blob/a1886481/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
index 88bd048..855ec10 100644
--- a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
+++ b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
@@ -18,11 +18,14 @@
package com.hp.hpl.jena.sparql.algebra.optimize;
+import java.util.List;
+
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.algebra.op.OpExtendAssign;
+import com.hp.hpl.jena.sparql.algebra.op.OpProject;
import com.hp.hpl.jena.sparql.core.Var;
import com.hp.hpl.jena.sparql.core.VarExprList;
import com.hp.hpl.jena.sparql.expr.Expr;
@@ -113,4 +116,17 @@ public class TransformRemoveAssignment extends TransformCopy {
}
}
+ public Op transform(OpProject opProject, Op subOp) {
+ if (!opProject.getVars().contains(this.var))
+ return super.transform(opProject, subOp);
+
+ List<Var> newVars = opProject.getVars();
+ newVars.remove(this.var);
+ if (newVars.size() > 0) {
+ return new OpProject(subOp, newVars);
+ } else {
+ return subOp;
+ }
+ }
+
}
http://git-wip-us.apache.org/repos/asf/jena/blob/a1886481/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
index 4f3a1a4..ed96785 100644
--- a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
+++ b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
@@ -440,6 +440,58 @@ public class TestTransformEliminateAssignments {
}
@Test
+ public void through_project_01() {
+ // We can inline out through a project by also eliminating the variable
+ // from the project
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (project (?x)",
+ " (extend (?x true)",
+ " (table unit)))))"),
+ "(project (?y)",
+ " (filter true",
+ " (table unit)))");
+ //@formatter:on
+ }
+
+ @Test
+ public void through_project_02() {
+ // We can inline out through a project by also eliminating the variable
+ // from the project
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (project (?x ?y)",
+ " (extend (?x true)",
+ " (bgp (triple ?y <urn:pred> <urn:obj>))))))"),
+ "(project (?y)",
+ " (filter true",
+ " (project (?y)",
+ " (bgp (triple ?y <urn:pred> <urn:obj>)))))");
+ //@formatter:on
+ }
+
+ @Test
+ public void through_project_03() {
+ // We can't inline out through a project if the assignment is not
+ // projected
+ // However we can still eliminate it from the inner projection if that
+ // would render it unused
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (project (?y)",
+ " (extend (?x true)",
+ " (table unit)))))"),
+ "(project (?y)",
+ " (filter (exprlist ?x)",
+ " (project (?y)",
+ " (table unit))))");
+ //@formatter:on
+ }
+
+ @Test
public void no_merge_01() {
// We should not merge extends
//@formatter:off
Re: More on TransformEliminateAssignments
Posted by Rob Vesse <rv...@dotnetrdf.org>.
Looks like I missed one commit off of the master branch
I've been developing this primarily on the Jena2 branch (we're still on
Java 7 and thus Jena 2 for the time being) and have been cherry-picking
across to master and managed to miss one commit off
Missing commit should be pushed shortly
TransformRemoveAssignment exists to support TransformEliminateAssignment
and serves to separate the logic of determining which assignments can be
inlined/eliminated from the logic of actually removing the original
assignment (which may be deep in the algebra tree relative to the point
where we inline it)
I think it is pretty much there now and should only inline/eliminate
things where it is safe to do so, however it may not be perfect and so I
have left this optimisation disabled by default so users can opt in if
they wish at their own risk
Rob
On 12/07/2015 12:13, "Andy Seaborne" <an...@apache.org> wrote:
>Rob,
>
>I've fixed the merge conflict; pelase check it because it did not quite
>agree with the imports so that might indicate I got it wrong. Maybe not
>everything got pushed? (It does at least build now and pass tests.)
>
>A few comments:
>
>1/ TransformEliminateAssignments & TransformRemoveAssignment
>
>Is TransformRemoveAssignment just there to support
>TransformEliminateAssignments?
>
>2/ What's the current status (thinking of jena3 timescales here)
>
>I see "TODO unclear"
>
> Andy
>
>On 11/07/15 17:51, Andy Seaborne wrote:
>> Rob,
>>
>> There is a conflict in TransformEliminateAssignments:
>>
>> I'm not quite sure what the correct resolution is.
>>
>> Andy
>>
>> (Extract):
>>
>> >
>>
>>http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/ma
>>in/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignm
>>ents.java
>>
>> >
>>----------------------------------------------------------------------
>> > diff --git
>>
>>a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Transfor
>>mEliminateAssignments.java
>>
>>b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Transfor
>>mEliminateAssignments.java
>>
>> > index 77ba124..3ce4198 100644
>> > ---
>>
>>a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Transfor
>>mEliminateAssignments.java
>>
>> > +++
>>
>>b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Transfor
>>mEliminateAssignments.java
>>
>>
>> ...
>>
>> > // See if we can inline anything
>> > for (Var var : vars) {
>> > @@ -532,5 +535,75 @@ public class TransformEliminateAssignments
>> extends TransformCopy {
>> > this.tracker.decrementDepth();
>> > }
>> >
>> > +<<<<<<<
>>
>>HEAD:jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Trans
>>formEliminateAssignments.java
>>
>> > +=======
>> > + @Override
>>
>>
>>
>>
>>
>>
>>
>> On 08/07/15 15:12, rvesse@apache.org wrote:
>>> Additional tests and fixes for assignment inlining (JENA-780)
>>>
>>> - Don't inline into EXISTS/NOT EXISTS since those are n-ary operators
>>> - Permit inlining within an EXISTS/NOT EXISTS only if the assignments
>>> are subject to all the normal restrictions
>>> - Don't remove projection when inlining/eliminating through projections
>>> since doing so could cause previously hidden variables to become
>>> visible
>>> - Various additional test cases for these
>>>
>>> Conflicts:
>>>
>>>jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Transform
>>>EliminateAssignments.java
>>>
>>>
>>>
>>> Project: http://git-wip-us.apache.org/repos/asf/jena/repo
>>> Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/a25c72c9
>>> Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/a25c72c9
>>> Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/a25c72c9
>>>
>>> Branch: refs/heads/master
>>> Commit: a25c72c94ecd433fbc4cca78d87f9a7c539841df
>>> Parents: 2031b78
>>> Author: Rob Vesse <rv...@apache.org>
>>> Authored: Wed Jul 8 12:01:28 2015 +0100
>>> Committer: Rob Vesse <rv...@apache.org>
>>> Committed: Wed Jul 8 15:08:47 2015 +0100
>>>
>>> ----------------------------------------------------------------------
>>> .../optimize/TransformEliminateAssignments.java | 83
>>>++++++++++++++++-
>>> .../optimize/TransformRemoveAssignment.java | 7 +-
>>> .../org/apache/jena/sparql/expr/ExprVars.java | 27 ++++++
>>> .../TestTransformEliminateAssignments.java | 97
>>> +++++++++++++++++++-
>>> 4 files changed, 202 insertions(+), 12 deletions(-)
>>> ----------------------------------------------------------------------
>>>
>>>
>>>
>>>http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/m
>>>ain/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssig
>>>nments.java
>>>
>>> ----------------------------------------------------------------------
>>> diff --git
>>>
>>>a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Transfo
>>>rmEliminateAssignments.java
>>>
>>>b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Transfo
>>>rmEliminateAssignments.java
>>>
>>> index 77ba124..3ce4198 100644
>>> ---
>>>
>>>a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Transfo
>>>rmEliminateAssignments.java
>>>
>>> +++
>>>
>>>b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Transfo
>>>rmEliminateAssignments.java
>>>
>>> @@ -21,9 +21,11 @@ package org.apache.jena.sparql.algebra.optimize;
>>> import java.util.ArrayList;
>>> import java.util.Collection;
>>> import java.util.HashMap;
>>> +import java.util.HashSet;
>>> import java.util.Iterator;
>>> import java.util.List;
>>> import java.util.Map;
>>> +import java.util.Set;
>>>
>>> import org.apache.jena.atlas.lib.CollectionUtils;
>>>
>>> @@ -99,8 +101,9 @@ public class TransformEliminateAssignments extends
>>> TransformCopy {
>>> AssignmentPusher pusher = new AssignmentPusher(tracker);
>>> AssignmentPopper popper = new AssignmentPopper(tracker);
>>> Transform transform = new
>>> TransformEliminateAssignments(tracker, pusher, popper, aggressive);
>>> + ExprTransform exprTransform = new
>>> ExprTransformEliminateAssignments(aggressive);
>>>
>>> - return Transformer.transformSkipService(transform, op,
>>> pusher, popper);
>>> + return Transformer.transformSkipService(transform,
>>> exprTransform, op, pusher, popper);
>>> }
>>>
>>> private final OpVisitor before, after;
>>> @@ -161,9 +164,9 @@ public class TransformEliminateAssignments extends
>>> TransformCopy {
>>> return super.transform(opFilter, subOp);
>>>
>>> // See what vars are used in the filter
>>> - Collection<Var> vars = new ArrayList<>();
>>> + Set<Var> vars = new HashSet<>();
>>> for (Expr expr : opFilter.getExprs().getList()) {
>>> - ExprVars.varsMentioned(vars, expr);
>>> + ExprVars.nonOpVarsMentioned(vars, expr);
>>> }
>>>
>>> // Are any of these vars single usage?
>>> @@ -222,8 +225,8 @@ public class TransformEliminateAssignments extends
>>> TransformCopy {
>>> Expr currExpr =
>>> opExtend.getVarExprList().getExpr(assignVar);
>>>
>>> // See what vars are used in the current expression
>>> - Collection<Var> vars = new ArrayList<>();
>>> - ExprVars.varsMentioned(vars, currExpr);
>>> + Set<Var> vars = new HashSet<Var>();
>>> + ExprVars.nonOpVarsMentioned(vars, currExpr);
>>>
>>> // See if we can inline anything
>>> for (Var var : vars) {
>>> @@ -532,5 +535,75 @@ public class TransformEliminateAssignments
>>> extends TransformCopy {
>>> this.tracker.decrementDepth();
>>> }
>>>
>>> +<<<<<<<
>>>
>>>HEAD:jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Tran
>>>sformEliminateAssignments.java
>>>
>>> +=======
>>> + @Override
>>> + public void visit(OpUnion opUnion) {
>>> + unsafe();
>>> + }
>>> +
>>> + @Override
>>> + public void visit(OpLeftJoin opLeftJoin) {
>>> + // TODO Technically if the assignment is single use and
>>> comes from
>>> + // the LHS we could keep it but for now we don't try and
>>> do this
>>> + unsafe();
>>> + }
>>> +
>>> + @Override
>>> + public void visit(OpMinus opMinus) {
>>> + // Anything from the RHS doesn't project out anyway
>>> + unsafe();
>>> + }
>>> +
>>> + @Override
>>> + public void visit(OpJoin opJoin) {
>>> + unsafe();
>>> + }
>>> +
>>> + private void unsafe() {
>>> + // Throw out any assignments because if they would be
>>> eligible their
>>> + // values can't be bound in every branch of the union and
>>> thus
>>> + // inlining could change the semantics
>>> + tracker.getAssignments().clear();
>>> + }
>>> +>>>>>>> 67bb248... Additional tests and fixes for assignment inlining
>>>
>>>(JENA-780):jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimiz
>>>e/TransformEliminateAssignments.java
>>>
>>> + }
>>> +
>>> + /**
>>> + * Handles expression transforms for eliminating assignments
>>> + */
>>> + private static class ExprTransformEliminateAssignments extends
>>> ExprTransformCopy {
>>> +
>>> + private final boolean aggressive;
>>> +
>>> + /**
>>> + * @param aggressive
>>> + * Whether to inline aggressively
>>> + */
>>> + public ExprTransformEliminateAssignments(boolean aggressive) {
>>> + this.aggressive = aggressive;
>>> + }
>>> +
>>> + @Override
>>> + public Expr transform(ExprFunctionOp funcOp, ExprList args,
>>> Op opArg) {
>>> + // Need to use fresh visitors when working inside an
>>> exists/not
>>> + // exists as we should only do self-contained inlining
>>> +
>>> + AssignmentTracker tracker = new AssignmentTracker();
>>> + AssignmentPusher pusher = new AssignmentPusher(tracker);
>>> + AssignmentPopper popper = new AssignmentPopper(tracker);
>>> + Transform transform = new
>>> TransformEliminateAssignments(tracker, pusher, popper, aggressive);
>>> + ExprTransformEliminateAssignments exprTransform = new
>>> ExprTransformEliminateAssignments(aggressive);
>>> +
>>> + Op opArg2 = Transformer.transform(transform,
>>> exprTransform, opArg, pusher, popper);
>>> + if (opArg2 == opArg)
>>> + return super.transform(funcOp, args, opArg);
>>> + if (funcOp instanceof E_Exists)
>>> + return new E_Exists(opArg2);
>>> + if (funcOp instanceof E_NotExists)
>>> + return new E_NotExists(opArg2);
>>> + throw new ARQInternalErrorException("Unrecognized
>>> ExprFunctionOp: \n" + funcOp);
>>> + }
>>> +
>>> }
>>> }
>>>
>>>
>>>http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/m
>>>ain/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignme
>>>nt.java
>>>
>>> ----------------------------------------------------------------------
>>> diff --git
>>>
>>>a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Transfo
>>>rmRemoveAssignment.java
>>>
>>>b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Transfo
>>>rmRemoveAssignment.java
>>>
>>> index 833de93..fd71ffb 100644
>>> ---
>>>
>>>a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Transfo
>>>rmRemoveAssignment.java
>>>
>>> +++
>>>
>>>b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/Transfo
>>>rmRemoveAssignment.java
>>>
>>> @@ -119,11 +119,6 @@ public class TransformRemoveAssignment extends
>>> TransformCopy {
>>>
>>> List<Var> newVars = opProject.getVars();
>>> newVars.remove(this.var);
>>> - if (newVars.size() > 0) {
>>> - return new OpProject(subOp, newVars);
>>> - } else {
>>> - return subOp;
>>> - }
>>> + return new OpProject(subOp, newVars);
>>> }
>>> -
>>> }
>>>
>>>
>>>http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/m
>>>ain/java/org/apache/jena/sparql/expr/ExprVars.java
>>>
>>> ----------------------------------------------------------------------
>>> diff --git
>>> a/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
>>> b/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
>>> index 4ca888a..64541e4 100644
>>> --- a/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
>>> +++ b/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
>>> @@ -59,6 +59,20 @@ public class ExprVars
>>> ExprWalker.walk(vv, expr) ;
>>> }
>>>
>>> + public static void nonOpVarsMentioned(Collection<Var> acc, Expr
>>> expr)
>>> + {
>>> + ExprVars.Action<Var> action =
>>> + new ExprVars.Action<Var>(){
>>> + @Override
>>> + public void var(Collection<Var> acc, Var var)
>>> + {
>>> + acc.add(var) ;
>>> + }
>>> + } ;
>>> + ExprNoOpVarsWorker<Var> vv = new ExprNoOpVarsWorker<>(acc,
>>> action) ;
>>> + ExprWalker.walk(vv, expr) ;
>>> + }
>>> +
>>> public static Set<String> getVarNamesMentioned(Expr expr)
>>> {
>>> Set<String> acc = new HashSet<>() ;
>>> @@ -124,4 +138,17 @@ public class ExprVars
>>> }
>>>
>>> }
>>> +
>>> + static class ExprNoOpVarsWorker<T> extends ExprVarsWorker<T>
>>> + {
>>> + public ExprNoOpVarsWorker(Collection<T> acc, Action<T>
>>>action) {
>>> + super(acc, action);
>>> + // TODO Auto-generated constructor stub
>>> + }
>>> +
>>> + public void visit(ExprFunctionOp funcOp) {
>>> + // Don't include op vars
>>> + return;
>>> + }
>>> + }
>>> }
>>>
>>>
>>>http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/t
>>>est/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateA
>>>ssignments.java
>>>
>>> ----------------------------------------------------------------------
>>> diff --git
>>>
>>>a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTra
>>>nsformEliminateAssignments.java
>>>
>>>b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTra
>>>nsformEliminateAssignments.java
>>>
>>> index 13e9c7e..56029e5 100644
>>> ---
>>>
>>>a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTra
>>>nsformEliminateAssignments.java
>>>
>>> +++
>>>
>>>b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTra
>>>nsformEliminateAssignments.java
>>>
>>> @@ -382,6 +382,100 @@ public class TestTransformEliminateAssignments {
>>> }
>>>
>>> @Test
>>> + public void ineligible_09() {
>>> + // We can't inline if the assignment will be projected out
>>> + //@formatter:off
>>> + testNoChange("(project (?x ?y)",
>>> + " (filter (exprlist ?x)",
>>> + " (extend (?x true)",
>>> + " (bgp (triple ?y <urn:pred>
>>> <urn:obj>)))))");;
>>> + //@formatter:on
>>> + }
>>> +
>>> + @Test
>>> + public void ineligible_10() {
>>> + // We can't inline out of an EXISTS since the assignment
>>> isn't projected
>>> + // out anyway
>>> + //@formatter:off
>>> + testNoChange("(project (?y)",
>>> + " (filter (exprlist ?x (exists",
>>> + " (extend (?x true)",
>>> + " (table unit))))",
>>> + " (table unit)))");
>>> + //@formatter:on
>>> + }
>>> +
>>> + @Test
>>> + public void ineligible_11() {
>>> + // We can't inline out of an EXISTS since the assignment
>>> isn't projected
>>> + // out anyway
>>> + //@formatter:off
>>> + testNoChange("(project (?y)",
>>> + " (filter (exprlist ?x)",
>>> + " (filter (exprlist (exists",
>>> + " (extend (?x true)",
>>> + " (table unit))))",
>>> + " (table unit))))");
>>> + //@formatter:on
>>> + }
>>> +
>>> + @Test
>>> + public void exists_01() {
>>> + // We can't inline into an EXISTS since the assignment isn't
>>> projected
>>> + // out anyway and its an n-ary operator so would change
>>> semantics
>>> + // However this makes the assignment unused so can still
>>> remove it
>>> + //@formatter:off
>>> + test(StrUtils.strjoinNL("(project (?y)",
>>> + " (filter (exprlist (exists",
>>> + " (filter
>>> (exprlist ?x)",
>>> + " (table
>>> unit))))",
>>> + " (extend (?x true)",
>>> + " (table unit))))"),
>>> + "(project (?y)",
>>> + " (filter (exprlist (exists",
>>> + " (filter (exprlist ?x)",
>>> + " (table unit))))",
>>> + " (table unit)))");
>>> + //@formatter:on
>>> + }
>>> +
>>> + @Test
>>> + public void exists_02() {
>>> + // Could inline within an exists but still needs to meet
>>> other rules
>>> + // Even though an exists is technically a form of projection
>>> can't
>>> + // discount the variable being needed elsewhere
>>> + //@formatter:off
>>> + testNoChange("(project (?y)",
>>> + " (filter (exprlist (exists",
>>> + " (filter (exprlist ?x)",
>>> + " (extend (?x true)",
>>> + " (table unit)))))",
>>> + " (table unit)))");
>>> + //@formatter:on
>>> + }
>>> +
>>> + @Test
>>> + public void exists_03() {
>>> + // Can inline within an exists provided it meets normal
>>> conditions of
>>> + // being inside a projection
>>> + //@formatter:off
>>> + test(StrUtils.strjoinNL("(project (?y)",
>>> + " (filter (exprlist (exists",
>>> + " (project (?y)",
>>> + " (filter
>>> (exprlist ?x)",
>>> + " (extend (?x
>>> true)",
>>> + " (table
>>> unit))))))",
>>> + " (table unit)))"),
>>> + "(project (?y)",
>>> + " (filter (exprlist (exists",
>>> + " (project (?y)",
>>> + " (filter (exprlist true)",
>>> + " (table unit)))))",
>>> + " (table unit)))");
>>> + //@formatter:on
>>> + }
>>> +
>>> + @Test
>>> public void through_project_01() {
>>> // We can inline out through a project by also eliminating
>>> the variable
>>> // from the project
>>> @@ -393,7 +487,8 @@ public class TestTransformEliminateAssignments {
>>> " (table unit)))))"),
>>> "(project (?y)",
>>> " (filter true",
>>> - " (table unit)))");
>>> + " (project ()",
>>> + " (table unit))))");
>>> //@formatter:on
>>> }
>>>
>>>
>>
>
More on TransformEliminateAssignments
Posted by Andy Seaborne <an...@apache.org>.
Rob,
I've fixed the merge conflict; pelase check it because it did not quite
agree with the imports so that might indicate I got it wrong. Maybe not
everything got pushed? (It does at least build now and pass tests.)
A few comments:
1/ TransformEliminateAssignments & TransformRemoveAssignment
Is TransformRemoveAssignment just there to support
TransformEliminateAssignments?
2/ What's the current status (thinking of jena3 timescales here)
I see "TODO unclear"
Andy
On 11/07/15 17:51, Andy Seaborne wrote:
> Rob,
>
> There is a conflict in TransformEliminateAssignments:
>
> I'm not quite sure what the correct resolution is.
>
> Andy
>
> (Extract):
>
> >
> http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>
> > ----------------------------------------------------------------------
> > diff --git
> a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
> b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>
> > index 77ba124..3ce4198 100644
> > ---
> a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>
> > +++
> b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>
>
> ...
>
> > // See if we can inline anything
> > for (Var var : vars) {
> > @@ -532,5 +535,75 @@ public class TransformEliminateAssignments
> extends TransformCopy {
> > this.tracker.decrementDepth();
> > }
> >
> > +<<<<<<<
> HEAD:jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>
> > +=======
> > + @Override
>
>
>
>
>
>
>
> On 08/07/15 15:12, rvesse@apache.org wrote:
>> Additional tests and fixes for assignment inlining (JENA-780)
>>
>> - Don't inline into EXISTS/NOT EXISTS since those are n-ary operators
>> - Permit inlining within an EXISTS/NOT EXISTS only if the assignments
>> are subject to all the normal restrictions
>> - Don't remove projection when inlining/eliminating through projections
>> since doing so could cause previously hidden variables to become
>> visible
>> - Various additional test cases for these
>>
>> Conflicts:
>> jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>>
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/jena/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/a25c72c9
>> Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/a25c72c9
>> Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/a25c72c9
>>
>> Branch: refs/heads/master
>> Commit: a25c72c94ecd433fbc4cca78d87f9a7c539841df
>> Parents: 2031b78
>> Author: Rob Vesse <rv...@apache.org>
>> Authored: Wed Jul 8 12:01:28 2015 +0100
>> Committer: Rob Vesse <rv...@apache.org>
>> Committed: Wed Jul 8 15:08:47 2015 +0100
>>
>> ----------------------------------------------------------------------
>> .../optimize/TransformEliminateAssignments.java | 83 ++++++++++++++++-
>> .../optimize/TransformRemoveAssignment.java | 7 +-
>> .../org/apache/jena/sparql/expr/ExprVars.java | 27 ++++++
>> .../TestTransformEliminateAssignments.java | 97
>> +++++++++++++++++++-
>> 4 files changed, 202 insertions(+), 12 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>>
>> ----------------------------------------------------------------------
>> diff --git
>> a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>> b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>>
>> index 77ba124..3ce4198 100644
>> ---
>> a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>>
>> +++
>> b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>>
>> @@ -21,9 +21,11 @@ package org.apache.jena.sparql.algebra.optimize;
>> import java.util.ArrayList;
>> import java.util.Collection;
>> import java.util.HashMap;
>> +import java.util.HashSet;
>> import java.util.Iterator;
>> import java.util.List;
>> import java.util.Map;
>> +import java.util.Set;
>>
>> import org.apache.jena.atlas.lib.CollectionUtils;
>>
>> @@ -99,8 +101,9 @@ public class TransformEliminateAssignments extends
>> TransformCopy {
>> AssignmentPusher pusher = new AssignmentPusher(tracker);
>> AssignmentPopper popper = new AssignmentPopper(tracker);
>> Transform transform = new
>> TransformEliminateAssignments(tracker, pusher, popper, aggressive);
>> + ExprTransform exprTransform = new
>> ExprTransformEliminateAssignments(aggressive);
>>
>> - return Transformer.transformSkipService(transform, op,
>> pusher, popper);
>> + return Transformer.transformSkipService(transform,
>> exprTransform, op, pusher, popper);
>> }
>>
>> private final OpVisitor before, after;
>> @@ -161,9 +164,9 @@ public class TransformEliminateAssignments extends
>> TransformCopy {
>> return super.transform(opFilter, subOp);
>>
>> // See what vars are used in the filter
>> - Collection<Var> vars = new ArrayList<>();
>> + Set<Var> vars = new HashSet<>();
>> for (Expr expr : opFilter.getExprs().getList()) {
>> - ExprVars.varsMentioned(vars, expr);
>> + ExprVars.nonOpVarsMentioned(vars, expr);
>> }
>>
>> // Are any of these vars single usage?
>> @@ -222,8 +225,8 @@ public class TransformEliminateAssignments extends
>> TransformCopy {
>> Expr currExpr =
>> opExtend.getVarExprList().getExpr(assignVar);
>>
>> // See what vars are used in the current expression
>> - Collection<Var> vars = new ArrayList<>();
>> - ExprVars.varsMentioned(vars, currExpr);
>> + Set<Var> vars = new HashSet<Var>();
>> + ExprVars.nonOpVarsMentioned(vars, currExpr);
>>
>> // See if we can inline anything
>> for (Var var : vars) {
>> @@ -532,5 +535,75 @@ public class TransformEliminateAssignments
>> extends TransformCopy {
>> this.tracker.decrementDepth();
>> }
>>
>> +<<<<<<<
>> HEAD:jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>>
>> +=======
>> + @Override
>> + public void visit(OpUnion opUnion) {
>> + unsafe();
>> + }
>> +
>> + @Override
>> + public void visit(OpLeftJoin opLeftJoin) {
>> + // TODO Technically if the assignment is single use and
>> comes from
>> + // the LHS we could keep it but for now we don't try and
>> do this
>> + unsafe();
>> + }
>> +
>> + @Override
>> + public void visit(OpMinus opMinus) {
>> + // Anything from the RHS doesn't project out anyway
>> + unsafe();
>> + }
>> +
>> + @Override
>> + public void visit(OpJoin opJoin) {
>> + unsafe();
>> + }
>> +
>> + private void unsafe() {
>> + // Throw out any assignments because if they would be
>> eligible their
>> + // values can't be bound in every branch of the union and
>> thus
>> + // inlining could change the semantics
>> + tracker.getAssignments().clear();
>> + }
>> +>>>>>>> 67bb248... Additional tests and fixes for assignment inlining
>> (JENA-780):jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>>
>> + }
>> +
>> + /**
>> + * Handles expression transforms for eliminating assignments
>> + */
>> + private static class ExprTransformEliminateAssignments extends
>> ExprTransformCopy {
>> +
>> + private final boolean aggressive;
>> +
>> + /**
>> + * @param aggressive
>> + * Whether to inline aggressively
>> + */
>> + public ExprTransformEliminateAssignments(boolean aggressive) {
>> + this.aggressive = aggressive;
>> + }
>> +
>> + @Override
>> + public Expr transform(ExprFunctionOp funcOp, ExprList args,
>> Op opArg) {
>> + // Need to use fresh visitors when working inside an
>> exists/not
>> + // exists as we should only do self-contained inlining
>> +
>> + AssignmentTracker tracker = new AssignmentTracker();
>> + AssignmentPusher pusher = new AssignmentPusher(tracker);
>> + AssignmentPopper popper = new AssignmentPopper(tracker);
>> + Transform transform = new
>> TransformEliminateAssignments(tracker, pusher, popper, aggressive);
>> + ExprTransformEliminateAssignments exprTransform = new
>> ExprTransformEliminateAssignments(aggressive);
>> +
>> + Op opArg2 = Transformer.transform(transform,
>> exprTransform, opArg, pusher, popper);
>> + if (opArg2 == opArg)
>> + return super.transform(funcOp, args, opArg);
>> + if (funcOp instanceof E_Exists)
>> + return new E_Exists(opArg2);
>> + if (funcOp instanceof E_NotExists)
>> + return new E_NotExists(opArg2);
>> + throw new ARQInternalErrorException("Unrecognized
>> ExprFunctionOp: \n" + funcOp);
>> + }
>> +
>> }
>> }
>>
>> http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
>>
>> ----------------------------------------------------------------------
>> diff --git
>> a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
>> b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
>>
>> index 833de93..fd71ffb 100644
>> ---
>> a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
>>
>> +++
>> b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
>>
>> @@ -119,11 +119,6 @@ public class TransformRemoveAssignment extends
>> TransformCopy {
>>
>> List<Var> newVars = opProject.getVars();
>> newVars.remove(this.var);
>> - if (newVars.size() > 0) {
>> - return new OpProject(subOp, newVars);
>> - } else {
>> - return subOp;
>> - }
>> + return new OpProject(subOp, newVars);
>> }
>> -
>> }
>>
>> http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
>>
>> ----------------------------------------------------------------------
>> diff --git
>> a/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
>> b/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
>> index 4ca888a..64541e4 100644
>> --- a/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
>> +++ b/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
>> @@ -59,6 +59,20 @@ public class ExprVars
>> ExprWalker.walk(vv, expr) ;
>> }
>>
>> + public static void nonOpVarsMentioned(Collection<Var> acc, Expr
>> expr)
>> + {
>> + ExprVars.Action<Var> action =
>> + new ExprVars.Action<Var>(){
>> + @Override
>> + public void var(Collection<Var> acc, Var var)
>> + {
>> + acc.add(var) ;
>> + }
>> + } ;
>> + ExprNoOpVarsWorker<Var> vv = new ExprNoOpVarsWorker<>(acc,
>> action) ;
>> + ExprWalker.walk(vv, expr) ;
>> + }
>> +
>> public static Set<String> getVarNamesMentioned(Expr expr)
>> {
>> Set<String> acc = new HashSet<>() ;
>> @@ -124,4 +138,17 @@ public class ExprVars
>> }
>>
>> }
>> +
>> + static class ExprNoOpVarsWorker<T> extends ExprVarsWorker<T>
>> + {
>> + public ExprNoOpVarsWorker(Collection<T> acc, Action<T> action) {
>> + super(acc, action);
>> + // TODO Auto-generated constructor stub
>> + }
>> +
>> + public void visit(ExprFunctionOp funcOp) {
>> + // Don't include op vars
>> + return;
>> + }
>> + }
>> }
>>
>> http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
>>
>> ----------------------------------------------------------------------
>> diff --git
>> a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
>> b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
>>
>> index 13e9c7e..56029e5 100644
>> ---
>> a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
>>
>> +++
>> b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
>>
>> @@ -382,6 +382,100 @@ public class TestTransformEliminateAssignments {
>> }
>>
>> @Test
>> + public void ineligible_09() {
>> + // We can't inline if the assignment will be projected out
>> + //@formatter:off
>> + testNoChange("(project (?x ?y)",
>> + " (filter (exprlist ?x)",
>> + " (extend (?x true)",
>> + " (bgp (triple ?y <urn:pred>
>> <urn:obj>)))))");;
>> + //@formatter:on
>> + }
>> +
>> + @Test
>> + public void ineligible_10() {
>> + // We can't inline out of an EXISTS since the assignment
>> isn't projected
>> + // out anyway
>> + //@formatter:off
>> + testNoChange("(project (?y)",
>> + " (filter (exprlist ?x (exists",
>> + " (extend (?x true)",
>> + " (table unit))))",
>> + " (table unit)))");
>> + //@formatter:on
>> + }
>> +
>> + @Test
>> + public void ineligible_11() {
>> + // We can't inline out of an EXISTS since the assignment
>> isn't projected
>> + // out anyway
>> + //@formatter:off
>> + testNoChange("(project (?y)",
>> + " (filter (exprlist ?x)",
>> + " (filter (exprlist (exists",
>> + " (extend (?x true)",
>> + " (table unit))))",
>> + " (table unit))))");
>> + //@formatter:on
>> + }
>> +
>> + @Test
>> + public void exists_01() {
>> + // We can't inline into an EXISTS since the assignment isn't
>> projected
>> + // out anyway and its an n-ary operator so would change
>> semantics
>> + // However this makes the assignment unused so can still
>> remove it
>> + //@formatter:off
>> + test(StrUtils.strjoinNL("(project (?y)",
>> + " (filter (exprlist (exists",
>> + " (filter
>> (exprlist ?x)",
>> + " (table
>> unit))))",
>> + " (extend (?x true)",
>> + " (table unit))))"),
>> + "(project (?y)",
>> + " (filter (exprlist (exists",
>> + " (filter (exprlist ?x)",
>> + " (table unit))))",
>> + " (table unit)))");
>> + //@formatter:on
>> + }
>> +
>> + @Test
>> + public void exists_02() {
>> + // Could inline within an exists but still needs to meet
>> other rules
>> + // Even though an exists is technically a form of projection
>> can't
>> + // discount the variable being needed elsewhere
>> + //@formatter:off
>> + testNoChange("(project (?y)",
>> + " (filter (exprlist (exists",
>> + " (filter (exprlist ?x)",
>> + " (extend (?x true)",
>> + " (table unit)))))",
>> + " (table unit)))");
>> + //@formatter:on
>> + }
>> +
>> + @Test
>> + public void exists_03() {
>> + // Can inline within an exists provided it meets normal
>> conditions of
>> + // being inside a projection
>> + //@formatter:off
>> + test(StrUtils.strjoinNL("(project (?y)",
>> + " (filter (exprlist (exists",
>> + " (project (?y)",
>> + " (filter
>> (exprlist ?x)",
>> + " (extend (?x
>> true)",
>> + " (table
>> unit))))))",
>> + " (table unit)))"),
>> + "(project (?y)",
>> + " (filter (exprlist (exists",
>> + " (project (?y)",
>> + " (filter (exprlist true)",
>> + " (table unit)))))",
>> + " (table unit)))");
>> + //@formatter:on
>> + }
>> +
>> + @Test
>> public void through_project_01() {
>> // We can inline out through a project by also eliminating
>> the variable
>> // from the project
>> @@ -393,7 +487,8 @@ public class TestTransformEliminateAssignments {
>> " (table unit)))))"),
>> "(project (?y)",
>> " (filter true",
>> - " (table unit)))");
>> + " (project ()",
>> + " (table unit))))");
>> //@formatter:on
>> }
>>
>>
>
Re: [6/7] jena git commit: Additional tests and fixes for assignment
inlining (JENA-780)
Posted by Andy Seaborne <an...@apache.org>.
Rob,
There is a conflict in TransformEliminateAssignments:
I'm not quite sure what the correct resolution is.
Andy
(Extract):
>
http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
> ----------------------------------------------------------------------
> diff --git
a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
> index 77ba124..3ce4198 100644
> ---
a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
> +++
b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
...
> // See if we can inline anything
> for (Var var : vars) {
> @@ -532,5 +535,75 @@ public class TransformEliminateAssignments
extends TransformCopy {
> this.tracker.decrementDepth();
> }
>
> +<<<<<<<
HEAD:jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
> +=======
> + @Override
On 08/07/15 15:12, rvesse@apache.org wrote:
> Additional tests and fixes for assignment inlining (JENA-780)
>
> - Don't inline into EXISTS/NOT EXISTS since those are n-ary operators
> - Permit inlining within an EXISTS/NOT EXISTS only if the assignments
> are subject to all the normal restrictions
> - Don't remove projection when inlining/eliminating through projections
> since doing so could cause previously hidden variables to become
> visible
> - Various additional test cases for these
>
> Conflicts:
> jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
>
>
> Project: http://git-wip-us.apache.org/repos/asf/jena/repo
> Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/a25c72c9
> Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/a25c72c9
> Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/a25c72c9
>
> Branch: refs/heads/master
> Commit: a25c72c94ecd433fbc4cca78d87f9a7c539841df
> Parents: 2031b78
> Author: Rob Vesse <rv...@apache.org>
> Authored: Wed Jul 8 12:01:28 2015 +0100
> Committer: Rob Vesse <rv...@apache.org>
> Committed: Wed Jul 8 15:08:47 2015 +0100
>
> ----------------------------------------------------------------------
> .../optimize/TransformEliminateAssignments.java | 83 ++++++++++++++++-
> .../optimize/TransformRemoveAssignment.java | 7 +-
> .../org/apache/jena/sparql/expr/ExprVars.java | 27 ++++++
> .../TestTransformEliminateAssignments.java | 97 +++++++++++++++++++-
> 4 files changed, 202 insertions(+), 12 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
> ----------------------------------------------------------------------
> diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
> index 77ba124..3ce4198 100644
> --- a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
> +++ b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
> @@ -21,9 +21,11 @@ package org.apache.jena.sparql.algebra.optimize;
> import java.util.ArrayList;
> import java.util.Collection;
> import java.util.HashMap;
> +import java.util.HashSet;
> import java.util.Iterator;
> import java.util.List;
> import java.util.Map;
> +import java.util.Set;
>
> import org.apache.jena.atlas.lib.CollectionUtils;
>
> @@ -99,8 +101,9 @@ public class TransformEliminateAssignments extends TransformCopy {
> AssignmentPusher pusher = new AssignmentPusher(tracker);
> AssignmentPopper popper = new AssignmentPopper(tracker);
> Transform transform = new TransformEliminateAssignments(tracker, pusher, popper, aggressive);
> + ExprTransform exprTransform = new ExprTransformEliminateAssignments(aggressive);
>
> - return Transformer.transformSkipService(transform, op, pusher, popper);
> + return Transformer.transformSkipService(transform, exprTransform, op, pusher, popper);
> }
>
> private final OpVisitor before, after;
> @@ -161,9 +164,9 @@ public class TransformEliminateAssignments extends TransformCopy {
> return super.transform(opFilter, subOp);
>
> // See what vars are used in the filter
> - Collection<Var> vars = new ArrayList<>();
> + Set<Var> vars = new HashSet<>();
> for (Expr expr : opFilter.getExprs().getList()) {
> - ExprVars.varsMentioned(vars, expr);
> + ExprVars.nonOpVarsMentioned(vars, expr);
> }
>
> // Are any of these vars single usage?
> @@ -222,8 +225,8 @@ public class TransformEliminateAssignments extends TransformCopy {
> Expr currExpr = opExtend.getVarExprList().getExpr(assignVar);
>
> // See what vars are used in the current expression
> - Collection<Var> vars = new ArrayList<>();
> - ExprVars.varsMentioned(vars, currExpr);
> + Set<Var> vars = new HashSet<Var>();
> + ExprVars.nonOpVarsMentioned(vars, currExpr);
>
> // See if we can inline anything
> for (Var var : vars) {
> @@ -532,5 +535,75 @@ public class TransformEliminateAssignments extends TransformCopy {
> this.tracker.decrementDepth();
> }
>
> +<<<<<<< HEAD:jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
> +=======
> + @Override
> + public void visit(OpUnion opUnion) {
> + unsafe();
> + }
> +
> + @Override
> + public void visit(OpLeftJoin opLeftJoin) {
> + // TODO Technically if the assignment is single use and comes from
> + // the LHS we could keep it but for now we don't try and do this
> + unsafe();
> + }
> +
> + @Override
> + public void visit(OpMinus opMinus) {
> + // Anything from the RHS doesn't project out anyway
> + unsafe();
> + }
> +
> + @Override
> + public void visit(OpJoin opJoin) {
> + unsafe();
> + }
> +
> + private void unsafe() {
> + // Throw out any assignments because if they would be eligible their
> + // values can't be bound in every branch of the union and thus
> + // inlining could change the semantics
> + tracker.getAssignments().clear();
> + }
> +>>>>>>> 67bb248... Additional tests and fixes for assignment inlining (JENA-780):jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
> + }
> +
> + /**
> + * Handles expression transforms for eliminating assignments
> + */
> + private static class ExprTransformEliminateAssignments extends ExprTransformCopy {
> +
> + private final boolean aggressive;
> +
> + /**
> + * @param aggressive
> + * Whether to inline aggressively
> + */
> + public ExprTransformEliminateAssignments(boolean aggressive) {
> + this.aggressive = aggressive;
> + }
> +
> + @Override
> + public Expr transform(ExprFunctionOp funcOp, ExprList args, Op opArg) {
> + // Need to use fresh visitors when working inside an exists/not
> + // exists as we should only do self-contained inlining
> +
> + AssignmentTracker tracker = new AssignmentTracker();
> + AssignmentPusher pusher = new AssignmentPusher(tracker);
> + AssignmentPopper popper = new AssignmentPopper(tracker);
> + Transform transform = new TransformEliminateAssignments(tracker, pusher, popper, aggressive);
> + ExprTransformEliminateAssignments exprTransform = new ExprTransformEliminateAssignments(aggressive);
> +
> + Op opArg2 = Transformer.transform(transform, exprTransform, opArg, pusher, popper);
> + if (opArg2 == opArg)
> + return super.transform(funcOp, args, opArg);
> + if (funcOp instanceof E_Exists)
> + return new E_Exists(opArg2);
> + if (funcOp instanceof E_NotExists)
> + return new E_NotExists(opArg2);
> + throw new ARQInternalErrorException("Unrecognized ExprFunctionOp: \n" + funcOp);
> + }
> +
> }
> }
>
> http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
> ----------------------------------------------------------------------
> diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
> index 833de93..fd71ffb 100644
> --- a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
> +++ b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
> @@ -119,11 +119,6 @@ public class TransformRemoveAssignment extends TransformCopy {
>
> List<Var> newVars = opProject.getVars();
> newVars.remove(this.var);
> - if (newVars.size() > 0) {
> - return new OpProject(subOp, newVars);
> - } else {
> - return subOp;
> - }
> + return new OpProject(subOp, newVars);
> }
> -
> }
>
> http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
> ----------------------------------------------------------------------
> diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java b/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
> index 4ca888a..64541e4 100644
> --- a/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
> +++ b/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
> @@ -59,6 +59,20 @@ public class ExprVars
> ExprWalker.walk(vv, expr) ;
> }
>
> + public static void nonOpVarsMentioned(Collection<Var> acc, Expr expr)
> + {
> + ExprVars.Action<Var> action =
> + new ExprVars.Action<Var>(){
> + @Override
> + public void var(Collection<Var> acc, Var var)
> + {
> + acc.add(var) ;
> + }
> + } ;
> + ExprNoOpVarsWorker<Var> vv = new ExprNoOpVarsWorker<>(acc, action) ;
> + ExprWalker.walk(vv, expr) ;
> + }
> +
> public static Set<String> getVarNamesMentioned(Expr expr)
> {
> Set<String> acc = new HashSet<>() ;
> @@ -124,4 +138,17 @@ public class ExprVars
> }
>
> }
> +
> + static class ExprNoOpVarsWorker<T> extends ExprVarsWorker<T>
> + {
> + public ExprNoOpVarsWorker(Collection<T> acc, Action<T> action) {
> + super(acc, action);
> + // TODO Auto-generated constructor stub
> + }
> +
> + public void visit(ExprFunctionOp funcOp) {
> + // Don't include op vars
> + return;
> + }
> + }
> }
>
> http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
> ----------------------------------------------------------------------
> diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
> index 13e9c7e..56029e5 100644
> --- a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
> +++ b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
> @@ -382,6 +382,100 @@ public class TestTransformEliminateAssignments {
> }
>
> @Test
> + public void ineligible_09() {
> + // We can't inline if the assignment will be projected out
> + //@formatter:off
> + testNoChange("(project (?x ?y)",
> + " (filter (exprlist ?x)",
> + " (extend (?x true)",
> + " (bgp (triple ?y <urn:pred> <urn:obj>)))))");;
> + //@formatter:on
> + }
> +
> + @Test
> + public void ineligible_10() {
> + // We can't inline out of an EXISTS since the assignment isn't projected
> + // out anyway
> + //@formatter:off
> + testNoChange("(project (?y)",
> + " (filter (exprlist ?x (exists",
> + " (extend (?x true)",
> + " (table unit))))",
> + " (table unit)))");
> + //@formatter:on
> + }
> +
> + @Test
> + public void ineligible_11() {
> + // We can't inline out of an EXISTS since the assignment isn't projected
> + // out anyway
> + //@formatter:off
> + testNoChange("(project (?y)",
> + " (filter (exprlist ?x)",
> + " (filter (exprlist (exists",
> + " (extend (?x true)",
> + " (table unit))))",
> + " (table unit))))");
> + //@formatter:on
> + }
> +
> + @Test
> + public void exists_01() {
> + // We can't inline into an EXISTS since the assignment isn't projected
> + // out anyway and its an n-ary operator so would change semantics
> + // However this makes the assignment unused so can still remove it
> + //@formatter:off
> + test(StrUtils.strjoinNL("(project (?y)",
> + " (filter (exprlist (exists",
> + " (filter (exprlist ?x)",
> + " (table unit))))",
> + " (extend (?x true)",
> + " (table unit))))"),
> + "(project (?y)",
> + " (filter (exprlist (exists",
> + " (filter (exprlist ?x)",
> + " (table unit))))",
> + " (table unit)))");
> + //@formatter:on
> + }
> +
> + @Test
> + public void exists_02() {
> + // Could inline within an exists but still needs to meet other rules
> + // Even though an exists is technically a form of projection can't
> + // discount the variable being needed elsewhere
> + //@formatter:off
> + testNoChange("(project (?y)",
> + " (filter (exprlist (exists",
> + " (filter (exprlist ?x)",
> + " (extend (?x true)",
> + " (table unit)))))",
> + " (table unit)))");
> + //@formatter:on
> + }
> +
> + @Test
> + public void exists_03() {
> + // Can inline within an exists provided it meets normal conditions of
> + // being inside a projection
> + //@formatter:off
> + test(StrUtils.strjoinNL("(project (?y)",
> + " (filter (exprlist (exists",
> + " (project (?y)",
> + " (filter (exprlist ?x)",
> + " (extend (?x true)",
> + " (table unit))))))",
> + " (table unit)))"),
> + "(project (?y)",
> + " (filter (exprlist (exists",
> + " (project (?y)",
> + " (filter (exprlist true)",
> + " (table unit)))))",
> + " (table unit)))");
> + //@formatter:on
> + }
> +
> + @Test
> public void through_project_01() {
> // We can inline out through a project by also eliminating the variable
> // from the project
> @@ -393,7 +487,8 @@ public class TestTransformEliminateAssignments {
> " (table unit)))))"),
> "(project (?y)",
> " (filter true",
> - " (table unit)))");
> + " (project ()",
> + " (table unit))))");
> //@formatter:on
> }
>
>
[6/7] jena git commit: Additional tests and fixes for assignment
inlining (JENA-780)
Posted by rv...@apache.org.
Additional tests and fixes for assignment inlining (JENA-780)
- Don't inline into EXISTS/NOT EXISTS since those are n-ary operators
- Permit inlining within an EXISTS/NOT EXISTS only if the assignments
are subject to all the normal restrictions
- Don't remove projection when inlining/eliminating through projections
since doing so could cause previously hidden variables to become
visible
- Various additional test cases for these
Conflicts:
jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/a25c72c9
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/a25c72c9
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/a25c72c9
Branch: refs/heads/master
Commit: a25c72c94ecd433fbc4cca78d87f9a7c539841df
Parents: 2031b78
Author: Rob Vesse <rv...@apache.org>
Authored: Wed Jul 8 12:01:28 2015 +0100
Committer: Rob Vesse <rv...@apache.org>
Committed: Wed Jul 8 15:08:47 2015 +0100
----------------------------------------------------------------------
.../optimize/TransformEliminateAssignments.java | 83 ++++++++++++++++-
.../optimize/TransformRemoveAssignment.java | 7 +-
.../org/apache/jena/sparql/expr/ExprVars.java | 27 ++++++
.../TestTransformEliminateAssignments.java | 97 +++++++++++++++++++-
4 files changed, 202 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
index 77ba124..3ce4198 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
@@ -21,9 +21,11 @@ package org.apache.jena.sparql.algebra.optimize;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import org.apache.jena.atlas.lib.CollectionUtils;
@@ -99,8 +101,9 @@ public class TransformEliminateAssignments extends TransformCopy {
AssignmentPusher pusher = new AssignmentPusher(tracker);
AssignmentPopper popper = new AssignmentPopper(tracker);
Transform transform = new TransformEliminateAssignments(tracker, pusher, popper, aggressive);
+ ExprTransform exprTransform = new ExprTransformEliminateAssignments(aggressive);
- return Transformer.transformSkipService(transform, op, pusher, popper);
+ return Transformer.transformSkipService(transform, exprTransform, op, pusher, popper);
}
private final OpVisitor before, after;
@@ -161,9 +164,9 @@ public class TransformEliminateAssignments extends TransformCopy {
return super.transform(opFilter, subOp);
// See what vars are used in the filter
- Collection<Var> vars = new ArrayList<>();
+ Set<Var> vars = new HashSet<>();
for (Expr expr : opFilter.getExprs().getList()) {
- ExprVars.varsMentioned(vars, expr);
+ ExprVars.nonOpVarsMentioned(vars, expr);
}
// Are any of these vars single usage?
@@ -222,8 +225,8 @@ public class TransformEliminateAssignments extends TransformCopy {
Expr currExpr = opExtend.getVarExprList().getExpr(assignVar);
// See what vars are used in the current expression
- Collection<Var> vars = new ArrayList<>();
- ExprVars.varsMentioned(vars, currExpr);
+ Set<Var> vars = new HashSet<Var>();
+ ExprVars.nonOpVarsMentioned(vars, currExpr);
// See if we can inline anything
for (Var var : vars) {
@@ -532,5 +535,75 @@ public class TransformEliminateAssignments extends TransformCopy {
this.tracker.decrementDepth();
}
+<<<<<<< HEAD:jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
+=======
+ @Override
+ public void visit(OpUnion opUnion) {
+ unsafe();
+ }
+
+ @Override
+ public void visit(OpLeftJoin opLeftJoin) {
+ // TODO Technically if the assignment is single use and comes from
+ // the LHS we could keep it but for now we don't try and do this
+ unsafe();
+ }
+
+ @Override
+ public void visit(OpMinus opMinus) {
+ // Anything from the RHS doesn't project out anyway
+ unsafe();
+ }
+
+ @Override
+ public void visit(OpJoin opJoin) {
+ unsafe();
+ }
+
+ private void unsafe() {
+ // Throw out any assignments because if they would be eligible their
+ // values can't be bound in every branch of the union and thus
+ // inlining could change the semantics
+ tracker.getAssignments().clear();
+ }
+>>>>>>> 67bb248... Additional tests and fixes for assignment inlining (JENA-780):jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
+ }
+
+ /**
+ * Handles expression transforms for eliminating assignments
+ */
+ private static class ExprTransformEliminateAssignments extends ExprTransformCopy {
+
+ private final boolean aggressive;
+
+ /**
+ * @param aggressive
+ * Whether to inline aggressively
+ */
+ public ExprTransformEliminateAssignments(boolean aggressive) {
+ this.aggressive = aggressive;
+ }
+
+ @Override
+ public Expr transform(ExprFunctionOp funcOp, ExprList args, Op opArg) {
+ // Need to use fresh visitors when working inside an exists/not
+ // exists as we should only do self-contained inlining
+
+ AssignmentTracker tracker = new AssignmentTracker();
+ AssignmentPusher pusher = new AssignmentPusher(tracker);
+ AssignmentPopper popper = new AssignmentPopper(tracker);
+ Transform transform = new TransformEliminateAssignments(tracker, pusher, popper, aggressive);
+ ExprTransformEliminateAssignments exprTransform = new ExprTransformEliminateAssignments(aggressive);
+
+ Op opArg2 = Transformer.transform(transform, exprTransform, opArg, pusher, popper);
+ if (opArg2 == opArg)
+ return super.transform(funcOp, args, opArg);
+ if (funcOp instanceof E_Exists)
+ return new E_Exists(opArg2);
+ if (funcOp instanceof E_NotExists)
+ return new E_NotExists(opArg2);
+ throw new ARQInternalErrorException("Unrecognized ExprFunctionOp: \n" + funcOp);
+ }
+
}
}
http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
index 833de93..fd71ffb 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
@@ -119,11 +119,6 @@ public class TransformRemoveAssignment extends TransformCopy {
List<Var> newVars = opProject.getVars();
newVars.remove(this.var);
- if (newVars.size() > 0) {
- return new OpProject(subOp, newVars);
- } else {
- return subOp;
- }
+ return new OpProject(subOp, newVars);
}
-
}
http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java b/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
index 4ca888a..64541e4 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprVars.java
@@ -59,6 +59,20 @@ public class ExprVars
ExprWalker.walk(vv, expr) ;
}
+ public static void nonOpVarsMentioned(Collection<Var> acc, Expr expr)
+ {
+ ExprVars.Action<Var> action =
+ new ExprVars.Action<Var>(){
+ @Override
+ public void var(Collection<Var> acc, Var var)
+ {
+ acc.add(var) ;
+ }
+ } ;
+ ExprNoOpVarsWorker<Var> vv = new ExprNoOpVarsWorker<>(acc, action) ;
+ ExprWalker.walk(vv, expr) ;
+ }
+
public static Set<String> getVarNamesMentioned(Expr expr)
{
Set<String> acc = new HashSet<>() ;
@@ -124,4 +138,17 @@ public class ExprVars
}
}
+
+ static class ExprNoOpVarsWorker<T> extends ExprVarsWorker<T>
+ {
+ public ExprNoOpVarsWorker(Collection<T> acc, Action<T> action) {
+ super(acc, action);
+ // TODO Auto-generated constructor stub
+ }
+
+ public void visit(ExprFunctionOp funcOp) {
+ // Don't include op vars
+ return;
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/jena/blob/a25c72c9/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
index 13e9c7e..56029e5 100644
--- a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
@@ -382,6 +382,100 @@ public class TestTransformEliminateAssignments {
}
@Test
+ public void ineligible_09() {
+ // We can't inline if the assignment will be projected out
+ //@formatter:off
+ testNoChange("(project (?x ?y)",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (bgp (triple ?y <urn:pred> <urn:obj>)))))");;
+ //@formatter:on
+ }
+
+ @Test
+ public void ineligible_10() {
+ // We can't inline out of an EXISTS since the assignment isn't projected
+ // out anyway
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (filter (exprlist ?x (exists",
+ " (extend (?x true)",
+ " (table unit))))",
+ " (table unit)))");
+ //@formatter:on
+ }
+
+ @Test
+ public void ineligible_11() {
+ // We can't inline out of an EXISTS since the assignment isn't projected
+ // out anyway
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (filter (exprlist (exists",
+ " (extend (?x true)",
+ " (table unit))))",
+ " (table unit))))");
+ //@formatter:on
+ }
+
+ @Test
+ public void exists_01() {
+ // We can't inline into an EXISTS since the assignment isn't projected
+ // out anyway and its an n-ary operator so would change semantics
+ // However this makes the assignment unused so can still remove it
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist (exists",
+ " (filter (exprlist ?x)",
+ " (table unit))))",
+ " (extend (?x true)",
+ " (table unit))))"),
+ "(project (?y)",
+ " (filter (exprlist (exists",
+ " (filter (exprlist ?x)",
+ " (table unit))))",
+ " (table unit)))");
+ //@formatter:on
+ }
+
+ @Test
+ public void exists_02() {
+ // Could inline within an exists but still needs to meet other rules
+ // Even though an exists is technically a form of projection can't
+ // discount the variable being needed elsewhere
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (filter (exprlist (exists",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (table unit)))))",
+ " (table unit)))");
+ //@formatter:on
+ }
+
+ @Test
+ public void exists_03() {
+ // Can inline within an exists provided it meets normal conditions of
+ // being inside a projection
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist (exists",
+ " (project (?y)",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (table unit))))))",
+ " (table unit)))"),
+ "(project (?y)",
+ " (filter (exprlist (exists",
+ " (project (?y)",
+ " (filter (exprlist true)",
+ " (table unit)))))",
+ " (table unit)))");
+ //@formatter:on
+ }
+
+ @Test
public void through_project_01() {
// We can inline out through a project by also eliminating the variable
// from the project
@@ -393,7 +487,8 @@ public class TestTransformEliminateAssignments {
" (table unit)))))"),
"(project (?y)",
" (filter true",
- " (table unit)))");
+ " (project ()",
+ " (table unit))))");
//@formatter:on
}
[3/7] jena git commit: Additional tests and fixes for assignment
inlining (JENA-780)
Posted by rv...@apache.org.
Additional tests and fixes for assignment inlining (JENA-780)
- Don't inline into EXISTS/NOT EXISTS since those are n-ary operators
- Permit inlining within an EXISTS/NOT EXISTS only if the assignments
are subject to all the normal restrictions
- Don't remove projection when inlining/eliminating through projections
since doing so could cause previously hidden variables to become
visible
- Various additional test cases for these
Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/67bb2482
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/67bb2482
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/67bb2482
Branch: refs/heads/jena2
Commit: 67bb2482edd163127318e5280f3a84142a83cc8b
Parents: a188648
Author: Rob Vesse <rv...@apache.org>
Authored: Wed Jul 8 12:01:28 2015 +0100
Committer: Rob Vesse <rv...@apache.org>
Committed: Wed Jul 8 12:01:28 2015 +0100
----------------------------------------------------------------------
.../optimize/TransformEliminateAssignments.java | 61 ++++++++++--
.../optimize/TransformRemoveAssignment.java | 7 +-
.../com/hp/hpl/jena/sparql/expr/ExprVars.java | 27 ++++++
.../TestTransformEliminateAssignments.java | 97 +++++++++++++++++++-
4 files changed, 178 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/jena/blob/67bb2482/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
index 7044cc9..d36f036 100644
--- a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
+++ b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
@@ -21,13 +21,16 @@ package com.hp.hpl.jena.sparql.algebra.optimize;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import org.apache.jena.atlas.lib.CollectionUtils;
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.OpVisitor;
import com.hp.hpl.jena.sparql.algebra.OpVisitorBase;
@@ -47,11 +50,15 @@ import com.hp.hpl.jena.sparql.algebra.op.OpTopN;
import com.hp.hpl.jena.sparql.algebra.op.OpUnion;
import com.hp.hpl.jena.sparql.core.Var;
import com.hp.hpl.jena.sparql.core.VarExprList;
+import com.hp.hpl.jena.sparql.expr.E_Exists;
+import com.hp.hpl.jena.sparql.expr.E_NotExists;
import com.hp.hpl.jena.sparql.expr.Expr;
import com.hp.hpl.jena.sparql.expr.ExprAggregator;
+import com.hp.hpl.jena.sparql.expr.ExprFunctionOp;
import com.hp.hpl.jena.sparql.expr.ExprLib;
import com.hp.hpl.jena.sparql.expr.ExprList;
import com.hp.hpl.jena.sparql.expr.ExprTransform;
+import com.hp.hpl.jena.sparql.expr.ExprTransformCopy;
import com.hp.hpl.jena.sparql.expr.ExprTransformSubstitute;
import com.hp.hpl.jena.sparql.expr.ExprTransformer;
import com.hp.hpl.jena.sparql.expr.ExprVars;
@@ -103,8 +110,9 @@ public class TransformEliminateAssignments extends TransformCopy {
AssignmentPusher pusher = new AssignmentPusher(tracker);
AssignmentPopper popper = new AssignmentPopper(tracker);
Transform transform = new TransformEliminateAssignments(tracker, pusher, popper, aggressive);
+ ExprTransform exprTransform = new ExprTransformEliminateAssignments(aggressive);
- return Transformer.transformSkipService(transform, op, pusher, popper);
+ return Transformer.transformSkipService(transform, exprTransform, op, pusher, popper);
}
private final OpVisitor before, after;
@@ -165,9 +173,9 @@ public class TransformEliminateAssignments extends TransformCopy {
return super.transform(opFilter, subOp);
// See what vars are used in the filter
- Collection<Var> vars = new ArrayList<>();
+ Set<Var> vars = new HashSet<>();
for (Expr expr : opFilter.getExprs().getList()) {
- ExprVars.varsMentioned(vars, expr);
+ ExprVars.nonOpVarsMentioned(vars, expr);
}
// Are any of these vars single usage?
@@ -226,8 +234,8 @@ public class TransformEliminateAssignments extends TransformCopy {
Expr currExpr = opExtend.getVarExprList().getExpr(assignVar);
// See what vars are used in the current expression
- Collection<Var> vars = new ArrayList<>();
- ExprVars.varsMentioned(vars, currExpr);
+ Set<Var> vars = new HashSet<Var>();
+ ExprVars.nonOpVarsMentioned(vars, currExpr);
// See if we can inline anything
for (Var var : vars) {
@@ -547,12 +555,13 @@ public class TransformEliminateAssignments extends TransformCopy {
// the LHS we could keep it but for now we don't try and do this
unsafe();
}
-
+
@Override
public void visit(OpMinus opMinus) {
+ // Anything from the RHS doesn't project out anyway
unsafe();
}
-
+
@Override
public void visit(OpJoin opJoin) {
unsafe();
@@ -565,4 +574,42 @@ public class TransformEliminateAssignments extends TransformCopy {
tracker.getAssignments().clear();
}
}
+
+ /**
+ * Handles expression transforms for eliminating assignments
+ */
+ private static class ExprTransformEliminateAssignments extends ExprTransformCopy {
+
+ private final boolean aggressive;
+
+ /**
+ * @param aggressive
+ * Whether to inline aggressively
+ */
+ public ExprTransformEliminateAssignments(boolean aggressive) {
+ this.aggressive = aggressive;
+ }
+
+ @Override
+ public Expr transform(ExprFunctionOp funcOp, ExprList args, Op opArg) {
+ // Need to use fresh visitors when working inside an exists/not
+ // exists as we should only do self-contained inlining
+
+ AssignmentTracker tracker = new AssignmentTracker();
+ AssignmentPusher pusher = new AssignmentPusher(tracker);
+ AssignmentPopper popper = new AssignmentPopper(tracker);
+ Transform transform = new TransformEliminateAssignments(tracker, pusher, popper, aggressive);
+ ExprTransformEliminateAssignments exprTransform = new ExprTransformEliminateAssignments(aggressive);
+
+ Op opArg2 = Transformer.transform(transform, exprTransform, opArg, pusher, popper);
+ if (opArg2 == opArg)
+ return super.transform(funcOp, args, opArg);
+ if (funcOp instanceof E_Exists)
+ return new E_Exists(opArg2);
+ if (funcOp instanceof E_NotExists)
+ return new E_NotExists(opArg2);
+ throw new ARQInternalErrorException("Unrecognized ExprFunctionOp: \n" + funcOp);
+ }
+
+ }
}
http://git-wip-us.apache.org/repos/asf/jena/blob/67bb2482/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
index 855ec10..da56116 100644
--- a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
+++ b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
@@ -122,11 +122,6 @@ public class TransformRemoveAssignment extends TransformCopy {
List<Var> newVars = opProject.getVars();
newVars.remove(this.var);
- if (newVars.size() > 0) {
- return new OpProject(subOp, newVars);
- } else {
- return subOp;
- }
+ return new OpProject(subOp, newVars);
}
-
}
http://git-wip-us.apache.org/repos/asf/jena/blob/67bb2482/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprVars.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprVars.java b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprVars.java
index 03da875..7afe2d2 100644
--- a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprVars.java
+++ b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/expr/ExprVars.java
@@ -59,6 +59,20 @@ public class ExprVars
ExprWalker.walk(vv, expr) ;
}
+ public static void nonOpVarsMentioned(Collection<Var> acc, Expr expr)
+ {
+ ExprVars.Action<Var> action =
+ new ExprVars.Action<Var>(){
+ @Override
+ public void var(Collection<Var> acc, Var var)
+ {
+ acc.add(var) ;
+ }
+ } ;
+ ExprNoOpVarsWorker<Var> vv = new ExprNoOpVarsWorker<>(acc, action) ;
+ ExprWalker.walk(vv, expr) ;
+ }
+
public static Set<String> getVarNamesMentioned(Expr expr)
{
Set<String> acc = new HashSet<>() ;
@@ -124,4 +138,17 @@ public class ExprVars
}
}
+
+ static class ExprNoOpVarsWorker<T> extends ExprVarsWorker<T>
+ {
+ public ExprNoOpVarsWorker(Collection<T> acc, Action<T> action) {
+ super(acc, action);
+ // TODO Auto-generated constructor stub
+ }
+
+ public void visit(ExprFunctionOp funcOp) {
+ // Don't include op vars
+ return;
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/jena/blob/67bb2482/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
index ed96785..52c6cf3 100644
--- a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
+++ b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
@@ -440,6 +440,100 @@ public class TestTransformEliminateAssignments {
}
@Test
+ public void ineligible_09() {
+ // We can't inline if the assignment will be projected out
+ //@formatter:off
+ testNoChange("(project (?x ?y)",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (bgp (triple ?y <urn:pred> <urn:obj>)))))");;
+ //@formatter:on
+ }
+
+ @Test
+ public void ineligible_10() {
+ // We can't inline out of an EXISTS since the assignment isn't projected
+ // out anyway
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (filter (exprlist ?x (exists",
+ " (extend (?x true)",
+ " (table unit))))",
+ " (table unit)))");
+ //@formatter:on
+ }
+
+ @Test
+ public void ineligible_11() {
+ // We can't inline out of an EXISTS since the assignment isn't projected
+ // out anyway
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (filter (exprlist (exists",
+ " (extend (?x true)",
+ " (table unit))))",
+ " (table unit))))");
+ //@formatter:on
+ }
+
+ @Test
+ public void exists_01() {
+ // We can't inline into an EXISTS since the assignment isn't projected
+ // out anyway and its an n-ary operator so would change semantics
+ // However this makes the assignment unused so can still remove it
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist (exists",
+ " (filter (exprlist ?x)",
+ " (table unit))))",
+ " (extend (?x true)",
+ " (table unit))))"),
+ "(project (?y)",
+ " (filter (exprlist (exists",
+ " (filter (exprlist ?x)",
+ " (table unit))))",
+ " (table unit)))");
+ //@formatter:on
+ }
+
+ @Test
+ public void exists_02() {
+ // Could inline within an exists but still needs to meet other rules
+ // Even though an exists is technically a form of projection can't
+ // discount the variable being needed elsewhere
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (filter (exprlist (exists",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (table unit)))))",
+ " (table unit)))");
+ //@formatter:on
+ }
+
+ @Test
+ public void exists_03() {
+ // Can inline within an exists provided it meets normal conditions of
+ // being inside a projection
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist (exists",
+ " (project (?y)",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (table unit))))))",
+ " (table unit)))"),
+ "(project (?y)",
+ " (filter (exprlist (exists",
+ " (project (?y)",
+ " (filter (exprlist true)",
+ " (table unit)))))",
+ " (table unit)))");
+ //@formatter:on
+ }
+
+ @Test
public void through_project_01() {
// We can inline out through a project by also eliminating the variable
// from the project
@@ -451,7 +545,8 @@ public class TestTransformEliminateAssignments {
" (table unit)))))"),
"(project (?y)",
" (filter true",
- " (table unit)))");
+ " (project ()",
+ " (table unit))))");
//@formatter:on
}
[4/7] jena git commit: Yet more fixes and test cases for assignment
inlining (JENA-780)
Posted by rv...@apache.org.
Yet more fixes and test cases for assignment inlining (JENA-780)
- Don't inline out of DISTINCT/REDUCED
- Ensure we don't break projections when editing out inlined assignments
that were projected
- Ensure that editing projections only edits the outermost projection
Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/9bc5aa97
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/9bc5aa97
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/9bc5aa97
Branch: refs/heads/jena2
Commit: 9bc5aa972d3c689b7a8e5052f3a19092d07a9ada
Parents: 67bb248
Author: Rob Vesse <rv...@apache.org>
Authored: Wed Jul 8 15:00:21 2015 +0100
Committer: Rob Vesse <rv...@apache.org>
Committed: Wed Jul 8 15:00:21 2015 +0100
----------------------------------------------------------------------
.../optimize/TransformEliminateAssignments.java | 61 +++++++++++--
.../optimize/TransformRemoveAssignment.java | 20 ++++-
.../TestTransformEliminateAssignments.java | 94 ++++++++++++++++++++
3 files changed, 164 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/jena/blob/9bc5aa97/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
index d36f036..ee2b91d 100644
--- a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
+++ b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
@@ -37,6 +37,7 @@ import com.hp.hpl.jena.sparql.algebra.OpVisitorBase;
import com.hp.hpl.jena.sparql.algebra.Transform;
import com.hp.hpl.jena.sparql.algebra.TransformCopy;
import com.hp.hpl.jena.sparql.algebra.Transformer;
+import com.hp.hpl.jena.sparql.algebra.op.OpDistinct;
import com.hp.hpl.jena.sparql.algebra.op.OpExt;
import com.hp.hpl.jena.sparql.algebra.op.OpExtend;
import com.hp.hpl.jena.sparql.algebra.op.OpFilter;
@@ -46,6 +47,7 @@ import com.hp.hpl.jena.sparql.algebra.op.OpLeftJoin;
import com.hp.hpl.jena.sparql.algebra.op.OpMinus;
import com.hp.hpl.jena.sparql.algebra.op.OpOrder;
import com.hp.hpl.jena.sparql.algebra.op.OpProject;
+import com.hp.hpl.jena.sparql.algebra.op.OpReduced;
import com.hp.hpl.jena.sparql.algebra.op.OpTopN;
import com.hp.hpl.jena.sparql.algebra.op.OpUnion;
import com.hp.hpl.jena.sparql.core.Var;
@@ -71,10 +73,11 @@ import com.hp.hpl.jena.sparql.expr.NodeValue;
* </p>
* <ol>
* <li>Assignments where the assigned variable is used only once in a subsequent
- * assignment can be in-lined</li>
+ * expression can be in-lined</li>
* <li>Assignments where the assigned value is never used elsewhere can be
* eliminated</li>
* </ol>
+ * <h3>Eligibility for In-lining</h3>
* <p>
* Both of these changes can only happen inside of projections as otherwise we
* have to assume that the user may need the resulting variable and thus we
@@ -84,19 +87,45 @@ import com.hp.hpl.jena.sparql.expr.NodeValue;
* expression is deterministic is defined by {@link ExprLib#isStable(Expr)}.
* </p>
* <p>
+ * In-lining must also respect variable scope, it is possible with a nested
+ * query to have an assignment in-lined out through a projection that projects
+ * it provided that the projection is appropriately modified.
+ * </p>
+ * <p>
+ * There are also various other conditions on assignments that might be eligible
+ * for in-lining:
+ * </p>
+ * <ul>
+ * <li>They cannot occur inside a n-ary operator (e.g. join, {@code UNION},
+ * {@code OPTIONAL} etc.) because then in-lining would change semantics because
+ * an expression that previously was only valid for part of the query might
+ * become valid for a larger part of the query</li>
+ * <li>They cannot be in-lined into an {@code EXISTS} or {@code NOT EXISTS} in a
+ * filter</li>
+ * <li>They cannot be in-lined out of a {@code DISTINCT} or {@code REDUCED}
+ * because the assignment would be relevant for the purposes of distinctness</li>
+ * </ul>
+ * <p>
+ * Please see <a
+ * href="https://issues.apache.org/jira/browse/JENA-780">JENA-780</a> for more
+ * information on this.
+ * </p>
+ * <h3>In-lining Application</h3>
+ * <p>
* Assignments may be in-lined in the following places:
* </p>
* <ul>
- * <li>Filter Expressions</li>
- * <li>Bind and Select Expressions</li>
- * <li>Order By Expressions if aggressive in-lining is enabled or the assigned
- * expression is a constant</li>
+ * <li>{@code FILTER} Expressions</li>
+ * <li>{@code BIND} and Project Expressions</li>
+ * <li>{@code ORDER BY} Expressions if aggressive in-lining is enabled or the
+ * assigned expression is a constant</li>
* </ul>
* <p>
- * In the case of order by we only in-line assignments when aggressive mode is
- * set as the realities of order by are that expressions may be recomputed
- * multiple times and so in-lining may actually hurt performance in those cases
- * unless the expression to be in-lined is itself a constant.
+ * In the case of {@code ORDER BY} we only in-line assignments when aggressive
+ * mode is set unless the assignment is a constant value. This is because during
+ * order by evaluation expressions may be recomputed multiple times and so
+ * in-lining may actually hurt performance in those cases unless the expression
+ * to be in-lined is itself a constant.
* </p>
*/
public class TransformEliminateAssignments extends TransformCopy {
@@ -567,6 +596,20 @@ public class TransformEliminateAssignments extends TransformCopy {
unsafe();
}
+ @Override
+ public void visit(OpDistinct opDistinct) {
+ // Inlining out through the distinct might change the results of the
+ // distinct
+ unsafe();
+ }
+
+ @Override
+ public void visit(OpReduced opReduced) {
+ // Inlining out through the reduced might change the results of the
+ // reduced
+ unsafe();
+ }
+
private void unsafe() {
// Throw out any assignments because if they would be eligible their
// values can't be bound in every branch of the union and thus
http://git-wip-us.apache.org/repos/asf/jena/blob/9bc5aa97/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
index da56116..ceddcff 100644
--- a/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
+++ b/jena-arq/src/main/java/com/hp/hpl/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
@@ -18,6 +18,7 @@
package com.hp.hpl.jena.sparql.algebra.optimize;
+import java.util.ArrayList;
import java.util.List;
import com.hp.hpl.jena.sparql.algebra.Op;
@@ -39,6 +40,7 @@ public class TransformRemoveAssignment extends TransformCopy {
private Var var;
private Expr expr;
private boolean topmostOnly = true;
+ private boolean aboveExtend = false;
public TransformRemoveAssignment(Var var, Expr expr, boolean topmostOnly) {
this.var = var;
@@ -88,6 +90,9 @@ public class TransformRemoveAssignment extends TransformCopy {
modified.add(v, orig.getExpr(v));
}
}
+ if (modified.size() > 0 && modified.size() == orig.size())
+ return null;
+
return modified;
}
@@ -96,6 +101,8 @@ public class TransformRemoveAssignment extends TransformCopy {
VarExprList assignments = processAssignments(opExtend);
if (assignments == null)
return super.transform(opExtend, subOp);
+
+ this.aboveExtend = true;
// Rewrite appropriately
if (this.topmostOnly) {
@@ -114,14 +121,23 @@ public class TransformRemoveAssignment extends TransformCopy {
return subOp;
}
}
+
}
public Op transform(OpProject opProject, Op subOp) {
if (!opProject.getVars().contains(this.var))
return super.transform(opProject, subOp);
- List<Var> newVars = opProject.getVars();
+ List<Var> newVars = new ArrayList<Var>(opProject.getVars());
newVars.remove(this.var);
- return new OpProject(subOp, newVars);
+ if (this.topmostOnly) {
+ if (this.aboveExtend) {
+ return new OpProject(subOp, newVars);
+ } else {
+ return opProject;
+ }
+ } else {
+ return new OpProject(subOp, newVars);
+ }
}
}
http://git-wip-us.apache.org/repos/asf/jena/blob/9bc5aa97/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
index 52c6cf3..e8e97be 100644
--- a/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
+++ b/jena-arq/src/test/java/com/hp/hpl/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
@@ -478,6 +478,30 @@ public class TestTransformEliminateAssignments {
}
@Test
+ public void ineligible_12() {
+ // Can't inline through a distinct
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (distinct",
+ " (extend (?x true)",
+ " (bgp (triple ?s ?p ?o))))))");
+ //@formatter:on
+ }
+
+ @Test
+ public void ineligible_13() {
+ // Can't inline through a reduced
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (reduced",
+ " (extend (?x true)",
+ " (bgp (triple ?s ?p ?o))))))");
+ //@formatter:on
+ }
+
+ @Test
public void exists_01() {
// We can't inline into an EXISTS since the assignment isn't projected
// out anyway and its an n-ary operator so would change semantics
@@ -587,6 +611,18 @@ public class TestTransformEliminateAssignments {
}
@Test
+ public void through_project_04() {
+ // Can't inline because the value is projected out
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (project (?x ?y)",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (bgp (triple ?y <urn:pred> <urn:obj>))))))");
+ //@formatter:on
+ }
+
+ @Test
public void no_merge_01() {
// We should not merge extends
//@formatter:off
@@ -646,4 +682,62 @@ public class TestTransformEliminateAssignments {
" (table unit))))");
//@formatter:on
}
+
+ @Test
+ public void scope_04() {
+ // Only the topmost assignment should be inlined since any deeper
+ // assignments are technically different variables
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (project (?z)",
+ " (filter (exprlist (|| ?x (! ?x)))",
+ " (extend (?x false)",
+ " (table unit)))))))"),
+ "(project (?y)",
+ " (filter (exprlist true)",
+ " (project (?z)",
+ " (filter (exprlist (|| ?x (! ?x)))",
+ " (extend (?x false)",
+ " (table unit))))))");
+ //@formatter:on
+ }
+
+ @Test
+ public void scope_05() {
+ // Technically invalid query but validates that the inner projection of
+ // ?x blocks later in-lining
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (project (?x)",
+ " (filter (exprlist (|| ?x (! ?x)))",
+ " (extend (?x false)",
+ " (table unit)))))))");
+ //@formatter:off
+ }
+
+ @Test
+ public void scope_06() {
+ // In-lining in the outer scope should not change the inner scope
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (project (?z)",
+ " (project (?x)",
+ " (filter (exprlist (|| ?x (! ?x)))",
+ " (extend (?x false)",
+ " (table unit))))))))"),
+ "(project (?y)",
+ " (filter (exprlist true)",
+ " (project (?z)",
+ " (project (?x)",
+ " (filter (exprlist (|| ?x (! ?x)))",
+ " (extend (?x false)",
+ " (table unit)))))))");
+ //@formatter:off
+ }
}
[5/7] jena git commit: Add additional test cases for inlining
(JENA-780)
Posted by rv...@apache.org.
Add additional test cases for inlining (JENA-780)
Adds some more test cases which checks behaviour with inlining through
projections which is possible if the assignment is projected and we're
in a stack of unary operators
Conflicts:
jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/2031b78f
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/2031b78f
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/2031b78f
Branch: refs/heads/master
Commit: 2031b78fdc160e21ca56a60e9434c5c74b608cae
Parents: d0d7664
Author: Rob Vesse <rv...@apache.org>
Authored: Tue Jul 7 17:47:08 2015 +0100
Committer: Rob Vesse <rv...@apache.org>
Committed: Wed Jul 8 15:07:46 2015 +0100
----------------------------------------------------------------------
.../optimize/TransformRemoveAssignment.java | 13 +++++
.../TestTransformEliminateAssignments.java | 52 ++++++++++++++++++++
2 files changed, 65 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/jena/blob/2031b78f/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
index 564d3b3..833de93 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
@@ -113,4 +113,17 @@ public class TransformRemoveAssignment extends TransformCopy {
}
}
+ public Op transform(OpProject opProject, Op subOp) {
+ if (!opProject.getVars().contains(this.var))
+ return super.transform(opProject, subOp);
+
+ List<Var> newVars = opProject.getVars();
+ newVars.remove(this.var);
+ if (newVars.size() > 0) {
+ return new OpProject(subOp, newVars);
+ } else {
+ return subOp;
+ }
+ }
+
}
http://git-wip-us.apache.org/repos/asf/jena/blob/2031b78f/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
index 117bd32..13e9c7e 100644
--- a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
@@ -382,6 +382,58 @@ public class TestTransformEliminateAssignments {
}
@Test
+ public void through_project_01() {
+ // We can inline out through a project by also eliminating the variable
+ // from the project
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (project (?x)",
+ " (extend (?x true)",
+ " (table unit)))))"),
+ "(project (?y)",
+ " (filter true",
+ " (table unit)))");
+ //@formatter:on
+ }
+
+ @Test
+ public void through_project_02() {
+ // We can inline out through a project by also eliminating the variable
+ // from the project
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (project (?x ?y)",
+ " (extend (?x true)",
+ " (bgp (triple ?y <urn:pred> <urn:obj>))))))"),
+ "(project (?y)",
+ " (filter true",
+ " (project (?y)",
+ " (bgp (triple ?y <urn:pred> <urn:obj>)))))");
+ //@formatter:on
+ }
+
+ @Test
+ public void through_project_03() {
+ // We can't inline out through a project if the assignment is not
+ // projected
+ // However we can still eliminate it from the inner projection if that
+ // would render it unused
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (project (?y)",
+ " (extend (?x true)",
+ " (table unit)))))"),
+ "(project (?y)",
+ " (filter (exprlist ?x)",
+ " (project (?y)",
+ " (table unit))))");
+ //@formatter:on
+ }
+
+ @Test
public void no_merge_01() {
// We should not merge extends
//@formatter:off
[7/7] jena git commit: Yet more fixes and test cases for assignment
inlining (JENA-780)
Posted by rv...@apache.org.
Yet more fixes and test cases for assignment inlining (JENA-780)
- Don't inline out of DISTINCT/REDUCED
- Ensure we don't break projections when editing out inlined assignments
that were projected
- Ensure that editing projections only edits the outermost projection
Conflicts:
jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/833bc704
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/833bc704
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/833bc704
Branch: refs/heads/master
Commit: 833bc7049d689c8fa3c177f705b24fe4ba6f7c86
Parents: a25c72c
Author: Rob Vesse <rv...@apache.org>
Authored: Wed Jul 8 15:00:21 2015 +0100
Committer: Rob Vesse <rv...@apache.org>
Committed: Wed Jul 8 15:11:23 2015 +0100
----------------------------------------------------------------------
.../optimize/TransformEliminateAssignments.java | 59 ++++++++++--
.../optimize/TransformRemoveAssignment.java | 23 ++++-
.../TestTransformEliminateAssignments.java | 94 ++++++++++++++++++++
3 files changed, 165 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/jena/blob/833bc704/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
index 3ce4198..818434b 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformEliminateAssignments.java
@@ -62,10 +62,11 @@ import org.apache.jena.sparql.expr.NodeValue;
* </p>
* <ol>
* <li>Assignments where the assigned variable is used only once in a subsequent
- * assignment can be in-lined</li>
+ * expression can be in-lined</li>
* <li>Assignments where the assigned value is never used elsewhere can be
* eliminated</li>
* </ol>
+ * <h3>Eligibility for In-lining</h3>
* <p>
* Both of these changes can only happen inside of projections as otherwise we
* have to assume that the user may need the resulting variable and thus we
@@ -75,19 +76,45 @@ import org.apache.jena.sparql.expr.NodeValue;
* expression is deterministic is defined by {@link ExprLib#isStable(Expr)}.
* </p>
* <p>
+ * In-lining must also respect variable scope, it is possible with a nested
+ * query to have an assignment in-lined out through a projection that projects
+ * it provided that the projection is appropriately modified.
+ * </p>
+ * <p>
+ * There are also various other conditions on assignments that might be eligible
+ * for in-lining:
+ * </p>
+ * <ul>
+ * <li>They cannot occur inside a n-ary operator (e.g. join, {@code UNION},
+ * {@code OPTIONAL} etc.) because then in-lining would change semantics because
+ * an expression that previously was only valid for part of the query might
+ * become valid for a larger part of the query</li>
+ * <li>They cannot be in-lined into an {@code EXISTS} or {@code NOT EXISTS} in a
+ * filter</li>
+ * <li>They cannot be in-lined out of a {@code DISTINCT} or {@code REDUCED}
+ * because the assignment would be relevant for the purposes of distinctness</li>
+ * </ul>
+ * <p>
+ * Please see <a
+ * href="https://issues.apache.org/jira/browse/JENA-780">JENA-780</a> for more
+ * information on this.
+ * </p>
+ * <h3>In-lining Application</h3>
+ * <p>
* Assignments may be in-lined in the following places:
* </p>
* <ul>
- * <li>Filter Expressions</li>
- * <li>Bind and Select Expressions</li>
- * <li>Order By Expressions if aggressive in-lining is enabled or the assigned
- * expression is a constant</li>
+ * <li>{@code FILTER} Expressions</li>
+ * <li>{@code BIND} and Project Expressions</li>
+ * <li>{@code ORDER BY} Expressions if aggressive in-lining is enabled or the
+ * assigned expression is a constant</li>
* </ul>
* <p>
- * In the case of order by we only in-line assignments when aggressive mode is
- * set as the realities of order by are that expressions may be recomputed
- * multiple times and so in-lining may actually hurt performance in those cases
- * unless the expression to be in-lined is itself a constant.
+ * In the case of {@code ORDER BY} we only in-line assignments when aggressive
+ * mode is set unless the assignment is a constant value. This is because during
+ * order by evaluation expressions may be recomputed multiple times and so
+ * in-lining may actually hurt performance in those cases unless the expression
+ * to be in-lined is itself a constant.
* </p>
*/
public class TransformEliminateAssignments extends TransformCopy {
@@ -560,6 +587,20 @@ public class TransformEliminateAssignments extends TransformCopy {
unsafe();
}
+ @Override
+ public void visit(OpDistinct opDistinct) {
+ // Inlining out through the distinct might change the results of the
+ // distinct
+ unsafe();
+ }
+
+ @Override
+ public void visit(OpReduced opReduced) {
+ // Inlining out through the reduced might change the results of the
+ // reduced
+ unsafe();
+ }
+
private void unsafe() {
// Throw out any assignments because if they would be eligible their
// values can't be bound in every branch of the union and thus
http://git-wip-us.apache.org/repos/asf/jena/blob/833bc704/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
index fd71ffb..503a13c 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformRemoveAssignment.java
@@ -18,11 +18,15 @@
package org.apache.jena.sparql.algebra.optimize;
+import java.util.ArrayList;
+import java.util.List;
+
import org.apache.jena.sparql.algebra.Op;
import org.apache.jena.sparql.algebra.TransformCopy;
import org.apache.jena.sparql.algebra.op.OpAssign;
import org.apache.jena.sparql.algebra.op.OpExtend;
import org.apache.jena.sparql.algebra.op.OpExtendAssign;
+import org.apache.jena.sparql.algebra.op.OpProject;
import org.apache.jena.sparql.core.Var;
import org.apache.jena.sparql.core.VarExprList;
import org.apache.jena.sparql.expr.Expr;
@@ -36,6 +40,7 @@ public class TransformRemoveAssignment extends TransformCopy {
private Var var;
private Expr expr;
private boolean topmostOnly = true;
+ private boolean aboveExtend = false;
public TransformRemoveAssignment(Var var, Expr expr, boolean topmostOnly) {
this.var = var;
@@ -85,6 +90,9 @@ public class TransformRemoveAssignment extends TransformCopy {
modified.add(v, orig.getExpr(v));
}
}
+ if (modified.size() > 0 && modified.size() == orig.size())
+ return null;
+
return modified;
}
@@ -93,6 +101,8 @@ public class TransformRemoveAssignment extends TransformCopy {
VarExprList assignments = processAssignments(opExtend);
if (assignments == null)
return super.transform(opExtend, subOp);
+
+ this.aboveExtend = true;
// Rewrite appropriately
if (this.topmostOnly) {
@@ -111,14 +121,23 @@ public class TransformRemoveAssignment extends TransformCopy {
return subOp;
}
}
+
}
public Op transform(OpProject opProject, Op subOp) {
if (!opProject.getVars().contains(this.var))
return super.transform(opProject, subOp);
- List<Var> newVars = opProject.getVars();
+ List<Var> newVars = new ArrayList<Var>(opProject.getVars());
newVars.remove(this.var);
- return new OpProject(subOp, newVars);
+ if (this.topmostOnly) {
+ if (this.aboveExtend) {
+ return new OpProject(subOp, newVars);
+ } else {
+ return opProject;
+ }
+ } else {
+ return new OpProject(subOp, newVars);
+ }
}
}
http://git-wip-us.apache.org/repos/asf/jena/blob/833bc704/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
index 56029e5..4c6c7f2 100644
--- a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformEliminateAssignments.java
@@ -420,6 +420,30 @@ public class TestTransformEliminateAssignments {
}
@Test
+ public void ineligible_12() {
+ // Can't inline through a distinct
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (distinct",
+ " (extend (?x true)",
+ " (bgp (triple ?s ?p ?o))))))");
+ //@formatter:on
+ }
+
+ @Test
+ public void ineligible_13() {
+ // Can't inline through a reduced
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (reduced",
+ " (extend (?x true)",
+ " (bgp (triple ?s ?p ?o))))))");
+ //@formatter:on
+ }
+
+ @Test
public void exists_01() {
// We can't inline into an EXISTS since the assignment isn't projected
// out anyway and its an n-ary operator so would change semantics
@@ -529,6 +553,18 @@ public class TestTransformEliminateAssignments {
}
@Test
+ public void through_project_04() {
+ // Can't inline because the value is projected out
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (project (?x ?y)",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (bgp (triple ?y <urn:pred> <urn:obj>))))))");
+ //@formatter:on
+ }
+
+ @Test
public void no_merge_01() {
// We should not merge extends
//@formatter:off
@@ -588,4 +624,62 @@ public class TestTransformEliminateAssignments {
" (table unit))))");
//@formatter:on
}
+
+ @Test
+ public void scope_04() {
+ // Only the topmost assignment should be inlined since any deeper
+ // assignments are technically different variables
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (project (?z)",
+ " (filter (exprlist (|| ?x (! ?x)))",
+ " (extend (?x false)",
+ " (table unit)))))))"),
+ "(project (?y)",
+ " (filter (exprlist true)",
+ " (project (?z)",
+ " (filter (exprlist (|| ?x (! ?x)))",
+ " (extend (?x false)",
+ " (table unit))))))");
+ //@formatter:on
+ }
+
+ @Test
+ public void scope_05() {
+ // Technically invalid query but validates that the inner projection of
+ // ?x blocks later in-lining
+ //@formatter:off
+ testNoChange("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (project (?x)",
+ " (filter (exprlist (|| ?x (! ?x)))",
+ " (extend (?x false)",
+ " (table unit)))))))");
+ //@formatter:off
+ }
+
+ @Test
+ public void scope_06() {
+ // In-lining in the outer scope should not change the inner scope
+ //@formatter:off
+ test(StrUtils.strjoinNL("(project (?y)",
+ " (filter (exprlist ?x)",
+ " (extend (?x true)",
+ " (project (?z)",
+ " (project (?x)",
+ " (filter (exprlist (|| ?x (! ?x)))",
+ " (extend (?x false)",
+ " (table unit))))))))"),
+ "(project (?y)",
+ " (filter (exprlist true)",
+ " (project (?z)",
+ " (project (?x)",
+ " (filter (exprlist (|| ?x (! ?x)))",
+ " (extend (?x false)",
+ " (table unit)))))))");
+ //@formatter:off
+ }
}