Repository: apache/jena (java)

Issue:
Incorrect JoinClassifier results with unbound values.
### Version

5.1.0-SNAPSHOT

### What happened?

The following two variants incorrectly return 1 binding whereas 0 would be expected.
The reason is that JoinClassifier.isLinear returns true because VarFinder does not consider the unbound values.

A linear join substitutes the (in-scope) variables on the right-hand-side (rhs) with each binding from the left-hand-side (lhs). This causes the incorrect substitution of ?x on the rhs with `0` such that `bound(?x)` yields true.

```sparql
SELECT * {
  VALUES ?x {
    0
  }
  {
    VALUES ?x {
      UNDEF
    }
    FILTER(bound(?x))
  }
}
```

```sparql
SELECT * {
  BIND(0 AS ?x)
  {
    BIND(coalesce() AS ?x)
    FILTER(bound(?x))
  }
}
```


### Relevant output and stacktrace

_No response_

### Are you interested in making a pull request?

Yes

Pull request:
GH-2412: Improved JoinClassifier for unbound values
GitHub issue resolved #2412

Pull request Description: Additional heuristics and test cases in JoinClassifier and VarFinder for dealing with unbound values.

----

 - [x] Tests are included.
 - [x] Commits have been squashed to remove intermediate development commit messages.
 - [x] Key commit messages start with the issue number (GH-xxxx)

