Conditions on left joins.  Tests for same.

Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/ab8f32ab
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/ab8f32ab
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/ab8f32ab

Branch: refs/heads/master
Commit: ab8f32ab10fc8a4da43c97fcff6a83e175257232
Parents: c3953a7
Author: Andy Seaborne <[email protected]>
Authored: Wed Sep 9 22:55:09 2015 +0100
Committer: Andy Seaborne <[email protected]>
Committed: Wed Sep 9 22:55:09 2015 +0100

----------------------------------------------------------------------
 .../apache/jena/sparql/engine/join/Join.java    | 18 ++----
 .../join/QueryIterNestedLoopLeftJoin.java       |  2 +-
 .../jena/sparql/engine/ref/TableJoin.java       |  3 -
 .../sparql/engine/join/AbstractTestJoin.java    | 64 ++++++++++++--------
 .../engine/join/AbstractTestLeftJoin.java       | 25 +++++++-
 .../sparql/engine/join/TestLeftJoinSimple.java  |  1 -
 6 files changed, 69 insertions(+), 44 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/jena/blob/ab8f32ab/jena-arq/src/main/java/org/apache/jena/sparql/engine/join/Join.java
----------------------------------------------------------------------
diff --git 
a/jena-arq/src/main/java/org/apache/jena/sparql/engine/join/Join.java 
b/jena-arq/src/main/java/org/apache/jena/sparql/engine/join/Join.java
index 6820c43..ed25af6 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/engine/join/Join.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/engine/join/Join.java
@@ -28,10 +28,8 @@ import org.apache.jena.sparql.algebra.TableFactory ;
 import org.apache.jena.sparql.engine.ExecutionContext ;
 import org.apache.jena.sparql.engine.QueryIterator ;
 import org.apache.jena.sparql.engine.binding.Binding ;
-import org.apache.jena.sparql.engine.iterator.QueryIterFilterExpr ;
 import org.apache.jena.sparql.engine.iterator.QueryIterPlainWrapper ;
 import org.apache.jena.sparql.engine.main.OpExecutor ;
-import org.apache.jena.sparql.expr.Expr ;
 import org.apache.jena.sparql.expr.ExprList ;
 
 /** API to various join algorithms */
