apacheGH-2924: Lateral - fixed injection for tables, disabled scope c…

…hecks on tables, enhanced QueryExec API for easier testing.

jena-arq/src/main/java/org/apache/jena/query/ResultSet.java

@@ -103,4 +103,8 @@ public default ResultSet materialise() {
     }
 
     public void close();
+
+    default RowSet toRowSet() {
+        return RowSet.adapt(this);
+    }
 }

jena-arq/src/main/java/org/apache/jena/sparql/algebra/Algebra.java

@@ -161,22 +161,28 @@ public static Binding merge(Binding bindingLeft, Binding bindingRight) {
 
         // If compatible, merge. Iterate over variables in right but not in left.
         BindingBuilder b = Binding.builder(bindingLeft);
-        for ( Iterator<Var> vIter = bindingRight.vars() ; vIter.hasNext() ; ) {
-            Var v = vIter.next();
-            Node n = bindingRight.get(v);
+        bindingRight.forEach((v, n) -> {
             if ( !bindingLeft.contains(v) )
                 b.add(v, n);
-        }
+        });
         return b.build();
     }
 
     public static boolean compatible(Binding bindingLeft, Binding bindingRight) {
         // Test to see if compatible: Iterate over variables in left
-        for ( Iterator<Var> vIter = bindingLeft.vars() ; vIter.hasNext() ; ) {
-            Var v = vIter.next();
+        return compatible(bindingLeft, bindingRight, bindingLeft.vars());
+    }
+
+    /** Test to see if bindings are compatible for all variables of the provided iterator. */
+    public static boolean compatible(Binding bindingLeft, Binding bindingRight, Iterator<Var> vars) {
+        while (vars.hasNext() ) {
+            Var v = vars.next();
             Node nLeft = bindingLeft.get(v);
-            Node nRight = bindingRight.get(v);
+            if ( nLeft == null ) {
+                continue;
+            }
 
+            Node nRight = bindingRight.get(v);
             if ( nRight != null && !nRight.equals(nLeft) )
                 return false;
         }

jena-arq/src/main/java/org/apache/jena/sparql/algebra/TableFactory.java

@@ -18,40 +18,65 @@
 
 package org.apache.jena.sparql.algebra;
 
+import java.util.Iterator;
 import java.util.List ;
 
 import org.apache.jena.graph.Node ;
+import org.apache.jena.query.ResultSet;
 import org.apache.jena.sparql.algebra.table.Table1 ;
 import org.apache.jena.sparql.algebra.table.TableEmpty ;
 import org.apache.jena.sparql.algebra.table.TableN ;
 import org.apache.jena.sparql.algebra.table.TableUnit ;
 import org.apache.jena.sparql.core.Var ;
 import org.apache.jena.sparql.engine.QueryIterator ;
+import org.apache.jena.sparql.engine.binding.Binding;
+import org.apache.jena.sparql.exec.RowSet;
 
 public class TableFactory
 {
     public static Table createUnit()
     { return new TableUnit() ; }
-    
+
     public static Table createEmpty()
     { return new TableEmpty() ; }
 
     public static Table create()
     { return new TableN() ; }
-    
+
     public static Table create(List<Var> vars)
     { return new TableN(vars) ; }
-    
+
     public static Table create(QueryIterator queryIterator)
-    { 
+    {
         if ( queryIterator.isJoinIdentity() ) {
             queryIterator.close();
             return createUnit() ;
         }
-        
+
         return new TableN(queryIterator) ;
     }
 
     public static Table create(Var var, Node value)
     { return new Table1(var, value) ; }
+
+    public static Table create(List<Var> vars, Iterable<Binding> bindings)
+    { return create(vars, bindings.iterator()); }
+
+    public static Table create(List<Var> vars, Iterator<Binding> bindings) {
+        Table result = create(vars);
+        bindings.forEachRemaining(result::addBinding);
+        return result;
+    }
+
+    public static Table create(RowSet rs)
+    {
+        List<Var> vars = rs.getResultVars();
+        return create(vars, rs);
+    }
+
+    public static Table create(ResultSet rs)
+    {
+        RowSet rowSet = RowSet.adapt(rs);
+        return create(rowSet);
+    }
 }

jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterLateral.java

@@ -19,6 +19,8 @@
 package org.apache.jena.sparql.engine.iterator;
 
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
@@ -27,6 +29,7 @@
 import org.apache.jena.atlas.lib.SetUtils;
 import org.apache.jena.graph.Node;
 import org.apache.jena.graph.Triple;
+import org.apache.jena.sparql.algebra.Algebra;
 import org.apache.jena.sparql.algebra.Op;
 import org.apache.jena.sparql.algebra.Table;
 import org.apache.jena.sparql.algebra.TransformCopy;
@@ -292,15 +295,31 @@ public Op transform(OpTable opTable) {
             // By the assignment restriction, the binding only needs to be added to each row of the table.
             Table table = opTable.getTable();
             // Table vars.
-            List<Var> vars = new ArrayList<>(table.getVars());
-            binding.vars().forEachRemaining(vars::add);
+            List<Var> tableVars = table.getVars();
+            List<Var> vars = new ArrayList<>(tableVars);
+
+            // Track variables that appear both in the table and the binding.
+            List<Var> commonVars = new ArrayList<>();
+
+            // Index variables in a set if there are more than a few of them.
+            Collection<Var> tableVarsIndex = tableVars.size() > 4 ? new HashSet<>(tableVars) : tableVars;
+            binding.vars().forEachRemaining(v -> {
+                if (tableVarsIndex.contains(v)) {
+                    commonVars.add(v);
+                } else {
+                    vars.add(v);
+                }
+            });
+
             TableN table2 = new TableN(vars);
             BindingBuilder builder = BindingFactory.builder();
             table.iterator(null).forEachRemaining(row->{
-                builder.reset();
-                builder.addAll(row);
-                builder.addAll(binding);
-                table2.addBinding(builder.build());
+                if (Algebra.compatible(row, binding, commonVars.iterator())) {
+                    builder.reset();
+                    builder.addAll(row);
+                    binding.forEach(builder::set);
+                    table2.addBinding(builder.build());
+                }
             });
             return OpTable.create(table2);
         }

jena-arq/src/main/java/org/apache/jena/sparql/exec/QueryExec.java

@@ -24,6 +24,7 @@
 import org.apache.jena.atlas.json.JsonObject;
 import org.apache.jena.graph.Graph;
 import org.apache.jena.graph.Triple;
+import org.apache.jena.query.DatasetFactory;
 import org.apache.jena.query.Query;
 import org.apache.jena.query.QueryExecution;
 import org.apache.jena.sparql.core.DatasetGraph;
@@ -54,6 +55,12 @@ public static QueryExecBuilder graph(Graph graph) {
         return QueryExecDatasetBuilder.create().graph(graph);
     }
 
+    /** Create a {@link QueryExecBuilder} for an empty dataset. */
+    public static QueryExecBuilder emptyDataset() {
+        DatasetGraph empty = DatasetGraphFactory.create();
+        return dataset(empty);
+    }
+
     /** Create a {@link QueryExecBuilder} for a remote endpoint. */
     public static QueryExecBuilder service(String serviceURL) {
         return QueryExecHTTPBuilder.create().endpoint(serviceURL);

jena-arq/src/main/java/org/apache/jena/sparql/exec/QueryExecBuilder.java

@@ -25,6 +25,9 @@
 import org.apache.jena.query.ARQ;
 import org.apache.jena.query.Query;
 import org.apache.jena.query.Syntax;
+import org.apache.jena.riot.rowset.RowSetOnClose;
+import org.apache.jena.sparql.algebra.Table;
+import org.apache.jena.sparql.algebra.TableFactory;
 import org.apache.jena.sparql.core.Var;
 import org.apache.jena.sparql.engine.binding.Binding;
 import org.apache.jena.sparql.util.Context;
@@ -81,9 +84,16 @@ public default QueryExecBuilder substitution(String var, Node value) {
 
     // build-and-use short cuts
 
-    /** Build and execute as a SELECT query. */
+    /**
+     * Build and execute as a SELECT query.
+     * The caller must eventually close the returned RowSet
+     * in order to free any associated resources.
+     * Use {@link #table()} to obtain an independent in-memory copy of the result set.
+     */
     public default RowSet select() {
-        return build().select();
+        QueryExec qExec = build();
+        RowSet core = qExec.select();
+        return new RowSetOnClose(core, qExec::close);
     }
 
     /** Build and execute as a CONSTRUCT query. */
@@ -106,4 +116,18 @@ public default boolean ask() {
             return qExec.ask();
         }
     }
+
+    /**
+     * Build and execute as an SELECT query.
+     * Returns an in-memory table of the fully consumed underlying result set.
+     * Use {@link Table#toRowSet()} to obtain a rowSet.
+     */
+    public default Table table() {
+        Table result;
+        try (QueryExec qExec = build()) {
+            RowSet rowSet = qExec.select();
+            result = TableFactory.create(rowSet);
+        }
+        return result;
+    }
 }

jena-arq/src/main/java/org/apache/jena/sparql/exec/RowSet.java

@@ -93,4 +93,8 @@ public default Stream<Binding> stream() {
     public static RowSet create(QueryIterator qIter, List<Var> vars) {
         return RowSetStream.create(vars, qIter);
     }
+
+    default ResultSet toResultSet() {
+        return ResultSet.adapt(this);
+    }
 }

jena-arq/src/main/java/org/apache/jena/sparql/lang/SyntaxVarScope.java

@@ -294,18 +294,23 @@ private void checkLATERAL(Collection<Var> accScope, Element el) {
             ElementVisitor checker = new ElementVisitorBase() {
                 @Override
                 public void visit(ElementBind eltBind) {
+                    /*
                     if ( accScope.contains(eltBind.getVar()) )
                         throw new QueryParseException("BIND: Variable " + eltBind.getVar() + " defined", -1, -1);
+                    */
                 }
                 // @Override public void visit(ElementAssign eltAssign) {} -- LET -
                 // always OK
 
                 @Override
                 public void visit(ElementData eltData) {
+                    // Bindings of the table will be filtered against in-scope variables
+                    /*
                     eltData.getVars().forEach(v -> {
                         if ( accScope.contains(v) )
                             throw new QueryParseException("VALUES: Variable " + v + " defined", -1, -1);
                     });
+                    */
                 }
 
                 @Override

jena-arq/src/test/java/org/apache/jena/sparql/exec/TestQueryExecution.java

@@ -18,15 +18,13 @@
 
 package org.apache.jena.sparql.exec;
 
+
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+import org.apache.jena.sparql.algebra.Table;
+import org.apache.jena.sparql.sse.SSE;
 import org.junit.Test;
 
-import org.apache.jena.graph.Node;
-import org.apache.jena.sparql.core.DatasetGraph;
-import org.apache.jena.sparql.core.DatasetGraphFactory;
-import org.apache.jena.sparql.engine.binding.Binding;
-
 /** Miscellaneous tests, e.g. from reports. */
 public class TestQueryExecution {
     @Test public void lateral_with_join() {
@@ -40,11 +38,60 @@ public class TestQueryExecution {
                 }
             }
             """;
-        DatasetGraph dsg = DatasetGraphFactory.empty();
-        RowSet rowSet = QueryExec.dataset(dsg).query(qsReport).select();
-        Binding row = rowSet.next();
-        row.contains("xOut");
-        Node x = row.get("xOut");
-        assertEquals("x", x.getLiteralLexicalForm());
+
+        Table expected = SSE.parseTable("(table (row (?xIn 'x') (?x 1) (?xOut 'x' ) ) )");
+        Table actual = QueryExec.emptyDataset().query(qsReport).table();
+        assertEquals(expected, actual);
+    }
+
+    @Test public void lateral_with_nesting() {
+        // Should NOT cause a runtime error in QueryIterLateral due to duplicate variables in bindings
+        String qsReport = """
+            SELECT * {
+                BIND(1 AS ?s)
+                LATERAL {
+                    BIND(?s AS ?x)
+                    LATERAL {
+                        BIND(?s AS ?y)
+                    }
+                }
+            }
+            """;
+
+        Table expected = SSE.parseTable("(table (row (?s 1) (?x 1) (?y 1) ) )");
+        Table actual = QueryExec.emptyDataset().query(qsReport).table();
+        assertEquals(expected, actual);
+    }
+
+    // TODO Perhaps should fail due to scoping?
+    @Test public void lateral_with_bind() {
+        String qsReport = """
+            SELECT * {
+                BIND(1 AS ?x)
+                LATERAL {
+                    BIND (1 AS ?x)
+                }
+            }
+            """;
+
+        Table expected = SSE.parseTable("(table (row (?x 1) ) )");
+        Table actual = QueryExec.emptyDataset().query(qsReport).table();
+        assertEquals(expected, actual);
+    }
+
+    // TODO Perhaps should fail due to scoping?
+    @Test public void lateral_with_table() {
+        String qsReport = """
+            SELECT * {
+                BIND(1 AS ?x)
+                LATERAL {
+                    VALUES ?x { 1 2 }
+                }
+            }
+            """;
+
+        Table expected = SSE.parseTable("(table (row (?x 1) ) )");
+        Table actual = QueryExec.emptyDataset().query(qsReport).table();
+        assertEquals(expected, actual);
     }
 }