By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the [Contributor's Agreement](https://www.apache.org/licenses/contributor-agreements.html).

----

See the [Apache Jena "Contributing" guide](https://github.com/apache/jena/blob/main/CONTRIBUTING.md).


Patch:
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/engine/main/JoinClassifier.java b/jena-arq/src/main/java/org/apache/jena/sparql/engine/main/JoinClassifier.java
index 2855857b43..3fd325bd87 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/engine/main/JoinClassifier.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/engine/main/JoinClassifier.java
@@ -70,16 +70,17 @@ public class JoinClassifier
     // -- pre check for ops we can't handle in a linear fashion.
     // These are the negation patterns (minus and diff)
     // FILTER NOT EXISTS is safe - it's defined by iteration like the linear execution algorithm.
-    private static class UnsafeLineraOpException extends RuntimeException {
+    private static class UnsafeLinearOpException extends RuntimeException {
         @Override public Throwable fillInStackTrace() { return this; }
     }
     private static OpVisitor checkForUnsafeVisitor = new OpVisitorBase() {
-        @Override public void visit(OpMinus opMinus) { throw new UnsafeLineraOpException(); }
-        @Override public void visit(OpDiff opDiff)   { throw new UnsafeLineraOpException(); }
+        @Override public void visit(OpMinus opMinus) { throw new UnsafeLinearOpException(); }
+        @Override public void visit(OpDiff opDiff)   { throw new UnsafeLinearOpException(); }
     };
+
     private static boolean isSafeForLinear(Op op) {
         try { OpWalker.walk(op, checkForUnsafeVisitor); return true; }
-        catch (UnsafeLineraOpException e) { return false; }
+        catch (UnsafeLinearOpException e) { return false; }
     }
     // --
 
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/engine/main/VarFinder.java b/jena-arq/src/main/java/org/apache/jena/sparql/engine/main/VarFinder.java
index 71b2403e68..882fa9b93d 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/engine/main/VarFinder.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/engine/main/VarFinder.java
@@ -25,18 +25,26 @@ import static org.apache.jena.sparql.util.VarUtils.addVarsFromTriple ;
 import static org.apache.jena.sparql.util.VarUtils.addVarsFromTriplePath ;
 
 import java.io.PrintStream ;
+import java.util.ArrayList;
 import java.util.HashSet ;
+import java.util.Iterator;
 import java.util.List ;
 import java.util.Set ;
 
+import org.apache.jena.atlas.iterator.Iter;
 import org.apache.jena.atlas.lib.SetUtils ;
 import org.apache.jena.sparql.algebra.Op ;
 import org.apache.jena.sparql.algebra.OpVisitor ;
+import org.apache.jena.sparql.algebra.Table ;
 import org.apache.jena.sparql.algebra.op.* ;
 import org.apache.jena.sparql.core.BasicPattern ;
 import org.apache.jena.sparql.core.Var ;
 import org.apache.jena.sparql.core.VarExprList ;
+import org.apache.jena.sparql.engine.binding.Binding ;
+import org.apache.jena.sparql.expr.E_Bound;
+import org.apache.jena.sparql.expr.E_Coalesce;
 import org.apache.jena.sparql.expr.Expr ;
+import org.apache.jena.sparql.expr.ExprFunction;
 import org.apache.jena.sparql.expr.ExprList ;
 import org.apache.jena.sparql.expr.ExprVars;
 import org.apache.jena.sparql.util.VarUtils ;
@@ -329,12 +337,45 @@ public class VarFinder
 
         private void processAssignVarExprList(VarExprList varExprList) {
             varExprList.forEachVarExpr((v,e)-> {
-                defines.add(v) ; // Expression may eval to error -> unset?
-                if ( e != null )
+                if ( e != null ) {
+                    boolean isLikelyToBeDefined = isLikelyToBeDefined(e);
+                    if (isLikelyToBeDefined) {
+                        defines.add(v) ; // Expression may eval to error -> unset?
+                    } else {
+                        optDefines.add(v);
+                    }
                     ExprVars.nonOpVarsMentioned(assignMentions, e);
+                }
             }) ;
         }
 
+        /**
+         * Heuristic to check whether the expression's evaluation result is likely
+         * to always be defined (i.e. never null / type error).
+         * Does not detect runtime type errors, such as when
+         * expr is (?x < 1) and ?x is assigned a string.
+         */
+        private boolean isLikelyToBeDefined(Expr expr) {
+            boolean result;
+            if (expr.isVariable()) {
+                result = defines.contains(expr.asVar());
+            } else if (expr.isConstant() || expr instanceof E_Bound) {
+                result = true;
+            } else if (expr.isFunction()) {
+                ExprFunction fn = expr.getFunction();
+                List<Expr> args = fn.getArgs();
+                if (fn instanceof E_Coalesce) {
+                    result = Iter.anyMatch(args.iterator(), this::isLikelyToBeDefined);
+                } else {
+                    result = Iter.allMatch(args.iterator(), this::isLikelyToBeDefined);
+                }
+            } else {
+                // Should never happen
+                result = false;
+            }
+            return result;
+        }
+
         @Override
         public void visit(OpProject opProject) {
             List<Var> vars = opProject.getVars();
@@ -351,9 +392,35 @@ public class VarFinder
             assignMentions.addAll(subUsage.assignMentions);
         }
 
+
         @Override
         public void visit(OpTable opTable) {
-            defines.addAll(opTable.getTable().getVars());
+            Table table = opTable.getTable();
+
+            // Start with a a copy of the table's declared variables.
+            // Then sieve out those variables which are unbound in any row.
+            Set<Var> remainingDefinedVars = new HashSet<>(table.getVars());
+
+            // The list of detected unbound variables.
+            // Tracked in a separate collection for clarity - could be added to optDefines directly.
+            List<Var> optVars = new ArrayList<>();
+
+            Iterator<Binding> rowIt = table.rows();
+            while (rowIt.hasNext()) {
+                Binding row = rowIt.next();
+
+                // Only the remaining defined variables need to be checked for unbound values
+                Iterator<Var> varIt = remainingDefinedVars.iterator();
+                while (varIt.hasNext()) {
+                    Var var = varIt.next();
+                    if (!row.contains(var)) {
+                        varIt.remove(); // Removes the current var from remainingDefinedVars
+                        optVars.add(var);
+                    }
+                }
+            }
+            defines.addAll(remainingDefinedVars);
+            optDefines.addAll(optVars);
         }
 
         @Override


Test patch:
diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/TS_Algebra.java b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/TS_Algebra.java
index 4e848bbd0b..d75e39ec17 100644
--- a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/TS_Algebra.java
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/TS_Algebra.java
@@ -33,6 +33,7 @@ import org.junit.runners.Suite ;
     , TestPattern2Join.class
     , TestTransformQuads.class
     , TestVarFinder.class
+    , TestVarFinder2.class
     , TestTransformOpExt.class
 })
 
diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/TestClassify.java b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/TestClassify.java
index fe55d00459..24010f89e3 100644
--- a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/TestClassify.java
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/TestClassify.java
@@ -33,56 +33,56 @@ import org.junit.Test ;
 public class TestClassify
 {
     @Test public void testClassify_Join_01()
-	{ classifyJ("{?s :p :o . { ?s :p :o FILTER(true) } }", true) ; }
+    { classifyJ("{?s :p :o . { ?s :p :o FILTER(true) } }", true) ; }
 
     @Test public void testClassify_Join_02()
-	{ classifyJ("{?s :p :o . { ?s :p :o FILTER(?s) } }", true) ; }
+    { classifyJ("{?s :p :o . { ?s :p :o FILTER(?s) } }", true) ; }
 
     @Test public void testClassify_Join_03()
-	{ classifyJ("{?s :p :o . { ?s :p ?o FILTER(?o) } }", true) ; }
+    { classifyJ("{?s :p :o . { ?s :p ?o FILTER(?o) } }", true) ; }
 
     // JENA-1167
     // This safe; ?o is not an input to the filter block.
     @Test public void testClassify_Join_04()
-	{ classifyJ("{?s :p :o . { ?s :p :o FILTER(?o) } }", true) ; }
+    { classifyJ("{?s :p :o . { ?s :p :o FILTER(?o) } }", true) ; }
 
     @Test public void testClassify_Join_05()
-	{ classifyJ("{?s :p :o . { ?x :p :o FILTER(?s) } }", false) ; }
+    { classifyJ("{?s :p :o . { ?x :p :o FILTER(?s) } }", false) ; }
 
     @Test public void testClassify_Join_06()
-	{ classifyJ("{ { ?s :p :o FILTER(true) } ?s :p :o }", true) ; }
+    { classifyJ("{ { ?s :p :o FILTER(true) } ?s :p :o }", true) ; }
 
-	@Test public void testClassify_Join_07()
-	{ classifyJ("{ { ?s :p :o FILTER(?s) }   ?s :p :o }", true) ; }
+    @Test public void testClassify_Join_07()
+    { classifyJ("{ { ?s :p :o FILTER(?s) }   ?s :p :o }", true) ; }
 
-	@Test public void testClassify_Join_08()
-	{ classifyJ("{ { ?s :p ?o FILTER(?o) }   ?s :p :o }", true) ; }
+    @Test public void testClassify_Join_08()
+    { classifyJ("{ { ?s :p ?o FILTER(?o) }   ?s :p :o }", true) ; }
 
-	@Test public void testClassify_Join_09()
-	{ classifyJ("{ { ?s :p :o FILTER(?o) }   ?s :p :o }", true) ; }
+    @Test public void testClassify_Join_09()
+    { classifyJ("{ { ?s :p :o FILTER(?o) }   ?s :p :o }", true) ; }
 
     // Actually, this is safe IF executed left, then streamed to right.
-	@Test public void testClassify_Join_10()
-	{ classifyJ("{ { ?x :p :o FILTER(?s) }   ?s :p :o }", true) ; }
+    @Test public void testClassify_Join_10()
+    { classifyJ("{ { ?x :p :o FILTER(?s) }   ?s :p :o }", true) ; }
 
 
     // OPTIONAL nested inside {} so it is a join of the LHS and the {}-RHS.
 
     // Not safe: ?s
     // Other parts of RHS may restrict ?s to things that can't match the LHS.
-	@Test public void testClassify_Join_11()
-	{ classifyJ("{?s :p :o . { OPTIONAL { ?s :p :o } } }", false) ; }
+    @Test public void testClassify_Join_11()
+    { classifyJ("{?s :p :o . { OPTIONAL { ?s :p :o } } }", false) ; }
 
     // Not safe: ?s
-	@Test public void testClassify_Join_12()
-	{ classifyJ("{?s :p :o . { OPTIONAL { ?s :p :o FILTER(?s) } } }", false) ; }
+    @Test public void testClassify_Join_12()
+    { classifyJ("{?s :p :o . { OPTIONAL { ?s :p :o FILTER(?s) } } }", false) ; }
 
-	@Test public void testClassify_Join_13()
-	{ classifyJ("{?s :p :o . { ?x :p :o OPTIONAL { :s :p :o FILTER(?x) } } }", true) ; }
+    @Test public void testClassify_Join_13()
+    { classifyJ("{?s :p :o . { ?x :p :o OPTIONAL { :s :p :o FILTER(?x) } } }", true) ; }
 
-	// See testClassify_Join_04()
-	@Test public void testClassify_Join_14()
-	{ classifyJ("{?s :p :o . { OPTIONAL { :s :p :o FILTER(?o) } } }", true) ; }
+    // See testClassify_Join_04()
+    @Test public void testClassify_Join_14()
+    { classifyJ("{?s :p :o . { OPTIONAL { :s :p :o FILTER(?o) } } }", true) ; }
 
     @Test public void testClassify_Join_14a()
     { classifyJ("{?s :p :o . { OPTIONAL { :s :p ?o FILTER(?o) } } }", true) ; }
@@ -91,7 +91,7 @@ public class TestClassify
     { classifyJ("{?s :p ?o . { OPTIONAL { :s :p :o FILTER(?o) } } }", false) ; }
 
     @Test public void testClassify_Join_15()
-	{ classifyJ("{?s :p :o . { OPTIONAL { ?x :p :o FILTER(?s) } } }", false) ; }
+    { classifyJ("{?s :p :o . { OPTIONAL { ?x :p :o FILTER(?s) } } }", false) ; }
 
     @Test public void testClassify_Join_20()
     { classifyJ("{ {?s :p ?x } . { {} OPTIONAL { :s :p ?x } } }", false) ; }
@@ -173,6 +173,42 @@ public class TestClassify
         TestClassify.classifyJ(x1, false);
     }
 
+    /** Can't linearize because rhs does not bind ?x */
+    @Test public void testClassify_Join_70a() {
+        String x1 = "{ VALUES ?x { UNDEF } VALUES ?x { UNDEF } }";
+        TestClassify.classifyJ(x1, false);
+    }
+
+    /** Can linearize because rhs binds ?x*/
+    @Test public void testClassify_Join_70b() {
+        String x1 = "{ VALUES ?x { UNDEF } VALUES ?x { 0 } }";
+        TestClassify.classifyJ(x1, true);
+    }
+
+    /** Can't linearize because rhs does not bind ?x */
+    @Test public void testClassify_Join_71() {
+        String x1 = "{ VALUES ?x { 0 } VALUES ?x { UNDEF } }";
+        TestClassify.classifyJ(x1, false);
+    }
+
+    /** Can't linearize because rhs does not bind ?x */
+    @Test public void testClassify_Join_80() {
+        String x1 = "{ BIND(0 AS ?x) { VALUES ?x { UNDEF } FILTER(bound(?x)) } }";
+        TestClassify.classifyJ(x1, false);
+    }
+
+    /** Can't linearize because rhs does not bind ?x */
+    @Test public void testClassify_Join_81() {
+        String x1 = "{ BIND(0 AS ?x) { BIND(coalesce() AS ?x) FILTER(bound(?x)) } }";
+        TestClassify.classifyJ(x1, false);
+    }
+
+    /** Can linearize because rhs binds ?x*/
+    @Test public void testClassify_Join_82() {
+        String x1 = "{ BIND('x' AS ?x) { BIND('y' AS ?x) FILTER(?x < 1) } }";
+        TestClassify.classifyJ(x1, true);
+    }
+
     public static void classifyJ(String pattern, boolean expected)
     {
         String qs1 = "PREFIX : <http://example/>\n" ;
diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/TestVarFinder2.java b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/TestVarFinder2.java
new file mode 100644
index 0000000000..3009819293
--- /dev/null
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/TestVarFinder2.java
@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.sparql.algebra;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+import java.util.Set ;
+
+import org.apache.jena.sparql.algebra.op.OpTable;
+import org.apache.jena.sparql.algebra.table.TableN;
+import org.apache.jena.sparql.core.Var ;
+import org.apache.jena.sparql.engine.binding.Binding;
+import org.apache.jena.sparql.engine.main.VarFinder ;
+import org.apache.jena.sparql.sse.SSE ;
+import org.junit.Test ;
+
+public class TestVarFinder2
+{
+    private static Binding row_xy = SSE.parseBinding("(row (?x 1) (?y 2))");
+    private static Binding row_x  = SSE.parseBinding("(row (?x 1))");
+    private static Binding row_y  = SSE.parseBinding("(row (?y 2))");
+    private static Binding row0  = SSE.parseBinding("(row)");
+    private static Var var_x  = Var.alloc("x");
+    private static Var var_y  = Var.alloc("y");
+    private static Var[] empty = {};
+
+    @Test public void varfind_table_1() {
+        OpTable opTable = createTable(row_xy);
+        VarFinder vf = VarFinder.process(opTable);
+        assertSetVars(vf.getFixed(), var_x, var_y);
+        assertSetVars(vf.getOpt(), empty);
+
+        assertDifferentVars(vf.getFixed(), var_x);
+        assertDifferentVars(vf.getFixed(), var_y);
+        assertDifferentVars(vf.getFixed(), empty);
+    }
+
+    @Test public void varfind_table_2() {
+        OpTable opTable = createTable(row_xy, row_x);
+        VarFinder vf = VarFinder.process(opTable);
+        assertSetVars(vf.getFixed(), var_x);
+        assertSetVars(vf.getOpt(), var_y);
+    }
+
+    @Test public void varfind_table_3() {
+        OpTable opTable = createTable(row_xy, row_x, row_y);
+        VarFinder vf = VarFinder.process(opTable);
+        assertSetVars(vf.getFixed(), empty);
+        assertSetVars(vf.getOpt(), var_x, var_y);
+    }
+
+    @Test public void varfind_table_4() {
+        OpTable opTable = createTable(row_x, row0);
+        VarFinder vf = VarFinder.process(opTable);
+        assertSetVars(vf.getFixed(), empty);
+        assertSetVars(vf.getOpt(), var_x);
+    }
+
+    /** Create a table of rows */
+    private static OpTable createTable(Binding... rows) {
+        Table table = new TableN();
+        for(Binding b : rows) {
+            table.addBinding(b);
+        }
+        return OpTable.create(table);
+    }
+
+    @Test public void varfind_extend_1() {
+        Op op = SSE.parseOp("(extend (?x 1) (table))");
+        VarFinder vf = VarFinder.process(op);
+        assertSetVars(vf.getFixed(), var_x);
+    }
+
+    @Test public void varfind_extend_2() {
+        Op op = SSE.parseOp("(extend (?x (coalesce 1 2)) (table))");
+        VarFinder vf = VarFinder.process(op);
+        assertSetVars(vf.getFixed(), var_x);
+    }
+
+    @Test public void varfind_extend_3() {
+        Op op = SSE.parseOp("(extend (?x (coalesce ?y 2)) (table))");
+        VarFinder vf = VarFinder.process(op);
+        assertSetVars(vf.getFixed(), var_x);
+    }
+
+    @Test public void varfind_extend_4() {
+        Op op = SSE.parseOp("(extend (?x (coalesce ?y)) (table))");
+        VarFinder vf = VarFinder.process(op);
+        assertSetVars(vf.getFixed(), empty);
+        assertSetVars(vf.getOpt(), var_x);
+    }
+
+    @Test public void varfind_extend_5() {
+        Op op = SSE.parseOp("""
+          (extend ((?x (coalesce ?y)))
+            (extend ((?y 1))
+              (table unit)
+           ))
+        """);
+        VarFinder vf = VarFinder.process(op);
+        assertSetVars(vf.getFixed(), var_x, var_y);
+    }
+
+    @Test public void varfind_extend_6() {
+        Op op = SSE.parseOp("""
+          (extend ((?x (coalesce ?z)))
+            (extend ((?y 1))
+              (table unit)
+           ))
+        """);
+        VarFinder vf = VarFinder.process(op);
+        assertSetVars(vf.getFixed(), var_y);
+        assertSetVars(vf.getOpt(), var_x);
+    }
+
+    /* Assert that a set contains exactly the given vars. */
+    private static void assertSetVars(Set<Var> vars, Var... expectedVars) {
+        Set<Var> expected = Set.of(expectedVars);
+        assertEquals(expected, vars);
+    }
+
+    /* Assert that a set differs from exactly the given vars. */
+    private static void assertDifferentVars(Set<Var> vars, Var... notExpectedVars) {
+        Set<Var> expected = Set.of(notExpectedVars);
+        assertNotEquals(expected, vars);
+    }
+}
diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/core/AbstractTestDynamicDataset.java b/jena-arq/src/test/java/org/apache/jena/sparql/core/AbstractTestDynamicDataset.java
index 152e8a7021..bc247c4ebd 100644
--- a/jena-arq/src/test/java/org/apache/jena/sparql/core/AbstractTestDynamicDataset.java
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/core/AbstractTestDynamicDataset.java
@@ -18,8 +18,6 @@
 
 package org.apache.jena.sparql.core;
 
-import static org.junit.Assert.assertEquals;
-
 import org.apache.jena.graph.Triple ;
 import org.apache.jena.query.* ;
 import org.apache.jena.rdf.model.Model ;
@@ -28,12 +26,14 @@ import org.junit.After ;
 import org.junit.Before ;
 import org.junit.Test ;
 
+import static org.apache.jena.sparql.core.AbstractTestQueryExec.testCount ;
+
 public abstract class AbstractTestDynamicDataset
 {
     protected abstract Dataset createDataset() ;
     protected abstract void releaseDataset(Dataset ds) ;
     protected Dataset  dataset ;
-    
+
     @Before public void before()
     {
         dataset = createDataset() ;
@@ -43,15 +43,15 @@ public abstract class AbstractTestDynamicDataset
         // Default model.
         Model m = dataset.getDefaultModel() ;
         Triple t1 = SSE.parseTriple("(<uri:x> <uri:p> 0)") ;
-        Triple t2 = SSE.parseTriple("(<uri:y> <uri:q> 'ABC')") ; 
-        Triple t3 = SSE.parseTriple("(<uri:z> <uri:property> 'DEF')") ; 
+        Triple t2 = SSE.parseTriple("(<uri:y> <uri:q> 'ABC')") ;
+        Triple t3 = SSE.parseTriple("(<uri:z> <uri:property> 'DEF')") ;
         m.getGraph().add(t1) ;
         m.getGraph().add(t2) ;
         m.getGraph().add(t3) ;
     }
-    
+
     @After public void after() {
-        releaseDataset(dataset) ; 
+        releaseDataset(dataset) ;
     }
 
     private static void addGraph(Dataset dataset, int i)
@@ -60,77 +60,77 @@ public abstract class AbstractTestDynamicDataset
         String x = "graph:"+i ;
         Model m = dataset.getNamedModel(x) ;
         Triple t1 = SSE.parseTriple("(<uri:x> <uri:p> "+i+")") ;
-        Triple t2 = SSE.parseTriple("(<uri:y> <uri:q> 'ABC')") ; 
+        Triple t2 = SSE.parseTriple("(<uri:y> <uri:q> 'ABC')") ;
         m.getGraph().add(t1) ;
         m.getGraph().add(t2) ;
     }
-    
+
     @Test public void dynamic01()    { testCount("SELECT * {?s ?p ?o}", 3, dataset) ; }
-    
+
     @Test public void dynamic02()    { testCount("SELECT ?g { GRAPH ?g {} }", 5, dataset) ; }
-    
+
     @Test public void dynamic03()    { testCount("SELECT * FROM <graph:1> {?s <uri:p> ?o}", 1, dataset) ; }
 
     @Test public void dynamic04()    { testCount("SELECT * FROM <graph:1> { GRAPH ?g { ?s ?p ?o} }", 0, dataset) ; }
-    
+
     @Test public void dynamic05()    { testCount("SELECT * FROM <graph:1> FROM <graph:2> {?s <uri:p> ?o}", 2, dataset) ; }
 
     // Duplicate surpression
     @Test public void dynamic06()    { testCount("SELECT ?s FROM <graph:1> FROM <graph:2> {?s <uri:q> ?o}", 1, dataset) ; }
-    
+
     @Test public void dynamic07()    { testCount("SELECT ?s FROM NAMED <graph:1> {?s <uri:q> ?o}", 0, dataset) ; }
-    
+
     @Test public void dynamic08()    { testCount("SELECT ?s FROM <graph:2> FROM NAMED <graph:1> {?s <uri:q> ?o}", 1, dataset) ; }
 
     @Test public void dynamic09()    { testCount("SELECT * "+
                                                 "FROM <graph:1> FROM <graph:2> "+
                                                 "FROM NAMED <graph:3> FROM NAMED <graph:4> "+
                                                 "{ GRAPH ?g { ?s <uri:q> ?o }}",
-                                                2, dataset) ; 
+                                                2, dataset) ;
                                     }
-    
+
     @Test public void dynamic10()    { testCount("SELECT * "+
                                                 "FROM <graph:1> FROM <graph:2>"+
                                                 "FROM NAMED <graph:3> FROM NAMED <graph:4> "+
                                                 "{ GRAPH ?g { ?s <uri:q> ?o }}",
-                                                2, dataset) ; 
+                                                2, dataset) ;
                                     }
 
     @Test public void dynamic11()    { testCount("SELECT * "+
                                                 "FROM <x:unknown>"+
                                                 "{ GRAPH ?g { ?s <uri:q> ?o }}",
-                                                0, dataset) ; 
+                                                0, dataset) ;
                                     }
 
     @Test public void dynamic12()    { testCount("SELECT * "+
                                                  "FROM  <graph:1>"+
                                                  "{ GRAPH ?g { }}",
-                                                 0, dataset) ; 
+                                                 0, dataset) ;
                                      }
 
     @Test public void dynamic13()    { testCount("SELECT * "+
                                                  "FROM NAMED <graph:1>"+
                                                  "{ GRAPH ?g { }}",
-                                                 1, dataset) ; 
+                                                 1, dataset) ;
                                      }
 
     @Test public void dynamic14()    { testCount("SELECT * "+
                                                  "FROM NAMED <graph:1> FROM NAMED <graph:2>"+
                                                  "FROM <graph:3> "+
                                                  "{ GRAPH ?g { }}",
-                                                 2, dataset) ; 
+                                                 2, dataset) ;
                                      }
 
     // -- Union graph.