@@ -159,7 +157,7 @@ public class Join {
      *  Builds output early. Materializes right, streams left.
      *  Does <b>not</b> scale. 
      */
-    public static QueryIterator nestedLoopLeftJoinBasic(QueryIterator left, 
QueryIterator right, ExprList condition, ExecutionContext execCxt) {
+    public static QueryIterator nestedLoopLeftJoinBasic(QueryIterator left, 
QueryIterator right, ExprList conditions, ExecutionContext execCxt) {
         // Stream from left, materialize right.
         List<Binding> rightRows = Iter.toList(right) ;
         List<Binding> output = DS.list() ;
@@ -169,7 +167,7 @@ public class Join {
             boolean match = false ;
             for ( Binding row2 : rightRows ) {
                 Binding r = Algebra.merge(row1, row2) ;
-                if ( r != null ) {
+                if ( r != null && applyConditions(r, conditions, execCxt)) {
                     output.add(r) ;
                     match = true ;
                 }
@@ -177,18 +175,14 @@ public class Join {
             if ( ! match )
                 output.add(row1) ;
         }
-        QueryIterator qIter = new QueryIterPlainWrapper(output.iterator(), 
execCxt) ;
-        qIter = applyConditions(qIter, condition, execCxt) ;
-        return qIter ;
+        return new QueryIterPlainWrapper(output.iterator(), execCxt) ;
     }
 
     // apply conditions.
-    private static QueryIterator applyConditions(QueryIterator qIter, ExprList 
conditions, ExecutionContext execCxt) {
+    private static boolean applyConditions(Binding row, ExprList conditions, 
ExecutionContext execCxt) {
         if ( conditions == null )
-            return qIter ;
-        for (Expr expr : conditions)
-            qIter = new QueryIterFilterExpr(qIter, expr, execCxt) ;
-        return qIter ;
+            return true ;
+        return conditions.isSatisfied(row, execCxt) ;
     }
 
     private static QueryIterator debug(QueryIterator left, QueryIterator 
right, ExecutionContext execCxt, JoinOp action) {

http://git-wip-us.apache.org/repos/asf/jena/blob/ab8f32ab/jena-arq/src/main/java/org/apache/jena/sparql/engine/join/QueryIterNestedLoopLeftJoin.java
----------------------------------------------------------------------
diff --git 
a/jena-arq/src/main/java/org/apache/jena/sparql/engine/join/QueryIterNestedLoopLeftJoin.java
 
b/jena-arq/src/main/java/org/apache/jena/sparql/engine/join/QueryIterNestedLoopLeftJoin.java
index 338f6e9..220eb78 100644
--- 
a/jena-arq/src/main/java/org/apache/jena/sparql/engine/join/QueryIterNestedLoopLeftJoin.java
+++ 
b/jena-arq/src/main/java/org/apache/jena/sparql/engine/join/QueryIterNestedLoopLeftJoin.java
@@ -109,7 +109,7 @@ public class QueryIterNestedLoopLeftJoin extends QueryIter2 
{
             if ( ! foundMatch ) {
                 s_countResults++;
                 Binding r = rowLeft ;
-                rowLeft = null;    
+                rowLeft = null; 
                 return r ;
             }
             rowLeft = null;

http://git-wip-us.apache.org/repos/asf/jena/blob/ab8f32ab/jena-arq/src/main/java/org/apache/jena/sparql/engine/ref/TableJoin.java
----------------------------------------------------------------------
diff --git 
a/jena-arq/src/main/java/org/apache/jena/sparql/engine/ref/TableJoin.java 
b/jena-arq/src/main/java/org/apache/jena/sparql/engine/ref/TableJoin.java
index acdb5cc..629ede6 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/engine/ref/TableJoin.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/engine/ref/TableJoin.java
@@ -69,8 +69,6 @@ public class TableJoin
     }
             
     private static QueryIterator joinWorkerN(QueryIterator left, Table right, 
JoinType joinType, ExprList conditions, ExecutionContext execCxt) {       
-        // We could hash the right except we don't know much about columns.
-        
         List<Binding> out = new ArrayList<>() ;
         for ( ; left.hasNext() ; ) {
             Binding bindingLeft = left.next() ;
@@ -92,7 +90,6 @@ public class TableJoin
                 // Conditions on left?
                 out.add(bindingLeft) ;
         }
-        
         return new QueryIterPlainWrapper(out.iterator(), execCxt) ;
     }
     

http://git-wip-us.apache.org/repos/asf/jena/blob/ab8f32ab/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/AbstractTestJoin.java
----------------------------------------------------------------------
diff --git 
a/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/AbstractTestJoin.java
 
b/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/AbstractTestJoin.java
index dd16393..d27255f 100644
--- 
a/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/AbstractTestJoin.java
+++ 
b/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/AbstractTestJoin.java
@@ -20,6 +20,7 @@ package org.apache.jena.sparql.engine.join;
 
 import java.util.List ;
 
+import org.apache.jena.atlas.io.IndentedWriter ;
 import org.apache.jena.atlas.iterator.Iter ;
 import org.apache.jena.atlas.lib.StrUtils ;
 import org.apache.jena.query.ResultSet ;
@@ -245,7 +246,7 @@ public abstract class AbstractTestJoin extends Assert {
         testJoin(var, left, right, null, tableOut); 
     }
     
-    protected void testJoin(String var, Table left, Table right, ExprList 
conditions, Table tableOut) {
+    protected void testJoin(String var, Table left, Table right, String 
conditions, Table tableOut) {
         JoinKey joinKey ;
         if ( var != null ) {
             if ( var.startsWith("?") )
@@ -258,8 +259,11 @@ public abstract class AbstractTestJoin extends Assert {
             // (and related algorithms).
             joinKey = null ;
         }
-
-        executeTest(joinKey, left, right, null, tableOut) ;
+        
+        ExprList exprs = null ;
+        if ( conditions != null )
+            exprs = SSE.parseExprList(conditions) ;
+        executeTest(joinKey, left, right, exprs, tableOut) ;
     }
 
     protected void testJoinWithKey(JoinKey joinKey, Table left, Table right, 
Table tableOut) {
@@ -280,41 +284,49 @@ public abstract class AbstractTestJoin extends Assert {
     protected void executeTestJoin(String msg, JoinKey joinKey, Table left, 
Table right, ExprList conditions, Table expectedResults) {
         Table x1 = joinMaterialize(joinKey, left, right, conditions) ;
         assertNotNull("Null table from join ("+msg+")", x1) ;
-        if ( false ) {
-            System.out.println("Test :    "+msg) ;
-            System.out.println("Joinkey:  "+joinKey) ;
-            System.out.println("Left:     \n"+left) ;
-            System.out.println("Right:    \n"+right) ;
-            System.out.println("Expected: \n"+expectedResults) ;
-            System.out.println("Actual:   \n"+x1) ;
-            System.out.println() ;
-        }
-        
-        check("Results not equal ("+msg+")", joinKey, left, right, 
expectedResults, x1) ;
+        if ( false )
+            print(msg, joinKey, left, right, conditions, expectedResults, x1) ;
+        check("Results not equal ("+msg+")", joinKey, left, right, conditions, 
expectedResults, x1) ;
     }
 
     private Table joinMaterialize(JoinKey joinKey, Table left, Table right, 
ExprList conditions) {
-        QueryIterator qIter = join(joinKey, left , right, null) ;
+        QueryIterator qIter = join(joinKey, left , right, conditions) ;
         return TableFactory.create(qIter) ;
     }
 
     public abstract QueryIterator join(JoinKey joinKey, Table left , Table 
right, ExprList conditions) ;
 
-    private static void check(String msg, JoinKey joinKey, Table left, Table 
right, Table expected, Table actual) {
+    private static void check(String msg, JoinKey joinKey, Table left, Table 
right, ExprList conditions, Table expected, Table actual) {
         boolean b = equalTables(expected, actual) ;
-        if ( ! b ) {
-            System.out.flush() ;
-            System.err.println("Joinkey:  "+joinKey) ;
-            System.err.println("Left:     \n"+left) ;
-            System.err.println("Right:    \n"+right) ;
-            System.err.println("Expected: \n"+expected) ;
-            System.err.println("Actual:   \n"+actual) ;
-            System.err.println() ;
-        }
-
+        if ( ! b ) 
+            print(msg, joinKey, left, right, conditions, expected, actual); 
         assertTrue(msg, b) ;
     }
 
+    protected static void print(String msg, JoinKey joinKey, Table left, Table 
right, ExprList conditions, Table expected, Table actual) {
+        System.err.flush() ;
+        System.out.flush() ;
+        IndentedWriter out = IndentedWriter.stderr ;
+        out.println("Test :    "+msg) ;
+        out.println("Joinkey:  "+joinKey) ;
+        
+        print(out, "Left:", left) ;
+        print(out, "Right:", right) ;
+        if ( conditions != null )
+            out.println("Conditions: "+conditions) ;    
+        print(out, "Expected:", expected) ;
+        print(out, "Actual:", actual) ;
+        out.println() ;
+        out.flush() ;
+    }
+    
+    protected static void print(IndentedWriter out, String label, Table table) 
{
+        out.println(label) ;
+        out.incIndent();
+        out.println(table.toString()) ;
+        out.decIndent();
+    }
+    
     private static boolean equalTables(Table table1, Table table2) {
         ResultSet rs1 =  ResultSetFactory.create(table1.iterator(null), 
table1.getVarNames()) ;
         ResultSet rs2 =  ResultSetFactory.create(table2.iterator(null), 
table2.getVarNames()) ;

http://git-wip-us.apache.org/repos/asf/jena/blob/ab8f32ab/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/AbstractTestLeftJoin.java
----------------------------------------------------------------------
diff --git 
a/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/AbstractTestLeftJoin.java
 
b/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/AbstractTestLeftJoin.java
index afca7db..e37564c 100644
--- 
a/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/AbstractTestLeftJoin.java
+++ 
b/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/AbstractTestLeftJoin.java
@@ -20,6 +20,7 @@ package org.apache.jena.sparql.engine.join;
 
 import org.apache.jena.sparql.algebra.Table ;
 import org.apache.jena.sparql.expr.ExprList ;
+import org.apache.jena.sparql.sse.SSE ;
 import org.junit.Test ;
 
 public abstract class AbstractTestLeftJoin extends AbstractTestJoin {
@@ -68,11 +69,33 @@ public abstract class AbstractTestLeftJoin extends 
AbstractTestJoin {
     // No key.
     @Test public void leftjoin_14()         { testJoin(null, tableD1(), 
tableD2(), tableD3()) ; }
 
-
     // Disjoint tables.
     @Test public void leftjoin_disjoint_01() { testJoin("a", tableD2(), 
tableD8(), tableD8x2()) ; }
     @Test public void leftjoin_disjoint_02() { testJoin("z", tableD2(), 
tableD8(), tableD8x2()) ; }
+    
+    // Conditions.
+    @Test public void leftjoin_condition_01() {
+        Table tableD1c = parseTableInt("(table", 
+                                       "   (row (?a 1) (?b 3))",
+                                       ")") ;
+        testJoin("a", table1(), tableD1(), "((= ?b 3))", tableD1c) ; 
+    }
+    
 
+    @Test public void leftjoin_condition_02() {
+        Table tableD3_LJc = parseTableInt("(table",
+                                       "   (row (?d 8) (?a 0))",
+                                       "   (row (?a 1) (?c 9) (?b 2))",
+                                       "   (row (?a 1) (?c 9) (?b 2))",
+                                       ")") ;
+        testJoin("a", tableD2(), tableD1(), "((= ?a 1) (= ?b 2))", 
tableD3_LJc) ; 
+    }
+    
+    @Test public void leftjoin_condition_03() {
+        // Never match 
+        ExprList exprs = SSE.parseExprList("((= ?b 99))") ; 
+        testJoin("a", table1(), tableD1(), "((= ?b 99))", table1()) ; 
+    }
 }
 
 

http://git-wip-us.apache.org/repos/asf/jena/blob/ab8f32ab/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/TestLeftJoinSimple.java
----------------------------------------------------------------------
diff --git 
a/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/TestLeftJoinSimple.java
 
b/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/TestLeftJoinSimple.java
index f9fc763..05d4031 100644
--- 
a/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/TestLeftJoinSimple.java
+++ 
b/jena-arq/src/test/java/org/apache/jena/sparql/engine/join/TestLeftJoinSimple.java
@@ -20,7 +20,6 @@ package org.apache.jena.sparql.engine.join;
 
 import org.apache.jena.sparql.algebra.Table ;
 import org.apache.jena.sparql.engine.QueryIterator ;
-import org.apache.jena.sparql.engine.join.JoinKey ;
 import org.apache.jena.sparql.engine.ref.TableJoin ;
 import org.apache.jena.sparql.expr.ExprList ;
 

Reply via email to