Repository: xerial/sqlite-jdbc (java)

Issue:
Statement.setFetchDirection non-compliant?
```
     * Gives the driver a hint as to the direction in which
     * rows will be processed in <code>ResultSet</code>
     * objects created using this <code>Statement</code> object.  The
     * default value is <code>ResultSet.FETCH_FORWARD</code>.
```

Giving this hint causes an exception.

```
java.sql.SQLException: ResultSet closed
	at org.sqlite.core.CoreResultSet.checkOpen(CoreResultSet.java:76)
	at org.sqlite.jdbc3.JDBC3ResultSet.setFetchDirection(JDBC3ResultSet.java:125)
	at org.sqlite.jdbc3.JDBC3Statement.setFetchDirection(JDBC3Statement.java:326)
```

Pull request:
Improve Compliance With JDBC Specifications
Should fix #515 and #546 

* Connection.setTransactionIsolation - promote isolation levels READ_COMMITTED and REPEATABLE_READ, which are not natively supported by SQLite, to SERIALIZABLE. The driver should accept all four values as long as it can substitute a higher isolation level than the one requested.
* Statement.setFetchDirection - while SQLite does not act on the fetch direction hint, it should accept the call so long as the argument is valid.

In both of these cases, spec-compliance makes it easier to write driver-agnostic user code.

ResultSet.setFetchDirection was not changed, because the current impl is actually correct. It is correct because SQLite only allows FORWARD_ONLY ResultSets.

Patch:
diff --git a/src/main/java/org/sqlite/SQLiteConnection.java b/src/main/java/org/sqlite/SQLiteConnection.java
index d5d78b86..b2034939 100644
--- a/src/main/java/org/sqlite/SQLiteConnection.java
+++ b/src/main/java/org/sqlite/SQLiteConnection.java
@@ -154,6 +154,9 @@ public abstract class SQLiteConnection
         checkOpen();
 
         switch (level) {
+            case java.sql.Connection.TRANSACTION_READ_COMMITTED:
+            case java.sql.Connection.TRANSACTION_REPEATABLE_READ:
+                // Fall-through: Spec allows upgrading isolation to a higher level
             case java.sql.Connection.TRANSACTION_SERIALIZABLE:
                 getDatabase().exec("PRAGMA read_uncommitted = false;", getAutoCommit());
                 break;
@@ -161,7 +164,9 @@ public abstract class SQLiteConnection
                 getDatabase().exec("PRAGMA read_uncommitted = true;", getAutoCommit());
                 break;
             default:
-                throw new SQLException("SQLite supports only TRANSACTION_SERIALIZABLE and TRANSACTION_READ_UNCOMMITTED.");
+                throw new SQLException("Unsupported transaction isolation level: " + level + ". " +
+                        "Must be one of TRANSACTION_READ_UNCOMMITTED, TRANSACTION_READ_COMMITTED, " +
+                        "TRANSACTION_REPEATABLE_READ, or TRANSACTION_SERIALIZABLE in java.sql.Connection");
         }
         connectionConfig.setTransactionIsolation(level);
     }
diff --git a/src/main/java/org/sqlite/jdbc3/JDBC3ResultSet.java b/src/main/java/org/sqlite/jdbc3/JDBC3ResultSet.java
index 303aad47..14e80402 100644
--- a/src/main/java/org/sqlite/jdbc3/JDBC3ResultSet.java
+++ b/src/main/java/org/sqlite/jdbc3/JDBC3ResultSet.java
@@ -123,7 +123,9 @@ public abstract class JDBC3ResultSet extends CoreResultSet {
      */
     public void setFetchDirection(int d) throws SQLException {
         checkOpen();
-        if (d != ResultSet.FETCH_FORWARD) {
+        // Only FORWARD_ONLY ResultSets exist in SQLite, so only FETCH_FORWARD is permitted
+        if (/*getType() == ResultSet.TYPE_FORWARD_ONLY &&*/
+                d != ResultSet.FETCH_FORWARD) {
             throw new SQLException("only FETCH_FORWARD direction supported");
         }
     }
diff --git a/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java b/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java
index c6c2e711..e0ee79f3 100644
--- a/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java
+++ b/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java
@@ -309,14 +309,23 @@ public abstract class JDBC3Statement extends CoreStatement {
      * @see java.sql.Statement#getFetchDirection()
      */
     public int getFetchDirection() throws SQLException {
-        return ((ResultSet)rs).getFetchDirection();
+        return ResultSet.FETCH_FORWARD;
     }
 
     /**
      * @see java.sql.Statement#setFetchDirection(int)
      */
-    public void setFetchDirection(int d) throws SQLException {
-        ((ResultSet)rs).setFetchDirection(d);
+    public void setFetchDirection(int direction) throws SQLException {
+        switch (direction) {
+        case ResultSet.FETCH_FORWARD:
+        case ResultSet.FETCH_REVERSE:
+        case ResultSet.FETCH_UNKNOWN:
+            // No-op: SQLite does not support a value other than FETCH_FORWARD
+            break;
+        default:
+            throw new SQLException("Unknown fetch direction " + direction + ". " +
+                    "Must be one of FETCH_FORWARD, FETCH_REVERSE, or FETCH_UNKNOWN in java.sql.ResultSet");
+        }
     }
 
     /**


Test patch:
diff --git a/src/test/java/org/sqlite/ReadUncommittedTest.java b/src/test/java/org/sqlite/ReadUncommittedTest.java
index 69868004..5fd2bd20 100644
--- a/src/test/java/org/sqlite/ReadUncommittedTest.java
+++ b/src/test/java/org/sqlite/ReadUncommittedTest.java
@@ -57,8 +57,8 @@ public class ReadUncommittedTest
         conn.setTransactionIsolation(SQLiteConnection.TRANSACTION_SERIALIZABLE);
     }
 
-    @Test(expected = SQLException.class)
-    public void setUnsupportedIsolationLevel() throws SQLException
+    @Test
+    public void setIsolationPromotedToSerializable() throws SQLException
     {
         conn.setTransactionIsolation(SQLiteConnection.TRANSACTION_REPEATABLE_READ);
     }
diff --git a/src/test/java/org/sqlite/StatementTest.java b/src/test/java/org/sqlite/StatementTest.java
index 13fabfc2..06afa387 100644
--- a/src/test/java/org/sqlite/StatementTest.java
+++ b/src/test/java/org/sqlite/StatementTest.java
@@ -492,4 +492,20 @@ public class StatementTest
         assertTrue( stat.isClosed() );
     }
 
+    @Test
+    public void setFetchDirection() throws SQLException {
+        stat.setFetchDirection(ResultSet.FETCH_FORWARD);
+        stat.setFetchDirection(ResultSet.FETCH_REVERSE);
+        stat.setFetchDirection(ResultSet.FETCH_UNKNOWN);
+    }
+
+    @Test(expected = SQLException.class)
+    public void setFetchDirectionBadArgument() throws SQLException {
+        stat.setFetchDirection(999);
+    }
+
+    @Test
+    public void getFetchDirection() throws SQLException {
+        assertEquals(ResultSet.FETCH_FORWARD, stat.getFetchDirection());
+    }
 }