-    
+
     // No FROM <union> the underlying dataset.
-    @Test public void dynamic_union_1() { 
-        testCount("SELECT * FROM <urn:x-arq:UnionGraph> { ?s <uri:p> ?o }", 5, dataset) ; 
-    } 
-    
+    @Test public void dynamic_union_1() {
+        testCount("SELECT * FROM <urn:x-arq:UnionGraph> { ?s <uri:p> ?o }", 5, dataset) ;
+    }
+
     // Should be able to see two graphs in the union.
     @Test public void dynamic_union_2() {
-        testCount("SELECT * FROM NAMED <graph:1> FROM NAMED <graph:2> FROM <graph:3>" + 
+        testCount("SELECT * FROM NAMED <graph:1> FROM NAMED <graph:2> FROM <graph:3>" +
                   "{ GRAPH <urn:x-arq:UnionGraph> { ?s <uri:p> ?o } }",
             2, dataset);
     }
@@ -138,7 +138,7 @@ public abstract class AbstractTestDynamicDataset
     @Test public void dynamic_union_3() {
         testCount("SELECT * FROM NAMED <urn:x-arq:UnionGraph> { GRAPH <urn:x-arq:UnionGraph> { } }", 1, dataset);
     }
-    
+
     // The union graph isn't in the named set, even if placed there explicitly.
     @Test
     public void dynamic_union_4() {
@@ -204,13 +204,4 @@ public abstract class AbstractTestDynamicDataset
                   "{ GRAPH <urn:x-arq:DefaultGraph> { ?s <uri:p> ?o . FILTER ( ?o IN ( 1, 2) ) } }",
                   2, dataset);
     }
