patch: fix ResultSet.isLast and friends with cursor-based resultsets - Mailing list pgsql-jdbc

The attached patch fixes (and adds testcases for) the various resultset
position querying methods (isFirst, isLast, isBeforeFirst, isAfterLast)
that can be called on any resultset. The current driver does not handle
these methods correctly when a TYPE_FORWARD_ONLY resultset that is
backed by a cursor is used.

isLast() may be more expensive after this patch (as it may need to do a
FETCH), but this cost is warned about in the JDBC javadoc.

A couple of other changes also in this patch:

  - prevent use of relative row movement methods (relative, next,
previous) when on the insert row -- as I read the spec these shouldn't
be allowed.
  - reset onInsertRow whenever an absolute row positioning method is called.

-O
Index: org/postgresql/errors.properties
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/errors.properties,v
retrieving revision 1.29
diff -c -r1.29 errors.properties
*** org/postgresql/errors.properties    2 Apr 2004 06:50:22 -0000    1.29
--- org/postgresql/errors.properties    9 Apr 2004 13:49:07 -0000
***************
*** 77,82 ****
--- 77,83 ----
  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.res.oninsertrow:Can't use relative move methods while on the insert row.
  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/jdbc1/AbstractJdbc1ResultSet.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v
retrieving revision 1.32
diff -c -r1.32 AbstractJdbc1ResultSet.java
*** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    29 Mar 2004 19:17:11 -0000    1.32
--- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    9 Apr 2004 13:49:10 -0000
***************
*** 44,53 ****
--- 44,55 ----
      protected int updateCount;        // How many rows did we get back?
      protected long insertOID;        // The oid of an inserted row
      protected int current_row;        // Our pointer to where we are at
+     protected int row_offset;       // Offset into the actual resultset of row 0
      protected byte[][] this_row;        // the current row result
      protected BaseConnection connection;    // the connection which we returned from
      protected SQLWarning warnings = null;    // The warning chain
      protected boolean wasNullFlag = false;    // the flag for wasNull()
+     protected boolean onInsertRow = false;  // are we on the insert row (for JDBC2 updatable resultsets)?

      // We can chain multiple resultSets together - this points to
      // next resultSet in the chain.
***************
*** 61,68 ****
       private SimpleDateFormat m_dateFormat = null;


!     private int fetchSize;      // Fetch size for next read (might be 0).
!     private int lastFetchSize;  // Fetch size of last read (might be 0).

      public abstract ResultSetMetaData getMetaData() throws SQLException;

--- 63,70 ----
       private SimpleDateFormat m_dateFormat = null;


!     protected int fetchSize;      // Fetch size for next read (might be 0).
!     protected int lastFetchSize;  // Fetch size of last read (might be 0).

      public abstract ResultSetMetaData getMetaData() throws SQLException;

***************
*** 83,88 ****
--- 85,91 ----
          this.insertOID = insertOID;
          this.this_row = null;
          this.current_row = -1;
+         this.row_offset = 0;

          this.lastFetchSize = this.fetchSize = (statement == null ? 0 : statement.getFetchSize());
      }
