This patch (against CVS HEAD) adds enforcement of the requirement that
you're working with a scrollable resultset before calling some methods
(last(), absolute(), etc) of ResultSet. Without this patch, these
methods complete "normally" but can return incorrect data if the
resultset is backed by a cursor. It also adds tests for this behaviour,
and fixes a number of tests and one case in the driver itself that try
to use these methods with the wrong resultset type.
There are also a couple of minor other fixes in related areas that I
made along the way.
Details:
- Add AbstractJdbc2ResultSet.checkScrollable() that throws if the
resultset is not scrollable, call it from appropriate methods.
- Allow ResultSet.absolute(0) per the spec: it should position before
the start of the resultset, not throw an exception. Add a test for this
case.
- Check the ResultSet.setFetchDirection() parameter and throw an
exception if it's not one of the allowed values.
- Use rs.next() not rs.first() in AbstractJdbc2ResultSet.refreshRow(),
as the resultset in use is not scrollable.
- If a SQLException is caught in AbstractJdbc2ResultSet.refreshRow(),
rethrow it unchanged to preserve the stack trace.
- Fix multiple cases in ResultSetTest where we were expecting a
scrollable resultset but didn't ask for one.
- Add a testcase for throwing an exception when inappropriate methods
are called on TYPE_FORWARD_ONLY resultsets.
-O
Index: org/postgresql/errors.properties
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/errors.properties,v
retrieving revision 1.28
diff -c -r1.28 errors.properties
*** org/postgresql/errors.properties 23 Feb 2004 12:57:49 -0000 1.28
--- org/postgresql/errors.properties 1 Apr 2004 09:21:25 -0000
***************
*** 75,80 ****
--- 75,82 ----
postgresql.res.colname:The column name {0} not found.
postgresql.res.colrange:The column index is out of range.
postgresql.res.nextrequired:Result set not positioned properly, perhaps you need to call next().
+ postgresql.res.notscrollable:Operation requires a scrollable resultset, but this resultset is FORWARD_ONLY.
+ postgresql.res.badfetchdirection:Invalid direction constant {0}.
postgresql.serial.interface:You cannot serialize an interface.
postgresql.serial.namelength:Class & Package name length cannot be longer than 64 characters. {0} is {1} characters.
postgresql.serial.noclass:No class found for {0}
Index: org/postgresql/jdbc2/AbstractJdbc2ResultSet.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/AbstractJdbc2ResultSet.java,v
retrieving revision 1.34
diff -c -r1.34 AbstractJdbc2ResultSet.java
*** org/postgresql/jdbc2/AbstractJdbc2ResultSet.java 29 Mar 2004 19:17:11 -0000 1.34
--- org/postgresql/jdbc2/AbstractJdbc2ResultSet.java 1 Apr 2004 09:21:28 -0000
***************
*** 173,186 ****
}
}
public boolean absolute(int index) throws SQLException
{
// index is 1-based, but internally we use 0-based indices
int internalIndex;
! if (index == 0)
! throw new SQLException("Cannot move to index of 0");
final int rows_size = rows.size();
--- 173,195 ----
}
}
+ private void checkScrollable() throws SQLException
+ {
+ if (resultsettype == ResultSet.TYPE_FORWARD_ONLY)
+ throw new PSQLException("postgresql.res.notscrollable");
+ }
public boolean absolute(int index) throws SQLException
{
+ checkScrollable();
+
// index is 1-based, but internally we use 0-based indices
int internalIndex;
! if (index == 0) {
! beforeFirst();
! return false;
! }
final int rows_size = rows.size();
***************
*** 222,227 ****
--- 231,238 ----
public void afterLast() throws SQLException
{
+ checkScrollable();
+
final int rows_size = rows.size();
if (rows_size > 0)
current_row = rows_size;
***************
*** 230,235 ****
--- 241,248 ----
public void beforeFirst() throws SQLException
{
+ checkScrollable();
+
if (rows.size() > 0)
current_row = -1;
}
***************
*** 237,242 ****
--- 250,257 ----
public boolean first() throws SQLException
{
+ checkScrollable();
+
if (rows.size() <= 0)
return false;
***************
*** 487,492 ****
--- 502,509 ----
public boolean last() throws SQLException
{
+ checkScrollable();
+
final int rows_size = rows.size();
if (rows_size <= 0)
return false;
***************
*** 503,508 ****
--- 520,527 ----
public boolean previous() throws SQLException
{
+ checkScrollable();
+
if (current_row-1 < 0) {
current_row = -1;
return false;
***************
*** 518,523 ****
--- 537,544 ----
public boolean relative(int rows) throws SQLException
{
+ checkScrollable();
+
//have to add 1 since absolute expects a 1-based index
return absolute(current_row + 1 + rows);
}
***************
*** 525,530 ****
--- 546,564 ----
public void setFetchDirection(int direction) throws SQLException
{
+ switch (direction) {
+ case ResultSet.FETCH_FORWARD:
+ break;
+ case ResultSet.FETCH_REVERSE:
+ case ResultSet.FETCH_UNKNOWN:
+ checkScrollable();
+ break;
+ default:
+ throw new PSQLException("postgresql.res.badfetchdirection",
+ null,
+ new Integer(direction));
+ }
+
this.fetchdirection = direction;
}
***************
*** 997,1003 ****
AbstractJdbc2ResultSet rs = (AbstractJdbc2ResultSet) selectStatement.executeQuery();
! if ( rs.first() )
{
rowBuffer = rs.rowBuffer;
}
--- 1031,1037 ----
AbstractJdbc2ResultSet rs = (AbstractJdbc2ResultSet) selectStatement.executeQuery();
! if ( rs.next() )
{
rowBuffer = rs.rowBuffer;
}
***************
*** 1011,1016 ****
--- 1045,1053 ----
selectStatement.close();
selectStatement = null;
+ }
+ catch (SQLException e) {
+ throw e;
}
catch (Exception e)
{
Index: org/postgresql/test/jdbc2/ResultSetTest.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/test/jdbc2/ResultSetTest.java,v
retrieving revision 1.15
diff -c -r1.15 ResultSetTest.java
*** org/postgresql/test/jdbc2/ResultSetTest.java 12 Feb 2004 19:09:47 -0000 1.15
--- org/postgresql/test/jdbc2/ResultSetTest.java 1 Apr 2004 09:21:30 -0000
***************
*** 86,92 ****
public void testBackward() throws SQLException
{
! Statement stmt = con.createStatement();
ResultSet rs = stmt.executeQuery("SELECT * FROM testrs");
rs.afterLast();
assertTrue(rs.previous());
--- 86,92 ----
public void testBackward() throws SQLException
{
! Statement stmt = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY);
ResultSet rs = stmt.executeQuery("SELECT * FROM testrs");
rs.afterLast();
assertTrue(rs.previous());
***************
*** 96,104 ****
public void testAbsolute() throws SQLException
{
! Statement stmt = con.createStatement();
ResultSet rs = stmt.executeQuery("SELECT * FROM testrs");
assertTrue(rs.absolute( -1));
assertEquals(6, rs.getRow());
--- 96,107 ----
public void testAbsolute() throws SQLException
{
! Statement stmt = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY);
ResultSet rs = stmt.executeQuery("SELECT * FROM testrs");
+ assertTrue(!rs.absolute(0));
+ assertEquals(0, rs.getRow());
+
assertTrue(rs.absolute( -1));
assertEquals(6, rs.getRow());
***************
*** 120,126 ****
public void testEmptyResult() throws SQLException
{
! Statement stmt = con.createStatement();
ResultSet rs = stmt.executeQuery("SELECT * FROM testrs where id=100");
rs.beforeFirst();
rs.afterLast();
--- 123,129 ----
public void testEmptyResult() throws SQLException
{
! Statement stmt = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY);
ResultSet rs = stmt.executeQuery("SELECT * FROM testrs where id=100");
rs.beforeFirst();
rs.afterLast();
***************
*** 342,345 ****
--- 345,376 ----
stmt.close();
}
+ public void testForwardOnlyExceptions() throws SQLException
+ {
+ // Test that illegal operations on a TYPE_FORWARD_ONLY resultset
+ // correctly result in throwing an exception.
+ Statement stmt = con.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
+ ResultSet rs = stmt.executeQuery("SELECT * FROM testnumeric");
+
+ try { rs.absolute(1); fail("absolute() on a TYPE_FORWARD_ONLY resultset did not throw an exception"); }
catch(SQLException e) {}
+ try { rs.afterLast(); fail("afterLast() on a TYPE_FORWARD_ONLY resultset did not throw an exception on a
TYPE_FORWARD_ONLYresultset"); } catch (SQLException e) {}
+ try { rs.beforeFirst(); fail("beforeFirst() on a TYPE_FORWARD_ONLY resultset did not throw an exception"); }
catch(SQLException e) {}
+ try { rs.first(); fail("first() on a TYPE_FORWARD_ONLY resultset did not throw an exception"); } catch
(SQLExceptione) {}
+ try { rs.last(); fail("last() on a TYPE_FORWARD_ONLY resultset did not throw an exception"); } catch
(SQLExceptione) {}
+ try { rs.previous(); fail("previous() on a TYPE_FORWARD_ONLY resultset did not throw an exception"); }
catch(SQLException e) {}
+ try { rs.relative(1); fail("relative() on a TYPE_FORWARD_ONLY resultset did not throw an exception"); }
catch(SQLException e) {}
+
+ try {
+ rs.setFetchDirection(ResultSet.FETCH_REVERSE);
+ fail("setFetchDirection(FETCH_REVERSE) on a TYPE_FORWARD_ONLY resultset did not throw an exception");
+ } catch (SQLException e) {}
+
+ try {
+ rs.setFetchDirection(ResultSet.FETCH_UNKNOWN);
+ fail("setFetchDirection(FETCH_UNKNOWN) on a TYPE_FORWARD_ONLY resultset did not throw an exception");
+ } catch (SQLException e) {}
+
+ rs.close();
+ stmt.close();
+ }
}