-    
-    private static void testCount(String queryString, int expected, Dataset ds)
-    {
-        Query query = QueryFactory.create(queryString) ;
-        QueryExecution qExec = QueryExecutionFactory.create(query, ds) ;
-        ResultSet rs = qExec.execSelect() ;
-        int n = ResultSetFormatter.consume(rs) ;
-        assertEquals(expected, n) ;
-    }
 }
diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/core/AbstractTestQueryExec.java b/jena-arq/src/test/java/org/apache/jena/sparql/core/AbstractTestQueryExec.java
new file mode 100644
index 0000000000..751988d19c
--- /dev/null
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/core/AbstractTestQueryExec.java
@@ -0,0 +1,78 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.sparql.core;
+
+import static org.junit.Assert.assertEquals;
+
+import org.apache.jena.query.Dataset;
+import org.apache.jena.query.Query;
+import org.apache.jena.query.QueryExecution;
+import org.apache.jena.query.QueryExecutionFactory;
+import org.apache.jena.query.QueryFactory;
+import org.apache.jena.query.ResultSet;
+import org.apache.jena.query.ResultSetFormatter;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+/** Test cases that operate on the query execution level (whole query). */
+public abstract class AbstractTestQueryExec {
+    protected abstract Dataset createDataset() ;
+    protected abstract void releaseDataset(Dataset ds) ;
+
+    protected Dataset  dataset ;
+
+    @Before public void before()
+    {
+        dataset = createDataset() ;
+    }
+
+    @After public void after() {
+        releaseDataset(dataset) ;
+    }
+
+    /* join skew tests based on unbound values. See also: AbstractTestInnerJoin */
+
+    /** GH-2412 */
+    @Test
+    public void join_skew_01() {
+        testCount("SELECT * { VALUES ?x { 0 } { VALUES ?x { UNDEF } FILTER(bound(?x)) } }", 0, dataset);
+    }
+
+    /** GH-2412 */
+    @Test
+    public void join_skew_02() {
+        testCount("SELECT * { BIND(0 AS ?x) { BIND(coalesce() AS ?x) FILTER(bound(?x)) } }", 0, dataset);
+    }
+
+    /**
+     * Execute the given query on the given dataset and assert that the number of result rows
+     * matches the expected value.
+     *
+     * This method has package visibility because it is also called from {@link AbstractTestDynamicDataset}.
+     */
+    static void testCount(String queryString, int expected, Dataset ds)
+    {
+        Query query = QueryFactory.create(queryString) ;
+        QueryExecution qExec = QueryExecutionFactory.create(query, ds) ;
+        ResultSet rs = qExec.execSelect() ;
+        int n = ResultSetFormatter.consume(rs) ;
+        assertEquals(expected, n) ;
+    }
+}
diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/core/TS_Core.java b/jena-arq/src/test/java/org/apache/jena/sparql/core/TS_Core.java
index 9dbd198258..6a18490e72 100644
--- a/jena-arq/src/test/java/org/apache/jena/sparql/core/TS_Core.java
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/core/TS_Core.java
@@ -27,6 +27,7 @@ import org.junit.runners.Suite ;
     , TestDatasetGraphMem.class
     , TestDatasetGraphMemTriplesQuads.class
     , TestDatasetGeneral.class
