You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jena.apache.org by an...@apache.org on 2022/02/12 12:39:19 UTC

[jena] branch main updated: JENA-2280: Map results back to scoped variables

This is an automated email from the ASF dual-hosted git repository.

andy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/jena.git


The following commit(s) were added to refs/heads/main by this push:
     new 7f477ef  JENA-2280: Map results back to scoped variables
     new df8590a  Merge pull request #1195 from afs/jena2280-service
7f477ef is described below

commit 7f477ef4c64bcc4bdcfa4ee012f23c9cda07f3d4
Author: Andy Seaborne <an...@apache.org>
AuthorDate: Fri Feb 11 12:43:09 2022 +0000

    JENA-2280: Map results back to scoped variables
---
 .../java/org/apache/jena/sparql/engine/Rename.java | 73 +++++++++++++++-------
 .../org/apache/jena/sparql/exec/http/Service.java  | 63 ++++++++++++++-----
 .../apache/jena/sparql/sse/writers/WriterOp.java   |  1 +
 .../apache/jena/sparql/exec/http/TestService.java  | 17 +++++
 4 files changed, 114 insertions(+), 40 deletions(-)

diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/engine/Rename.java b/jena-arq/src/main/java/org/apache/jena/sparql/engine/Rename.java
index 413bdc7..daad7e4 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/engine/Rename.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/engine/Rename.java
@@ -93,10 +93,53 @@ public class Rename {
      * op was renamed by VarRename.rename
      */
     public static Op reverseVarRename(Op op, boolean repeatedly) {
-        NodeTransform renamer = new UnrenameAnyVars(prefix, repeatedly);
-        return NodeTransformLib.transform(renamer, op);
+        NodeTransform x = n->Rename.reverseVarRename(n, prefix, repeatedly);
+        //NodeTransform renamer = new UnrenameAnyVars(prefix, repeatedly);
+        return NodeTransformLib.transform(x, op);
+    }
+
+    // ---- Node reverseRename.
+
+    /**
+     * Undo the effect of the rename operation, once or repeatedly. This assumes the
+     * node was renamed by VarRename.rename
+     */
+    public static Node reverseVarRename(Node node) {
+        //return new UnrenameAnyVars(prefix, true).apply(node);
+        return reverseVarRename(node, prefix, true);
+    }
+
+    private static Node reverseVarRename(Node node, String varPrefix, boolean repeatedly) {
+        if ( node.isNodeTriple() ) {
+            Triple t1 = node.getTriple();
+            Triple t2 = NodeTransformLib.transform(n->Rename.reverseVarRename(n, varPrefix, repeatedly), t1);
+            return Objects.equals(t1, t2) ? node : NodeFactory.createTripleNode(t2);
+        }
+
+        if ( !Var.isVar(node) )
+            return node;
+
+        Var var = (Var)node;
+        return reverseVarRename(var, varPrefix, repeatedly);
     }
 
+    private static Node reverseVarRename(Var var, String varPrefix, boolean repeatedly) {
+        String varName = var.getName();
+        if ( repeatedly ) {
+            while (varName.startsWith(varPrefix))
+                varName = varName.substring(varPrefix.length());
+        } else {
+            if ( varName.startsWith(varPrefix) )
+                varName = varName.substring(varPrefix.length());
+        }
+
+        if ( varName == var.getName() )
+            return var;
+        return Var.alloc(varName);
+
+    }
+
+
     // ---- Transforms that do the renaming and unrenaming.
 
     static class RenameNode implements NodeTransform {
@@ -116,6 +159,7 @@ public class Rename {
         }
     }
 
+    // This transform is stateful - it keeps aliases.
     static class RenameAnyVars implements NodeTransform {
         private final Map<Var, Var> aliases = new HashMap<>();
         private final Collection<Var> constants;
@@ -154,6 +198,8 @@ public class Rename {
 
     /**
      * Reverse a renaming (assuming renaming was done by prefixing variable names)
+     * This does not need to track allocations.
+     * Retained for symmetry.
      */
     static class UnrenameAnyVars implements NodeTransform {
         private final String varPrefix;
@@ -166,28 +212,7 @@ public class Rename {
 
         @Override
         public Node apply(Node node) {
-            if ( node.isNodeTriple() ) {
-                Triple t1 = node.getTriple();
-                Triple t2 = NodeTransformLib.transform(this, t1);
-                return Objects.equals(t1, t2) ? node : NodeFactory.createTripleNode(t2);
-            } else if ( !Var.isVar(node) ) {
-                return node;
-            }
-
-            Var var = (Var)node;
-            String varName = var.getName();
-
-            if ( repeatedly ) {
-                while (varName.startsWith(varPrefix))
-                    varName = varName.substring(varPrefix.length());
-            } else {
-                if ( varName.startsWith(varPrefix) )
-                    varName = varName.substring(varPrefix.length());
-            }
-
-            if ( varName == var.getName() )
-                return node;
-            return Var.alloc(varName);
+            return reverseVarRename(node, varPrefix, repeatedly);
         }
     }
 }
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/exec/http/Service.java b/jena-arq/src/main/java/org/apache/jena/sparql/exec/http/Service.java
index 90c5ddc..2b56fcb 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/exec/http/Service.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/exec/http/Service.java
@@ -19,8 +19,10 @@
 package org.apache.jena.sparql.exec.http;
 
 import java.net.http.HttpClient;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.jena.atlas.logging.Log;
@@ -34,11 +36,14 @@ import org.apache.jena.query.QueryFactory;
 import org.apache.jena.sparql.SystemARQ ;
 import org.apache.jena.sparql.algebra.Op ;
 import org.apache.jena.sparql.algebra.OpAsQuery ;
+import org.apache.jena.sparql.algebra.OpVars;
 import org.apache.jena.sparql.algebra.op.OpService ;
+import org.apache.jena.sparql.core.Var;
 import org.apache.jena.sparql.engine.QueryIterator ;
 import org.apache.jena.sparql.engine.Rename;
 import org.apache.jena.sparql.engine.http.HttpParams;
 import org.apache.jena.sparql.engine.http.QueryExceptionHTTP;
+import org.apache.jena.sparql.engine.iterator.QueryIter;
 import org.apache.jena.sparql.engine.iterator.QueryIterPlainWrapper;
 import org.apache.jena.sparql.exec.RowSet;
 import org.apache.jena.sparql.syntax.Element;
@@ -125,22 +130,46 @@ public class Service {
                 query.setQueryPattern(el);
                 query.setResultVars();
             }
-        } else {
-            // This relies on the observation that the query was originally correct,
-            // so reversing the scope renaming is safe (it merely restores the
-            // algebra expression).
-            //
-            // Any variables that reappear should be internal ones that were hidden
-            // by renaming in the first place.
-            //
-            // Any substitution is also safe because it replaces variables by
-            // values.
-            //
-            // It is safer to rename/unrename than skipping SERVICE during rename
-            // to avoid substituting hidden variables.
-
-            Op opRestored = Rename.reverseVarRename(opRemote, true);
-            query = OpAsQuery.asQuery(opRestored);
+        }
+
+        // This relies on the observation that the query was originally correct,
+        // so reversing the scope renaming is safe (it merely restores the
+        // algebra expression).
+        //
+        // Any variables that reappear should be internal ones that were hidden
+        // by renaming in the first place.
+        //
+        // Any substitution is also safe because it replaces variables by
+        // values.
+        //
+        // It is safer to rename/unrename than skipping SERVICE during rename
+        // to avoid substituting hidden variables.
+
+        Op opRestored = Rename.reverseVarRename(opRemote, true);
+        query = OpAsQuery.asQuery(opRestored);
+        // Transforming: Same object means "no change"
+        boolean requiresRemapping = false;
+        Map<Var, Var> varMapping = null;
+        if ( ! opRestored.equals(opRemote) ) {
+            varMapping = new HashMap<>();
+            Set<Var> originalVars = OpVars.visibleVars(op);
+            Set<Var> remoteVars = OpVars.visibleVars(opRestored);
+
+            for (Var v : originalVars) {
+                if (v.getName().contains("/")) {
+                    // A variable which was scope renamed so has a different name
+                    String origName = v.getName().substring(v.getName().lastIndexOf('/') + 1);
+                    Var remoteVar = Var.alloc(origName);
+                    if (remoteVars.contains(remoteVar)) {
+                        varMapping.put(remoteVar, v);
+                        requiresRemapping = true;
+                    }
+                } else {
+                    // A variable which does not have a different name
+                    if (remoteVars.contains(v))
+                        varMapping.put(v, v);
+                }
+            }
         }
 
         // -- Setup
@@ -170,6 +199,8 @@ public class Service {
             // Detach from the network stream.
             RowSet rowSet = qExec.select().materialize();
             QueryIterator qIter = QueryIterPlainWrapper.create(rowSet);
+            if (requiresRemapping)
+                qIter = QueryIter.map(qIter, varMapping);
             return qIter;
         } catch (HttpException ex) {
             throw QueryExceptionHTTP.rewrap(ex);
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/sse/writers/WriterOp.java b/jena-arq/src/main/java/org/apache/jena/sparql/sse/writers/WriterOp.java
index c83004c..00874c4 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/sse/writers/WriterOp.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/sse/writers/WriterOp.java
@@ -336,6 +336,7 @@ public class WriterOp
             if ( opService.getSilent() )
                 out.println("silent ");
             WriterNode.output(out, opService.getService(), sContext);
+            out.println();
             opService.getSubOp().visit(this);
             finish(opService);
         }
diff --git a/jena-integration-tests/src/test/java/org/apache/jena/sparql/exec/http/TestService.java b/jena-integration-tests/src/test/java/org/apache/jena/sparql/exec/http/TestService.java
index 34e2f06..edb0ea6 100644
--- a/jena-integration-tests/src/test/java/org/apache/jena/sparql/exec/http/TestService.java
+++ b/jena-integration-tests/src/test/java/org/apache/jena/sparql/exec/http/TestService.java
@@ -27,6 +27,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.jena.atlas.iterator.Iter;
+import org.apache.jena.atlas.lib.StrUtils;
 import org.apache.jena.atlas.logging.LogCtl;
 import org.apache.jena.fuseki.Fuseki;
 import org.apache.jena.graph.Node ;
@@ -444,6 +445,22 @@ public class TestService {
         QueryExec.dataset(localDataset()).query(queryString).ask();
     }
 
+    // JENA-2280
+    // ?value is scped as ?/value and this needs dealing with in SERVCE results.
+    @Test public void service_scope_service_1() {
+
+        String queryString = StrUtils.strjoinNL
+                ("SELECT ?temp {"
+                ,"  SELECT (?value as ?temp) {"
+                ,"    SERVICE <http://localhost:3030/test/query> { VALUES ?value { 'test' }  }"
+                ,"  }"
+                ,"}");
+        RowSet rs = QueryExec.dataset(localDataset()).query(queryString).select().materialize();
+        Binding row = rs.next();
+        assertTrue(row.contains("temp"));
+    }
+
+
     private static void runWithModifier(String key, HttpRequestModifier modifier, Runnable action) {
         RegistryRequestModifier.get().add(SERVICE, modifier);
         try {