You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jena.apache.org by rv...@apache.org on 2013/05/30 01:30:44 UTC

svn commit: r1487680 - in /jena/Experimental/jena-jdbc/jena-jdbc-core/src: main/java/org/apache/jena/jdbc/results/MaterializedResults.java test/java/org/apache/jena/jdbc/results/AbstractResultSetTests.java

Author: rvesse
Date: Wed May 29 23:30:44 2013
New Revision: 1487680

URL: http://svn.apache.org/r1487680
Log:
Fix another logic bug in relative() method for scrollable result sets

Modified:
    jena/Experimental/jena-jdbc/jena-jdbc-core/src/main/java/org/apache/jena/jdbc/results/MaterializedResults.java
    jena/Experimental/jena-jdbc/jena-jdbc-core/src/test/java/org/apache/jena/jdbc/results/AbstractResultSetTests.java

Modified: jena/Experimental/jena-jdbc/jena-jdbc-core/src/main/java/org/apache/jena/jdbc/results/MaterializedResults.java
URL: http://svn.apache.org/viewvc/jena/Experimental/jena-jdbc/jena-jdbc-core/src/main/java/org/apache/jena/jdbc/results/MaterializedResults.java?rev=1487680&r1=1487679&r2=1487680&view=diff
==============================================================================
--- jena/Experimental/jena-jdbc/jena-jdbc-core/src/main/java/org/apache/jena/jdbc/results/MaterializedResults.java (original)
+++ jena/Experimental/jena-jdbc/jena-jdbc-core/src/main/java/org/apache/jena/jdbc/results/MaterializedResults.java Wed May 29 23:30:44 2013
@@ -26,8 +26,9 @@ import org.apache.jena.jdbc.statements.J
 import com.hp.hpl.jena.query.QueryExecution;
 
 /**
- * Represents a set of materialized results backed by some {@link QueryExecution},
- * materialized results permit scrolling but are not sensitive to changes in the underlying data
+ * Represents a set of materialized results backed by some
+ * {@link QueryExecution}, materialized results permit scrolling but are not
+ * sensitive to changes in the underlying data
  * 
  * @param <T>
  *            Type of the underlying result rows
@@ -182,9 +183,10 @@ public abstract class MaterializedResult
     public final void beforeFirst() throws SQLException {
         if (this.isClosed())
             throw new SQLException("Result Set is closed");
-        
-        // Are we after the last row?  If so reset current row or movePrevious() may break
-        if (this.isAfterLast()) 
+
+        // Are we after the last row? If so reset current row or movePrevious()
+        // may break
+        if (this.isAfterLast())
             this.currRow = this.getTotalRows();
 
         // Move to start of results if necessary
@@ -216,9 +218,10 @@ public abstract class MaterializedResult
         // Are we already at the first row?
         if (this.currRow == 1)
             return true;
-        
-        // Are we after the last row?  If so reset current row or movePrevious() may break
-        if (this.isAfterLast()) 
+
+        // Are we after the last row? If so reset current row or movePrevious()
+        // may break
+        if (this.isAfterLast())
             this.currRow = this.getTotalRows();
 
         // Before first row?
@@ -299,7 +302,7 @@ public abstract class MaterializedResult
         // If no rows this should always return false
         if (this.getTotalRows() == 0)
             return false;
-        
+
         // Are we already at the last row?
         if (this.currRow == this.getTotalRows())
             return true;
@@ -307,7 +310,7 @@ public abstract class MaterializedResult
         // Are we after the last row?
         if (this.isAfterLast()) {
             this.currRow = this.getTotalRows();
-            
+
             // Move backwards to last row
             if (this.hasPrevious()) {
                 this.currItem = this.movePrevious();
@@ -371,6 +374,12 @@ public abstract class MaterializedResult
         } else if (rows < 0) {
             // Calculate destination row and use absolute() to move there
             int destRow = this.currRow + rows;
+
+            // Need to sanitize resulting values below zero as otherwise
+            // absolute will move us to a row relative to end of results when
+            // actually this means we should be moving to before the first row
+            if (destRow < 0)
+                destRow = 0;
             return this.absolute(destRow);
         } else if (this.currRow + rows > this.getTotalRows()) {
             // Would result in moving beyond the end of the results

Modified: jena/Experimental/jena-jdbc/jena-jdbc-core/src/test/java/org/apache/jena/jdbc/results/AbstractResultSetTests.java
URL: http://svn.apache.org/viewvc/jena/Experimental/jena-jdbc/jena-jdbc-core/src/test/java/org/apache/jena/jdbc/results/AbstractResultSetTests.java?rev=1487680&r1=1487679&r2=1487680&view=diff
==============================================================================
--- jena/Experimental/jena-jdbc/jena-jdbc-core/src/test/java/org/apache/jena/jdbc/results/AbstractResultSetTests.java (original)
+++ jena/Experimental/jena-jdbc/jena-jdbc-core/src/test/java/org/apache/jena/jdbc/results/AbstractResultSetTests.java Wed May 29 23:30:44 2013
@@ -856,14 +856,15 @@ public abstract class AbstractResultSetT
         Assert.assertFalse(rset.isClosed());
         Assert.assertTrue(rset.isBeforeFirst());
         Assert.assertFalse(rset.isLast());
-        
+
         // Expect exactly one row we can move to
         Assert.assertTrue(rset.next());
         Assert.assertFalse(rset.isAfterLast());
         Assert.assertTrue(rset.isLast());
         Assert.assertFalse(rset.next());
 
-        // Attempting to move backwards in a forwards only result set should result in an error
+        // Attempting to move backwards in a forwards only result set should
+        // result in an error
         try {
             rset.beforeFirst();
             Assert.fail("Should not be permitted to move backwards in a FORWARD_ONLY result set");
@@ -876,7 +877,7 @@ public abstract class AbstractResultSetT
             Assert.assertTrue(rset.isClosed());
         }
     }
-    
+
     /**
      * Tests movement through SELECT results
      * 
@@ -901,7 +902,7 @@ public abstract class AbstractResultSetT
         rset.close();
         Assert.assertTrue(rset.isClosed());
     }
-    
+
     /**
      * Tests movement through SELECT results
      * 
@@ -923,7 +924,7 @@ public abstract class AbstractResultSetT
         rset.close();
         Assert.assertTrue(rset.isClosed());
     }
-    
+
     /**
      * Tests movement through SELECT results
      * 
@@ -937,21 +938,22 @@ public abstract class AbstractResultSetT
         Assert.assertFalse(rset.isClosed());
         Assert.assertTrue(rset.isBeforeFirst());
         Assert.assertFalse(rset.isLast());
-        
+
         // Expect exactly one row we can move to
         Assert.assertTrue(rset.next());
         Assert.assertFalse(rset.isAfterLast());
         Assert.assertTrue(rset.isLast());
         Assert.assertFalse(rset.next());
 
-        // Attempting to move backwards in a scrollable result set should result set should be fine
+        // Attempting to move backwards in a scrollable result set should result
+        // set should be fine
         rset.beforeFirst();
         Assert.assertTrue(rset.isBeforeFirst());
 
         rset.close();
         Assert.assertTrue(rset.isClosed());
     }
-    
+
     /**
      * Tests movement through SELECT results
      * 
@@ -965,12 +967,12 @@ public abstract class AbstractResultSetT
         Assert.assertFalse(rset.isClosed());
         Assert.assertTrue(rset.isBeforeFirst());
         Assert.assertFalse(rset.isLast());
-        
+
         // Can move forwards to first row
         Assert.assertTrue(rset.next());
         Assert.assertTrue(rset.isFirst());
         Assert.assertFalse(rset.isLast());
-        
+
         // Then can move forwards to second row
         Assert.assertTrue(rset.next());
         Assert.assertTrue(rset.isLast());
@@ -982,7 +984,7 @@ public abstract class AbstractResultSetT
         rset.close();
         Assert.assertTrue(rset.isClosed());
     }
-    
+
     /**
      * Tests movement through SELECT results
      * 
@@ -996,7 +998,7 @@ public abstract class AbstractResultSetT
         Assert.assertFalse(rset.isClosed());
         Assert.assertTrue(rset.isBeforeFirst());
         Assert.assertFalse(rset.isLast());
-        
+
         // Can move to various absolute rows
         Assert.assertTrue(rset.absolute(1));
         Assert.assertTrue(rset.absolute(1));
@@ -1009,31 +1011,32 @@ public abstract class AbstractResultSetT
         Assert.assertTrue(rset.absolute(-3));
         Assert.assertTrue(rset.absolute(-4));
         Assert.assertTrue(rset.absolute(-5));
-        
+
         // 0 is treated as moving to before first
         Assert.assertFalse(rset.absolute(0));
         Assert.assertTrue(rset.isBeforeFirst());
-        
+
         // 1 is treated as moving to first row
         Assert.assertTrue(rset.absolute(1));
         Assert.assertTrue(rset.isFirst());
-        
+
         // -1 is treated as moving to last row
         Assert.assertTrue(rset.absolute(-1));
         Assert.assertTrue(rset.isLast());
-        
+
         // Moving to a row beyond end positions us after last and returns false
         Assert.assertFalse(rset.absolute(6));
         Assert.assertTrue(rset.isAfterLast());
-        
-        // Moving to a row before start positions us before first and returns false
+
+        // Moving to a row before start positions us before first and returns
+        // false
         Assert.assertFalse(rset.absolute(-6));
         Assert.assertTrue(rset.isBeforeFirst());
-        
+
         rset.close();
         Assert.assertTrue(rset.isClosed());
     }
-    
+
     /**
      * Tests movement through SELECT results
      * 
@@ -1047,11 +1050,11 @@ public abstract class AbstractResultSetT
         Assert.assertFalse(rset.isClosed());
         Assert.assertTrue(rset.isBeforeFirst());
         Assert.assertFalse(rset.isLast());
-        
+
         // Can move forwards to after last row
         rset.afterLast();
         Assert.assertTrue(rset.isAfterLast());
-        
+
         // Can move backwards to last row
         Assert.assertTrue(rset.last());
         Assert.assertTrue(rset.isLast());
@@ -1059,7 +1062,34 @@ public abstract class AbstractResultSetT
         rset.close();
         Assert.assertTrue(rset.isClosed());
     }
-    
+
+    /**
+     * Tests movement through SELECT results
+     * 
+     * @throws SQLException
+     */
+    @Test
+    public void test_results_select_movement_09() throws SQLException {
+        ResultSet rset = this.createResults(ds, "SELECT * { ?s ?p ?o . } LIMIT 2", ResultSet.TYPE_SCROLL_INSENSITIVE);
+        Assert.assertEquals(ResultSet.TYPE_SCROLL_INSENSITIVE, rset.getType());
+        Assert.assertNotNull(rset);
+        Assert.assertFalse(rset.isClosed());
+        Assert.assertTrue(rset.isBeforeFirst());
+        Assert.assertFalse(rset.isLast());
+
+        // Can move forwards to after last row
+        rset.afterLast();
+        Assert.assertTrue(rset.isAfterLast());
+
+        // Moving backwards more rows than possible with relative movement
+        // should place us before first row
+        Assert.assertFalse(rset.relative(-4));
+        Assert.assertTrue(rset.isBeforeFirst());
+
+        rset.close();
+        Assert.assertTrue(rset.isClosed());
+    }
+
     /**
      * Tests movement through CONSTRUCT results
      * 
@@ -1098,14 +1128,15 @@ public abstract class AbstractResultSetT
         Assert.assertFalse(rset.isClosed());
         Assert.assertTrue(rset.isBeforeFirst());
         Assert.assertFalse(rset.isLast());
-        
+
         // Expect exactly one row we can move to
         Assert.assertTrue(rset.next());
         Assert.assertFalse(rset.isAfterLast());
         Assert.assertTrue(rset.isLast());
         Assert.assertFalse(rset.next());
 
-        // Attempting to move backwards in a forwards only result set should result in an error
+        // Attempting to move backwards in a forwards only result set should
+        // result in an error
         try {
             rset.beforeFirst();
             Assert.fail("Should not be permitted to move backwards in a FORWARD_ONLY result set");
@@ -1118,7 +1149,7 @@ public abstract class AbstractResultSetT
             Assert.assertTrue(rset.isClosed());
         }
     }
-    
+
     /**
      * Tests movement through CONSTRUCT results
      * 
@@ -1143,7 +1174,7 @@ public abstract class AbstractResultSetT
         rset.close();
         Assert.assertTrue(rset.isClosed());
     }
-    
+
     /**
      * Tests movement through CONSTRUCT results
      * 
@@ -1165,7 +1196,7 @@ public abstract class AbstractResultSetT
         rset.close();
         Assert.assertTrue(rset.isClosed());
     }
-    
+
     /**
      * Tests movement through CONSTRUCT results
      * 
@@ -1179,21 +1210,22 @@ public abstract class AbstractResultSetT
         Assert.assertFalse(rset.isClosed());
         Assert.assertTrue(rset.isBeforeFirst());
         Assert.assertFalse(rset.isLast());
-        
+
         // Expect exactly one row we can move to
         Assert.assertTrue(rset.next());
         Assert.assertFalse(rset.isAfterLast());
         Assert.assertTrue(rset.isLast());
         Assert.assertFalse(rset.next());
 
-        // Attempting to move backwards in a scrollable result set should result set should be fine
+        // Attempting to move backwards in a scrollable result set should result
+        // set should be fine
         rset.beforeFirst();
         Assert.assertTrue(rset.isBeforeFirst());
 
         rset.close();
         Assert.assertTrue(rset.isClosed());
     }
-    
+
     /**
      * Tests movement through CONSTRUCT results
      * 
@@ -1207,12 +1239,12 @@ public abstract class AbstractResultSetT
         Assert.assertFalse(rset.isClosed());
         Assert.assertTrue(rset.isBeforeFirst());
         Assert.assertFalse(rset.isLast());
-        
+
         // Can move forwards to first row
         Assert.assertTrue(rset.next());
         Assert.assertTrue(rset.isFirst());
         Assert.assertFalse(rset.isLast());
-        
+
         // Then can move forwards to second row
         Assert.assertTrue(rset.next());
         Assert.assertTrue(rset.isLast());
@@ -1224,7 +1256,7 @@ public abstract class AbstractResultSetT
         rset.close();
         Assert.assertTrue(rset.isClosed());
     }
-    
+
     /**
      * Tests movement through CONSTRUCT results
      * 
@@ -1238,7 +1270,7 @@ public abstract class AbstractResultSetT
         Assert.assertFalse(rset.isClosed());
         Assert.assertTrue(rset.isBeforeFirst());
         Assert.assertFalse(rset.isLast());
-        
+
         // Can move to various absolute rows
         Assert.assertTrue(rset.absolute(1));
         Assert.assertTrue(rset.absolute(1));
@@ -1251,31 +1283,32 @@ public abstract class AbstractResultSetT
         Assert.assertTrue(rset.absolute(-3));
         Assert.assertTrue(rset.absolute(-4));
         Assert.assertTrue(rset.absolute(-5));
-        
+
         // 0 is treated as moving to before first
         Assert.assertFalse(rset.absolute(0));
         Assert.assertTrue(rset.isBeforeFirst());
-        
+
         // 1 is treated as moving to first row
         Assert.assertTrue(rset.absolute(1));
         Assert.assertTrue(rset.isFirst());
-        
+
         // -1 is treated as moving to last row
         Assert.assertTrue(rset.absolute(-1));
         Assert.assertTrue(rset.isLast());
-        
+
         // Moving to a row beyond end positions us after last and returns false
         Assert.assertFalse(rset.absolute(6));
         Assert.assertTrue(rset.isAfterLast());
-        
-        // Moving to a row before start positions us before first and returns false
+
+        // Moving to a row before start positions us before first and returns
+        // false
         Assert.assertFalse(rset.absolute(-6));
         Assert.assertTrue(rset.isBeforeFirst());
-        
+
         rset.close();
         Assert.assertTrue(rset.isClosed());
     }
-    
+
     /**
      * Tests movement through CONSTRUCT results
      * 
@@ -1289,11 +1322,11 @@ public abstract class AbstractResultSetT
         Assert.assertFalse(rset.isClosed());
         Assert.assertTrue(rset.isBeforeFirst());
         Assert.assertFalse(rset.isLast());
-        
+
         // Can move forwards to after last row
         rset.afterLast();
         Assert.assertTrue(rset.isAfterLast());
-        
+
         // Can move backwards to last row
         Assert.assertTrue(rset.last());
         Assert.assertTrue(rset.isLast());
@@ -1301,4 +1334,31 @@ public abstract class AbstractResultSetT
         rset.close();
         Assert.assertTrue(rset.isClosed());
     }
+
+    /**
+     * Tests movement through SELECT results
+     * 
+     * @throws SQLException
+     */
+    @Test
+    public void test_results_construct_movement_09() throws SQLException {
+        ResultSet rset = this.createResults(ds, "CONSTRUCT WHERE { ?s ?p ?o . } LIMIT 2", ResultSet.TYPE_SCROLL_INSENSITIVE);
+        Assert.assertEquals(ResultSet.TYPE_SCROLL_INSENSITIVE, rset.getType());
+        Assert.assertNotNull(rset);
+        Assert.assertFalse(rset.isClosed());
+        Assert.assertTrue(rset.isBeforeFirst());
+        Assert.assertFalse(rset.isLast());
+
+        // Can move forwards to after last row
+        rset.afterLast();
+        Assert.assertTrue(rset.isAfterLast());
+
+        // Moving backwards more rows than possible with relative movement
+        // should place us before first row
+        Assert.assertFalse(rset.relative(-4));
+        Assert.assertTrue(rset.isBeforeFirst());
+
+        rset.close();
+        Assert.assertTrue(rset.isClosed());
+    }
 }