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






Reply via email to