Re: Result Set Cursor Patch

On Tue, 11 May 2004, Andy Zeneski wrote:

> Here is that patch again, correct without any formatting or whitespace
> changes (white space was ignored in the diff). Let me know if this is
> better then the last one.
>

This is much better, but there are still a fair number of inconsequential
changes like renaming this_row and other variables. If we had
some naming standards that you were matching to, or you were making
everything in one localized area match I wouldn't object, but I can't say
I understand the name changes. Anyway that's a minor issue at this point,
so lets get to the heart of the matter.

The JDBC driver's goal is compatibility for the current version and the
previous two releases. Two key features this depends on were introduced
in 7.4: ABSOLUTE cursor positioning and the SCROLL keyword actually
guaranteeing a scrollable cursor in all situations. Ideally we could
retain the current FORWARD_ONLY requirement for 7.3 servers.

I notice ResultSetChunk has get/setFetchSize, but they are unused and lack
the checking provided in the AbstractJdbc2ResultSet versions.

I tried creating three 100 row tables which in an unconstrained join
produce a million rows and testing some things.

rs.absolute(44000000); // past the end of the result

LOG: statement: DECLARE JDBC_CURS_1 SCROLL CURSOR FOR SELECT a,b,c FROM
t1,t2,t3 ; FETCH FORWARD 3 FROM JDBC_CURS_1
LOG: duration: 1.425 ms
LOG: statement: MOVE ABSOLUTE 43999999 IN JDBC_CURS_1
LOG: duration: 3565.519 ms
LOG: statement: FETCH FORWARD 3 FROM JDBC_CURS_1
LOG: duration: 2.236 ms
LOG: statement: MOVE FORWARD ALL IN JDBC_CURS_1
LOG: duration: 2.134 ms
LOG: statement: MOVE ABSOLUTE 0 IN JDBC_CURS_1
LOG: duration: 2.139 ms
LOG: statement: MOVE FORWARD ALL IN JDBC_CURS_1
LOG: duration: 1312.637 ms
LOG: statement: MOVE ABSOLUTE 43999999 IN JDBC_CURS_1
LOG: duration: 2.135 ms

This jumps back to zero and then to the end again for some reason costing
us 1.3 seconds in this test.

In general something seems wrong. Testing using previous() with a forward
fetch direction I see:

rs.absolute();
while (rs.previous()) ;

LOG: statement: DECLARE JDBC_CURS_1 SCROLL CURSOR FOR SELECT a,b,c FROM
t1,t2,t3 ; FETCH FORWARD 3 FROM JDBC_CURS_1
LOG: duration: 1.441 ms
LOG: statement: MOVE ABSOLUTE 19 IN JDBC_CURS_1
LOG: duration: 0.394 ms
LOG: statement: FETCH FORWARD 3 FROM JDBC_CURS_1
LOG: duration: 0.349 ms
LOG: statement: MOVE ABSOLUTE 0 IN JDBC_CURS_1
LOG: duration: 0.225 ms
LOG: statement: MOVE FORWARD ALL IN JDBC_CURS_1
LOG: duration: 3550.779 ms
LOG: statement: MOVE ABSOLUTE 22 IN JDBC_CURS_1
LOG: duration: 2.287 ms
LOG: statement: MOVE ABSOLUTE 18 IN JDBC_CURS_1
LOG: duration: 2.164 ms
LOG: statement: FETCH FORWARD 3 FROM JDBC_CURS_1
LOG: duration: 2.231 ms
LOG: statement: MOVE ABSOLUTE 17 IN JDBC_CURS_1
LOG: duration: 2.156 ms
LOG: statement: FETCH FORWARD 3 FROM JDBC_CURS_1
LOG: duration: 2.268 ms

This strangely jumps back and forward after the first fetch and kills
performance for this query.

The way that previous() currently works will be a real downer for many
people, especially with a reasonably large fetch size. While FETCH
BACKWARD is ideal, a stopgap solution would be to MOVE back and fetch
forward which looks like it might be easier with your current code setup.

For this particular case (especially because people probably aren't
setting fetch direction) I would disagree with your defaulting of
fetchSize to 1000 instead of it's previous 0 meaning no cursor fetching.

Finally I notice that the cursor is not closed when either the ResultSet
or Statement are closed. This wasn't really a problem with non-scrolling
cursors, but now with the possibility of consuming significant server side
resources this is a necessity.

Kris Jurka