You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@jena.apache.org by GitBox <gi...@apache.org> on 2022/11/15 13:25:45 UTC

[GitHub] [jena] SimonBin commented on a diff in pull request #1616: improve jena performance and behaviour in certain path cases

SimonBin commented on code in PR #1616:
URL: https://github.com/apache/jena/pull/1616#discussion_r1022784175


##########
jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformPathFlattern.java:
##########
@@ -53,9 +58,60 @@ public TransformPathFlattern(PathCompiler pathCompiler)
     @Override
     public Op transform(OpPath opPath)
     {
-        // Flatten down to triples where possible.
-        PathBlock pattern = pathCompiler.reduce(opPath.getTriplePath()) ;
-        // Any generated paths of exactly one to triple; convert to Op.
-        return PathLib.pathToTriples(pattern) ;
+        Op op = transformReduce(opPath, pathCompiler);
+        return op;
     }
-}
+
+    static Op transformReduce(Op op, PathCompiler pathCompiler) {
+        return Transformer.transform(
+                new TransformCopy() {
+                    @Override
+                    public Op transform(OpPath opPath) {
+                        // Flatten down to triples where possible.
+                        PathBlock pattern = pathCompiler.reduce(opPath.getTriplePath());
+                        // Any generated paths of exactly one to triple; convert to Op.
+                        Op op = PathLib.pathToTriples(pattern);
+                        return transformAlt(op, pathCompiler);
+                    }
+                },
+                op);
+    }
+
+    static Op transformAlt(Op op, PathCompiler pathCompiler) {
+        return Transformer.transform(new AltTransformer(pathCompiler), op);
+    }
+
+    private static class AltTransformer extends TransformCopy {
+        private PathCompiler pathCompiler;
+
+        public AltTransformer(PathCompiler pathCompiler) {
+            this.pathCompiler = pathCompiler;
+        }
+
+        @Override
+        public Op transform(OpPath opPath) {
+            TriplePath triplePath = opPath.getTriplePath();
+            Path path = triplePath.getPath();
+            if (path instanceof P_Alt) {
+                Node s = triplePath.getSubject();

Review Comment:
   Thank you for the thorough analysis and concern. In my testing, I got the expected results when querying e.g. ` { ?s ex:foo _:bnode . _:bnode (ex:a | ex:b) ?o }` as you suggested. Do you have any additional pointers how to verify the safety of the transform?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org