***************
*** 133,138 ****
--- 136,144 ----
      {
          if (rows == null)
              throw new PSQLException("postgresql.con.closed", PSQLState.CONNECTION_DOES_NOT_EXIST);
+
+         if (onInsertRow)
+             throw new PSQLException("postgresql.res.oninsertrow");

          if (current_row+1 >= rows.size())
          {
***************
*** 155,160 ****
--- 161,167 ----
                   ("FETCH FORWARD " + fetchSize + " FROM " + cursorName)
               };

+             row_offset += rows.size(); // We are discarding some data.
                QueryExecutor.execute(sql,
                                     new String[0],
                                      this);
Index: org/postgresql/jdbc2/AbstractJdbc2ResultSet.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/AbstractJdbc2ResultSet.java,v
retrieving revision 1.35
diff -c -r1.35 AbstractJdbc2ResultSet.java
*** org/postgresql/jdbc2/AbstractJdbc2ResultSet.java    2 Apr 2004 06:50:23 -0000    1.35
--- org/postgresql/jdbc2/AbstractJdbc2ResultSet.java    9 Apr 2004 13:49:12 -0000
***************
*** 29,34 ****
--- 29,35 ----
  import org.postgresql.core.BaseStatement;
  import org.postgresql.core.Field;
  import org.postgresql.core.Encoding;
+ import org.postgresql.core.QueryExecutor;
  import org.postgresql.util.PSQLException;
  import org.postgresql.util.PSQLState;

***************
*** 39,45 ****
      //needed for updateable result set support
      protected boolean updateable = false;
      protected boolean doingUpdates = false;
-     protected boolean onInsertRow = false;
      protected Hashtable updateValues = new Hashtable();
      private boolean usingOID = false;    // are we using the OID for the primary key?
      private Vector primaryKeys;    // list of primary keys
--- 40,45 ----
***************
*** 224,229 ****
--- 224,230 ----

          rowBuffer = new byte[this_row.length][];
          System.arraycopy(this_row, 0, rowBuffer, 0, this_row.length);
+         onInsertRow = false;

          return true;
      }
***************
*** 236,241 ****
--- 237,244 ----
          final int rows_size = rows.size();
          if (rows_size > 0)
              current_row = rows_size;
+
+         onInsertRow = false;
      }


***************
*** 245,250 ****
--- 248,255 ----

          if (rows.size() > 0)
              current_row = -1;
+
+         onInsertRow = false;
      }


***************
*** 261,266 ****
--- 266,272 ----

          rowBuffer = new byte[this_row.length][];
          System.arraycopy(this_row, 0, rowBuffer, 0, this_row.length);
+         onInsertRow = false;

          return true;
      }