+    , TestQueryExecMem.class
     , TestDynamicDatasetMem.class
     , TestDatasetGraphsRegular.class
     , TestDatasetGraphLink.class
diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/core/TestQueryExecMem.java b/jena-arq/src/test/java/org/apache/jena/sparql/core/TestQueryExecMem.java
new file mode 100644
index 0000000000..f1391290af
--- /dev/null
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/core/TestQueryExecMem.java
@@ -0,0 +1,59 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.sparql.core;
+
+import java.util.Arrays ;
+import java.util.Collection ;
+
+import org.apache.jena.atlas.lib.Creator ;
+import org.apache.jena.query.Dataset ;
+import org.apache.jena.query.DatasetFactory ;
+import org.junit.runner.RunWith ;
+import org.junit.runners.Parameterized ;
+import org.junit.runners.Parameterized.Parameters ;
+
+@RunWith(Parameterized.class)
+public class TestQueryExecMem extends AbstractTestQueryExec
+{
+    @Parameters(name = "{index}: {0}")
+    public static Collection<Object[]> data() {
+        Creator<Dataset> datasetGeneralMaker = ()-> DatasetFactory.createGeneral() ;
+        Creator<Dataset> datasetTxnMemMaker = ()-> DatasetFactory.createTxnMem() ;
+        return Arrays.asList(new Object[][] {
+            { "General",  datasetGeneralMaker },
+            { "TxnMem",   datasetTxnMemMaker} });
+    }
+
+    private final Creator<Dataset> maker;
+
+    public TestQueryExecMem(String name, Creator<Dataset> maker) {
+        this.maker = maker ;
+    }
+
+    @Override
+    protected Dataset createDataset()
+    {
+        return maker.create() ;
+    }
+
+    @Override
+    protected void releaseDataset(Dataset ds) {}
+
+}
+
diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/exec/TS_ExecSPARQL.java b/jena-arq/src/test/java/org/apache/jena/sparql/exec/TS_ExecSPARQL.java
index 8ccf2418e1..6ae60d2e42 100644
--- a/jena-arq/src/test/java/org/apache/jena/sparql/exec/TS_ExecSPARQL.java
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/exec/TS_ExecSPARQL.java
@@ -26,7 +26,7 @@ import org.junit.runners.Suite;
     TestExecEnvironment.class
     , TestQueryExecDataset.class
 } )
