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
