AFAIK quad transform is the only one affected since none of the other transformations in ARQ require any external state to be maintained by visitors and we don't have any custom ones for Urika that need this.
Historically we put quad transformation partway through our optimisation pipeline because we thought that at least some ARQ optimisations only expected to work over BGPs which may be an entirely incorrect and unfounded belief The workaround for us would probably be to move the quad form to the front of our list of transforms Rob On 11/06/2014 13:04, "Andy Seaborne" <[email protected]> wrote: >Rob - other than the quad transform, what other transforms are affected? > >I ask because one approach is to work either in quad-algebra or in >non-quad-algebra and make that choice at the start. > >In other words, consider the quad transform as special; it just happens >to be implemented as transform but that's an implementation detail. > >If it's not just quadization, then yes, there is something here. > > Andy > >On 10/06/14 15:48, Andy Seaborne wrote: >> (briefly for now - JENA-710 needed some attention as it's a "bit >>exciting") >> >> Yes - the quad transformation is special because it is a pair of classes >> managing the graph setting (push on entry, pop on exit) and that's a >> top-down action. The transformation itself is bottom up tree rewrite in >> the usual fashion. >> >> Let me look at your example a bit. There are some possibilities that >> come to mind but I need to refresh my detailed knowledge first. >> >> ----- >> General observation: externally extension nodes and the visitor or >> transformer patterns don't play well together. You have at least >> remember to add OpExt to the visitor at the right point and there isn't >> a automatic check for new types. >> >> Andy >> >> On 10/06/14 11:56, Rob Vesse wrote: >>> Digging into this a little more I think I understand slightly more >>> what is >>> going on. All transforms are applied bottom up it is just that some >>>also >>> use visitors to track necessary state in a top down fashion and the >>>quad >>> transform is an example of this. >>> >>> It expects to be have before and after visitors hooked up to it which >>> handle tracking the current active graph as it descends the algebra. >>> Something about how I'm applying the transform to my inner operator >>> causes >>> the visitors to not be fired at all so they never track the graph at >>>all >>> >>> I'm not sure how to resolve this because the point at which apply() is >>> called I have no idea if additional visitors were in use in the >>>original >>> transformation calls >>> >>> Rob >>> >>> On 10/06/2014 11:06, "Claude Warren" <[email protected]> wrote: >>> >>>> Are there a class of operators that should not be done bottom up? I >>>>am >>>> thinking along the lines of there are classes of operators that are >>>>not >>>> transitive. Perhaps there is a name for this property. If we can >>>> identify >>>> it and name it, we could flag it in the operator implementation so >>>>that >>>> the >>>> transform would work correctly. >>>> >>>> Just a thought, >>>> Claude >>>> >>>> >>>> On Tue, Jun 10, 2014 at 10:20 AM, Rob Vesse <[email protected]> >>>> wrote: >>>> >>>>> Andy, >>>>> >>>>> I've run into a thorny problem to do with applying transformations to >>>>> custom >>>>> algebra operators. Essentially we have a number of custom algebra >>>>> operators >>>>> defined (which derived from OpExt) and in order to try and allow >>>>> optimisations to work correctly for these we override the apply() >>>>> method >>>>> and >>>>> create a new instance of the custom operator with the transformer >>>>> applied >>>>> to >>>>> the sub operators using the Transformer.transform() helper method. >>>>> >>>>> This works great for bottom up transformations but for top down >>>>> transformations e.g transformation to quad form we've found this can >>>>> result >>>>> in incorrect transformations. The following code is a trivial >>>>> demonstration >>>>> of this problem: >>>>> >>>>> package org.apache.jena.playground; >>>>> >>>>> >>>>> >>>>> import org.apache.jena.atlas.io.IndentedWriter; >>>>> >>>>> >>>>> >>>>> import com.hp.hpl.jena.graph.Node; >>>>> >>>>> import com.hp.hpl.jena.graph.NodeFactory; >>>>> >>>>> import com.hp.hpl.jena.graph.Triple; >>>>> >>>>> import com.hp.hpl.jena.sparql.algebra.AlgebraQuad; >>>>> >>>>> import com.hp.hpl.jena.sparql.algebra.Op; >>>>> >>>>> import com.hp.hpl.jena.sparql.algebra.Transform; >>>>> >>>>> import com.hp.hpl.jena.sparql.algebra.Transformer; >>>>> >>>>> import com.hp.hpl.jena.sparql.algebra.op.OpBGP; >>>>> >>>>> import com.hp.hpl.jena.sparql.algebra.op.OpExt; >>>>> >>>>> import com.hp.hpl.jena.sparql.algebra.op.OpGraph; >>>>> >>>>> import com.hp.hpl.jena.sparql.core.BasicPattern; >>>>> >>>>> import com.hp.hpl.jena.sparql.engine.ExecutionContext; >>>>> >>>>> import com.hp.hpl.jena.sparql.engine.QueryIterator; >>>>> >>>>> import com.hp.hpl.jena.sparql.serializer.SerializationContext; >>>>> >>>>> import com.hp.hpl.jena.sparql.util.NodeIsomorphismMap; >>>>> >>>>> >>>>> >>>>> public class CustomOperatorTransformPassing { >>>>> >>>>> >>>>> >>>>> public static void main(String[] args) { >>>>> >>>>> Node customGraph = NodeFactory.createURI("http://graph";); >>>>> >>>>> Node s = NodeFactory.createVariable("s"); >>>>> >>>>> Node p = NodeFactory.createVariable("p"); >>>>> >>>>> Node o = NodeFactory.createVariable("o"); >>>>> >>>>> >>>>> >>>>> BasicPattern bp = new BasicPattern(); >>>>> >>>>> bp.add(new Triple(s, p, o)); >>>>> >>>>> >>>>> >>>>> OpBGP bgp = new OpBGP(bp); >>>>> >>>>> OpGraph graph = new OpGraph(customGraph, bgp); >>>>> >>>>> >>>>> >>>>> // Transform normal algebra to quads >>>>> >>>>> Op quads = AlgebraQuad.quadize(graph); >>>>> >>>>> >>>>> >>>>> System.out.println("Original Algebra:"); >>>>> >>>>> System.out.println(graph.toString()); >>>>> >>>>> System.out.println(); >>>>> >>>>> System.out.println("Quad Form Algebra:"); >>>>> >>>>> System.out.println(quads.toString()); >>>>> >>>>> System.out.println(); >>>>> >>>>> >>>>> >>>>> // Now wrap in custom algebra and repeat >>>>> >>>>> Op foo = new OpFoo(graph); >>>>> >>>>> quads = AlgebraQuad.quadize(foo); >>>>> >>>>> >>>>> >>>>> System.out.println("Original Algebra:"); >>>>> >>>>> System.out.println(foo.toString()); >>>>> >>>>> System.out.println(); >>>>> >>>>> System.out.println("Quad Form Algebra:"); >>>>> >>>>> System.out.println(quads.toString()); >>>>> >>>>> System.out.println(); >>>>> >>>>> } >>>>> >>>>> >>>>> >>>>> /** >>>>> >>>>> * Trivial custom algebra operator which tries to allow >>>>>transforms >>>>> to >>>>> pass >>>>> >>>>> * through it >>>>> >>>>> * >>>>> >>>>> */ >>>>> >>>>> private static class OpFoo extends OpExt { >>>>> >>>>> private Op subOp; >>>>> >>>>> >>>>> >>>>> public OpFoo(Op subOp) { >>>>> >>>>> super("foo"); >>>>> >>>>> this.subOp = subOp; >>>>> >>>>> } >>>>> >>>>> >>>>> >>>>> public Op getSubOp() { >>>>> >>>>> return this.subOp; >>>>> >>>>> } >>>>> >>>>> >>>>> >>>>> @Override >>>>> >>>>> public Op effectiveOp() { >>>>> >>>>> return null; >>>>> >>>>> } >>>>> >>>>> >>>>> >>>>> @Override >>>>> >>>>> public QueryIterator eval(QueryIterator input, >>>>> ExecutionContext >>>>> execCxt) { >>>>> >>>>> throw new UnsupportedOperationException(); >>>>> >>>>> } >>>>> >>>>> >>>>> >>>>> @Override >>>>> >>>>> public Op apply(Transform transform) { >>>>> >>>>> // Apply transforms on inner operator so we don't block >>>>> transformations >>>>> >>>>> return new OpFoo(Transformer.transform(transform, >>>>> this.subOp)); >>>>> >>>>> } >>>>> >>>>> >>>>> >>>>> @Override >>>>> >>>>> public void outputArgs(IndentedWriter out, >>>>> SerializationContext >>>>> sCxt) { >>>>> >>>>> // Not needed as we override output directly >>>>> >>>>> } >>>>> >>>>> >>>>> >>>>> @Override >>>>> >>>>> public void output(IndentedWriter out, SerializationContext >>>>> sCxt) { >>>>> >>>>> out.println("(foo"); >>>>> >>>>> out.incIndent(); >>>>> >>>>> subOp.output(out, sCxt); >>>>> >>>>> out.decIndent(); >>>>> >>>>> out.write(")"); >>>>> >>>>> } >>>>> >>>>> >>>>> >>>>> @Override >>>>> >>>>> public int hashCode() { >>>>> >>>>> return subOp.hashCode(); >>>>> >>>>> } >>>>> >>>>> >>>>> >>>>> @Override >>>>> >>>>> public boolean equalTo(Op other, NodeIsomorphismMap >>>>> labelMap) { >>>>> >>>>> if (other instanceof OpFoo) { >>>>> >>>>> return this.subOp.equalTo(((OpFoo) >>>>>other).getSubOp(), >>>>> labelMap); >>>>> >>>>> } >>>>> >>>>> return false; >>>>> >>>>> } >>>>> >>>>> >>>>> >>>>> } >>>>> >>>>> } >>>>> >>>>> >>>>> >>>>> This can also be found at >>>>> https://gist.github.com/rvesse/85aeac225b7907db1155 for when the >>>>> mailing >>>>> list mangles the code >>>>> >>>>> >>>>> >>>>> It produces the following output: >>>>> >>>>> >>>>> >>>>> Original Algebra: >>>>> >>>>> (graph <http://graph> >>>>> >>>>> (bgp (triple ?s ?p ?o))) >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> Quad Form Algebra: >>>>> >>>>> (quadpattern (quad <http://graph> ?s ?p ?o)) >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> Original Algebra: >>>>> >>>>> (foo >>>>> >>>>> (graph <http://graph> >>>>> >>>>> (bgp (triple ?s ?p ?o))) >>>>> >>>>> ) >>>>> >>>>> >>>>> >>>>> Quad Form Algebra: >>>>> >>>>> (foo >>>>> >>>>> (quadpattern (quad <urn:x-arq:DefaultGraphNode> ?s ?p ?o)) >>>>> >>>>> ) >>>>> >>>>> >>>>> Also included with the gist. >>>>> >>>>> As can be seen in the above example output the transformation causes >>>>> the >>>>> graph field in the resulting quadpattern operator to be incorrectly >>>>> overwritten >>>>> >>>>> Under a debugger I can see that this is because when invoked via >>>>> Transformer.Transform() the quad form transform gets applied bottom >>>>>up >>>>> instead of top down so the bgp is visited prior to the graph and so >>>>>the >>>>> correct graph node is not honoured >>>>> >>>>> I'm assuming this is essentially a user error on my part because of >>>>>the >>>>> way >>>>> I am blindly using Transform.transform() to apply the transform >>>>>given. >>>>> Part >>>>> of the problem is that in every other case bottom up application is >>>>> going >>>>> to >>>>> be the desired behaviour. Is there an alternative way to apply the >>>>> transform or to detect when a transform wants to be top down? >>>>> >>>>> Any thoughts would be most appreciated, >>>>> >>>>> Cheers, >>>>> >>>>> Rob >>>>> >>>>> >>>>> >>>>> >>>> >>>> >>>> -- >>>> I like: Like Like - The likeliest place on the web >>>> <http://like-like.xenei.com> >>>> LinkedIn: http://www.linkedin.com/in/claudewarren >>> >>> >>> >>> >> >