-
+ 
 public class TS_ExecSPARQL {
 
 }
diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/exec/TestQueryExecDataset.java b/jena-arq/src/test/java/org/apache/jena/sparql/exec/TestQueryExecDataset.java
index 8e7f6d5365..dd0bce2e1b 100644
--- a/jena-arq/src/test/java/org/apache/jena/sparql/exec/TestQueryExecDataset.java
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/exec/TestQueryExecDataset.java
@@ -32,6 +32,7 @@ import org.junit.Test;
 
 public class TestQueryExecDataset {
     // Most QueryExec testing gets tested via QueryExecution usage
+    // See also AbstractTestQueryExec
 
     @Test public void queryExec_substitution_01() {
         QueryExec queryExec = QueryExec.dataset(DatasetGraphFactory.empty())
diff --git a/jena-tdb1/src/test/java/org/apache/jena/tdb1/store/TS_Store.java b/jena-tdb1/src/test/java/org/apache/jena/tdb1/store/TS_Store.java
index 652a909f89..ced984c335 100644
--- a/jena-tdb1/src/test/java/org/apache/jena/tdb1/store/TS_Store.java
+++ b/jena-tdb1/src/test/java/org/apache/jena/tdb1/store/TS_Store.java
@@ -39,6 +39,7 @@ import org.junit.runners.Suite ;
     , TestLoader.class
     , Test_SPARQL_TDB.class
     , TestConcurrentAccess.class
+    , TestQueryExecTDB.class
     , TestDynamicDatasetTDB.class
     , TestStoreConnectionsDirect.class
     , TestStoreConnectionsMapped.class
diff --git a/jena-tdb1/src/test/java/org/apache/jena/tdb1/store/TestQueryExecTDB.java b/jena-tdb1/src/test/java/org/apache/jena/tdb1/store/TestQueryExecTDB.java
new file mode 100644
index 0000000000..45abbf86d3
--- /dev/null
+++ b/jena-tdb1/src/test/java/org/apache/jena/tdb1/store/TestQueryExecTDB.java
@@ -0,0 +1,39 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.tdb1.store;
+
+import org.apache.jena.query.Dataset ;
+import org.apache.jena.sparql.core.AbstractTestQueryExec;
+import org.apache.jena.tdb1.TDB1Factory;
+import org.apache.jena.tdb1.sys.TDBInternal;
+
+public class TestQueryExecTDB extends AbstractTestQueryExec
+{
+    @Override
+    protected Dataset createDataset()
+    {
+        return TDB1Factory.createDataset() ;
+    }
+
+    @Override
+    protected void releaseDataset(Dataset ds) {
+        TDBInternal.expel(ds.asDatasetGraph());
+    }
+}
+
diff --git a/jena-tdb2/src/test/java/org/apache/jena/tdb2/store/TS_Store.java b/jena-tdb2/src/test/java/org/apache/jena/tdb2/store/TS_Store.java
index dce1b44568..2ca65c8acb 100644
--- a/jena-tdb2/src/test/java/org/apache/jena/tdb2/store/TS_Store.java
+++ b/jena-tdb2/src/test/java/org/apache/jena/tdb2/store/TS_Store.java
@@ -42,6 +42,7 @@ import org.junit.runners.Suite;
     , TestDatasetTDB.class
     , TestDatasetTDBPersist.class
     , Test_SPARQL_TDB.class
+    , TestQueryExecTDB.class
     , TestDynamicDatasetTDB.class
     , TestStoreConnectionMem.class
     , TestStoreConnectionDirect.class
diff --git a/jena-tdb2/src/test/java/org/apache/jena/tdb2/store/TestQueryExecTDB.java b/jena-tdb2/src/test/java/org/apache/jena/tdb2/store/TestQueryExecTDB.java
new file mode 100644
index 0000000000..a3dffb0e49
--- /dev/null
+++ b/jena-tdb2/src/test/java/org/apache/jena/tdb2/store/TestQueryExecTDB.java
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.tdb2.store;
+
+import org.apache.jena.query.Dataset;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.sparql.core.AbstractTestQueryExec;
+import org.apache.jena.tdb2.junit.TL;
+
+public class TestQueryExecTDB extends AbstractTestQueryExec
+{
+    @Override
+    protected Dataset createDataset()
+    {
+        Dataset ds = TL.createTestDatasetMem();
+        ds.begin(ReadWrite.WRITE);
+        return ds;
+    }
+
+    @Override
+    protected void releaseDataset(Dataset ds) { ds.abort(); TL.expel(ds); }
+}
+