***************
*** 452,457 ****
--- 458,466 ----

      public int getRow() throws SQLException
      {
+         if (onInsertRow)
+             return 0;
+
          final int rows_size = rows.size();

          if (current_row < 0 || current_row >= rows_size)
***************
*** 476,481 ****
--- 485,493 ----

      public boolean isAfterLast() throws SQLException
      {
+         if (onInsertRow)
+             return false;
+
          final int rows_size = rows.size();
          return (current_row >= rows_size && rows_size > 0);
      }
***************
*** 483,504 ****

      public boolean isBeforeFirst() throws SQLException
      {
!         return (current_row < 0 && rows.size() > 0);
      }


      public boolean isFirst() throws SQLException
      {
!         return (current_row == 0 && rows.size() >= 0);
      }


      public boolean isLast() throws SQLException
      {
          final int rows_size = rows.size();
-         return (current_row == rows_size - 1 && rows_size > 0);
-     }


      public boolean last() throws SQLException
      {
--- 495,567 ----

      public boolean isBeforeFirst() throws SQLException
      {
!         if (onInsertRow)
!             return false;
!
!         return ((row_offset + current_row) < 0 && rows.size() > 0);
      }


      public boolean isFirst() throws SQLException
      {
!         if (onInsertRow)
!             return false;
!
!         return ((row_offset + current_row) == 0);
      }


      public boolean isLast() throws SQLException
      {
+         if (onInsertRow)
+             return false;
+
          final int rows_size = rows.size();

+         if (rows_size == 0)
+             return false; // No rows.
+
+         if (current_row != (rows_size - 1))
+             return false; // Not on the last row of this block.
+
+         // We are on the last row of the current block.
+         String cursorName = statement.getFetchingCursorName();
+         if (cursorName == null || lastFetchSize == 0 || rows_size < lastFetchSize) {
+             // This is the last block and therefore the last row.
+             return true;
+         }
+
+         // Now the more painful case begins.
+         // We are on the last row of the current block, but we don't know if the
+         // current block is the last block; we must try to fetch some more data to
+         // find out.
+
+         // We do a fetch of the next block, then prepend the current row to that
+         // block (so current_row == 0). This works as the current row
+         // must be the last row of the current block if we got this far.
+
+         byte[][] saveThisRow = this_row;   // Cleared by reInit().
+
+         String[] sql = new String[] {
+             fetchSize == 0 ? ("FETCH FORWARD ALL FROM " + cursorName) :
+             "FETCH FORWARD " + fetchSize + " FROM " + cursorName
+         };
+
+
+         QueryExecutor.execute(sql,
+                               new String[0],
+                               this);
+
+         // Now prepend our one saved row and move to it.
+         rows.insertElementAt(saveThisRow, 0);
+         current_row = 0;
+         this_row = saveThisRow;
+         row_offset += (rows_size - 1);  // We discarded all but one row of the last block.
+         lastFetchSize = (fetchSize == 0 ? 0 : fetchSize + 1); // There should be one more row than we fetched.
+
+         // Finally, now we can tell if we're the last row or not.
+         return (rows.size() == 1);
+     }

      public boolean last() throws SQLException
      {
***************
*** 513,518 ****
--- 576,582 ----

          rowBuffer = new byte[this_row.length][];
          System.arraycopy(this_row, 0, rowBuffer, 0, this_row.length);
+         onInsertRow = false;

          return true;
      }
***************
*** 522,527 ****
--- 586,594 ----
      {
          checkScrollable();

+         if (onInsertRow)
+             throw new PSQLException("postgresql.res.oninsertrow");
+
          if (current_row-1 < 0) {
              current_row = -1;
              return false;
***************
*** 538,543 ****
--- 605,613 ----
      public boolean relative(int rows) throws SQLException
      {
          checkScrollable();
+
+         if (onInsertRow)
+             throw new PSQLException("postgresql.res.oninsertrow");

          //have to add 1 since absolute expects a 1-based index
          return absolute(current_row + 1 + rows);
Index: org/postgresql/test/jdbc2/CursorFetchTest.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/test/jdbc2/CursorFetchTest.java,v
retrieving revision 1.5
diff -c -r1.5 CursorFetchTest.java
*** org/postgresql/test/jdbc2/CursorFetchTest.java    15 Jan 2004 08:50:40 -0000    1.5
--- org/postgresql/test/jdbc2/CursorFetchTest.java    9 Apr 2004 13:49:13 -0000
***************
*** 247,252 ****
--- 247,342 ----
          assertEquals(100, count);
      }

+     public void testSingleRowResultPositioning() throws Exception
+     {
+         String msg;
+         createRows(1);
+
+         int[] sizes = { 0, 1, 10 };
+         for (int i = 0; i < sizes.length; ++i) {
+             Statement stmt = con.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_UPDATABLE);
+             stmt.setFetchSize(sizes[i]);
+
+             // Create a one row result set.
+             ResultSet rs = stmt.executeQuery("select * from test_fetch order by value");
+
+             msg = "before-first row positioning error with fetchsize=" + sizes[i];
+             assertTrue(msg, rs.isBeforeFirst());
+             assertTrue(msg, !rs.isAfterLast());
+             assertTrue(msg, !rs.isFirst());
+             assertTrue(msg, !rs.isLast());
+
+             msg = "row 1 positioning error with fetchsize=" + sizes[i];
+             assertTrue(msg, rs.next());
+
+             assertTrue(msg, !rs.isBeforeFirst());
+             assertTrue(msg, !rs.isAfterLast());
+             assertTrue(msg, rs.isFirst());
+             assertTrue(msg, rs.isLast());
+             assertEquals(msg, 0, rs.getInt(1));
+
+             msg = "after-last row positioning error with fetchsize=" + sizes[i];
+             assertTrue(msg, !rs.next());
+
+             assertTrue(msg, !rs.isBeforeFirst());
+             assertTrue(msg, rs.isAfterLast());
+             assertTrue(msg, !rs.isFirst());
+             assertTrue(msg, !rs.isLast());
+
+             rs.close();
+             stmt.close();
+         }
+     }
+
+     public void testMultiRowResultPositioning() throws Exception
+     {
+         String msg;
+
+         createRows(100);
+
+         int[] sizes = { 0, 1, 10, 100 };
+         for (int i = 0; i < sizes.length; ++i) {
+             Statement stmt = con.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_UPDATABLE);
+             stmt.setFetchSize(sizes[i]);
+
+             ResultSet rs = stmt.executeQuery("select * from test_fetch order by value");
+             msg = "before-first row positioning error with fetchsize=" + sizes[i];
+             assertTrue(msg, rs.isBeforeFirst());
+             assertTrue(msg, !rs.isAfterLast());
+             assertTrue(msg, !rs.isFirst());
+             assertTrue(msg, !rs.isLast());
+
+             for (int j = 0; j < 100; ++j) {
+                 msg = "row " + j + " positioning error with fetchsize=" + sizes[i];
+                 assertTrue(msg, rs.next());
+                 assertEquals(msg, j, rs.getInt(1));
+
+                 assertTrue(msg, !rs.isBeforeFirst());
+                 assertTrue(msg, !rs.isAfterLast());
+                 if (j == 0)
+                     assertTrue(msg, rs.isFirst());
+                 else
+                     assertTrue(msg, !rs.isFirst());
+
+                 if (j == 99)
+                     assertTrue(msg, rs.isLast());
+                 else
+                     assertTrue(msg, !rs.isLast());
+             }
+
+             msg = "after-last row positioning error with fetchsize=" + sizes[i];
+             assertTrue(msg, !rs.next());
+
+             assertTrue(msg, !rs.isBeforeFirst());
+             assertTrue(msg, rs.isAfterLast());
+             assertTrue(msg, !rs.isFirst());
+             assertTrue(msg, !rs.isLast());
+
+             rs.close();
+             stmt.close();
+         }
+     }
+
      // Test odd queries that should not be transformed into cursor-based fetches.
      public void testInsert() throws Exception
      {
Index: org/postgresql/test/jdbc2/ResultSetTest.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/test/jdbc2/ResultSetTest.java,v
retrieving revision 1.16
diff -c -r1.16 ResultSetTest.java
*** org/postgresql/test/jdbc2/ResultSetTest.java    2 Apr 2004 06:50:24 -0000    1.16
--- org/postgresql/test/jdbc2/ResultSetTest.java    9 Apr 2004 13:49:14 -0000
***************
*** 333,346 ****
--- 333,386 ----
          Statement stmt = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE);
          // Create a one row result set.
          ResultSet rs = stmt.executeQuery("SELECT * FROM pg_database WHERE datname='template1'");
+
          assertTrue(rs.isBeforeFirst());
+         assertTrue(!rs.isAfterLast());
+         assertTrue(!rs.isFirst());
+         assertTrue(!rs.isLast());
+
          assertTrue(rs.next());
+
+         assertTrue(!rs.isBeforeFirst());
+         assertTrue(!rs.isAfterLast());
          assertTrue(rs.isFirst());
          assertTrue(rs.isLast());
+
          assertTrue(!rs.next());
+
+         assertTrue(!rs.isBeforeFirst());
          assertTrue(rs.isAfterLast());
+         assertTrue(!rs.isFirst());
+         assertTrue(!rs.isLast());
+
          assertTrue(rs.previous());
+
+         assertTrue(!rs.isBeforeFirst());
+         assertTrue(!rs.isAfterLast());
+         assertTrue(rs.isFirst());
+         assertTrue(rs.isLast());
+
          assertTrue(rs.absolute(1));
+
+         assertTrue(!rs.isBeforeFirst());
+         assertTrue(!rs.isAfterLast());
+         assertTrue(rs.isFirst());
+         assertTrue(rs.isLast());
+
+         assertTrue(!rs.absolute(0));
+
+         assertTrue(rs.isBeforeFirst());
+         assertTrue(!rs.isAfterLast());
+         assertTrue(!rs.isFirst());
+         assertTrue(!rs.isLast());
+
+         assertTrue(!rs.absolute(2));
+
+         assertTrue(!rs.isBeforeFirst());
+         assertTrue(rs.isAfterLast());
+         assertTrue(!rs.isFirst());
+         assertTrue(!rs.isLast());
+
          rs.close();
          stmt.close();
      }