[jira] Created: (DERBY-4251) Derby EmbedResultSet does not conform to JDBC API for getRow()

Derby EmbedResultSet does not conform to JDBC API for getRow()
--------------------------------------------------------------

                 Key: DERBY-4251
                 URL: https://issues.apache.org/jira/browse/DERBY-4251
             Project: Derby
          Issue Type: Bug
          Components: JDBC
    Affects Versions: 10.5.1.1, 10.4.2.0
         Environment: Windows XP, and Java 6 or Java 1.4.2
            Reporter: Andrew McRae


According to source of the Derby version I first found it in, and the version just released today, here is the offending code:

https://svn.apache.org/repos/asf/db/derby/code/tags/10.5.1.1/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java
{quote}
{{
        public int getRow() throws SQLException {
                // getRow() is only allowed on scroll cursors
                checkScrollCursor("getRow()");

                /*
                 * * We probably needn't bother getting the text of * the underlying
                 * statement but it is better to be * consistent and we aren't
                 * particularly worried * about performance of getRow().
                 */
                return theResults.getRowNumber();
        }
}}
{quote}

The comment and the check for scrollability is incorrect. The Javadoc comment for this getRow is a duplicate of the Sun JDK API for ResultSet.getRow(), and if the author had just written it according to their own javadoc there would be no problem.
See spec here:
 http://java.sun.com/j2se/1.3/docs/api/java/sql/ResultSet.html#getRow()

The Sun Javadoc does not spell out any exceptions to the required behaviour. getRow should always work regardless of whether the result set is scrollable because the issue of scrollability is whether the client can *reposition* the cursor to change the current row of the result set. Simply *reading* the current row number is different and unrelated to changing the current row number.
Therefore getRow() should work for FORWARD_ONLY ResultSets, but Derby currently fails on this.

I haven't looked at what " theResults.getRowNumber() " does, but I hope the fix for this bug is as simple as removing the call to checkScrollCursor;


--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-4251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12715052#action_12715052 ]

Andrew McRae commented on DERBY-4251:
-------------------------------------

By the way....
In the meantime, a possible workaround in my own application is to detect when Derby is the connected database and try to enable a scrollable ResultSet when creating the Statement, but continue to open the simpler forward_only variety for all other database types. I don't know if my workaround works or not. I will not be able to test it until tomorrow.
-Andrew McRae.

> Derby EmbedResultSet does not conform to JDBC API for getRow()
> --------------------------------------------------------------
>
>                 Key: DERBY-4251
>                 URL: https://issues.apache.org/jira/browse/DERBY-4251
>             Project: Derby
>          Issue Type: Bug
>          Components: JDBC
>    Affects Versions: 10.4.2.0, 10.5.1.1
>         Environment: Windows XP, and Java 6 or Java 1.4.2
>            Reporter: Andrew McRae
>   Original Estimate: 2h
>  Remaining Estimate: 2h
>
> According to source of the Derby version I first found it in, and the version just released today, here is the offending code:
> https://svn.apache.org/repos/asf/db/derby/code/tags/10.5.1.1/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java
> {quote}
> public int getRow() throws SQLException {
> // getRow() is only allowed on scroll cursors
> checkScrollCursor("getRow()");
> /*
> * * We probably needn't bother getting the text of * the underlying
> * statement but it is better to be * consistent and we aren't
> * particularly worried * about performance of getRow().
> */
> return theResults.getRowNumber();
> }
> {quote}
> The comment and the check for scrollability is incorrect. The Javadoc comment for this getRow is a duplicate of the Sun JDK API for ResultSet.getRow(), and if the author had just written it according to their own javadoc there would be no problem.
> See spec here:
>  http://java.sun.com/j2se/1.3/docs/api/java/sql/ResultSet.html#getRow()
> The Sun Javadoc does not spell out any exceptions to the required behaviour. getRow should always work regardless of whether the result set is scrollable because the issue of scrollability is whether the client can *reposition* the cursor to change the current row of the result set. Simply *reading* the current row number is different and unrelated to changing the current row number.
> Therefore getRow() should work for FORWARD_ONLY ResultSets, but Derby currently fails on this.
> Quite aside from the spec nonconformance, it's a bit annoying that my application has worked fine on Oracle, MS SQL, the JDBC-LDAP bridge driver, and even MS Access MDB files via the Sun JDBC-ODBC bridge driver, but then the Derby engine falls over just trying to run a simple Select statement on a non-updatable forward-only result set - the simplest kind of all!
> I haven't looked at what " theResults.getRowNumber() " does, but I hope the fix for this bug is as simple as removing the call to checkScrollCursor;

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-4251?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andrew McRae updated DERBY-4251:
--------------------------------

    Description:
According to source of the Derby version I first found it in, and the version just released today, here is the offending code:

https://svn.apache.org/repos/asf/db/derby/code/tags/10.5.1.1/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java
{quote}

        public int getRow() throws SQLException {
                // getRow() is only allowed on scroll cursors
                checkScrollCursor("getRow()");

                /*
                 * * We probably needn't bother getting the text of * the underlying
                 * statement but it is better to be * consistent and we aren't
                 * particularly worried * about performance of getRow().
                 */
                return theResults.getRowNumber();
        }

{quote}

The comment and the check for scrollability is incorrect. The Javadoc comment for this getRow is a duplicate of the Sun JDK API for ResultSet.getRow(), and if the author had just written it according to their own javadoc there would be no problem.
See spec here:
 http://java.sun.com/j2se/1.3/docs/api/java/sql/ResultSet.html#getRow()

The Sun Javadoc does not spell out any exceptions to the required behaviour. getRow should always work regardless of whether the result set is scrollable because the issue of scrollability is whether the client can *reposition* the cursor to change the current row of the result set. Simply *reading* the current row number is different and unrelated to changing the current row number.
Therefore getRow() should work for FORWARD_ONLY ResultSets, but Derby currently fails on this.

Quite aside from the spec nonconformance, it's a bit annoying that my application has worked fine on Oracle, MS SQL, the JDBC-LDAP bridge driver, and even MS Access MDB files via the Sun JDBC-ODBC bridge driver, but then the Derby engine falls over just trying to run a simple Select statement on a non-updatable forward-only result set - the simplest kind of all!

I haven't looked at what " theResults.getRowNumber() " does, but I hope the fix for this bug is as simple as removing the call to checkScrollCursor;

  was:
According to source of the Derby version I first found it in, and the version just released today, here is the offending code:

https://svn.apache.org/repos/asf/db/derby/code/tags/10.5.1.1/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java
{quote}
{{
        public int getRow() throws SQLException {
                // getRow() is only allowed on scroll cursors
                checkScrollCursor("getRow()");

                /*
                 * * We probably needn't bother getting the text of * the underlying
                 * statement but it is better to be * consistent and we aren't
                 * particularly worried * about performance of getRow().
                 */
                return theResults.getRowNumber();
        }
}}
{quote}

The comment and the check for scrollability is incorrect. The Javadoc comment for this getRow is a duplicate of the Sun JDK API for ResultSet.getRow(), and if the author had just written it according to their own javadoc there would be no problem.
See spec here:
 http://java.sun.com/j2se/1.3/docs/api/java/sql/ResultSet.html#getRow()

The Sun Javadoc does not spell out any exceptions to the required behaviour. getRow should always work regardless of whether the result set is scrollable because the issue of scrollability is whether the client can *reposition* the cursor to change the current row of the result set. Simply *reading* the current row number is different and unrelated to changing the current row number.
Therefore getRow() should work for FORWARD_ONLY ResultSets, but Derby currently fails on this.

I haven't looked at what " theResults.getRowNumber() " does, but I hope the fix for this bug is as simple as removing the call to checkScrollCursor;

> Derby EmbedResultSet does not conform to JDBC API for getRow()
> --------------------------------------------------------------
>
>                 Key: DERBY-4251
>                 URL: https://issues.apache.org/jira/browse/DERBY-4251
>             Project: Derby
>          Issue Type: Bug
>          Components: JDBC
>    Affects Versions: 10.4.2.0, 10.5.1.1
>         Environment: Windows XP, and Java 6 or Java 1.4.2
>            Reporter: Andrew McRae
>   Original Estimate: 2h
>  Remaining Estimate: 2h
>
> According to source of the Derby version I first found it in, and the version just released today, here is the offending code:
> https://svn.apache.org/repos/asf/db/derby/code/tags/10.5.1.1/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java
> {quote}
> public int getRow() throws SQLException {
> // getRow() is only allowed on scroll cursors
> checkScrollCursor("getRow()");
> /*
> * * We probably needn't bother getting the text of * the underlying
> * statement but it is better to be * consistent and we aren't
> * particularly worried * about performance of getRow().
> */
> return theResults.getRowNumber();
> }
> {quote}
> The comment and the check for scrollability is incorrect. The Javadoc comment for this getRow is a duplicate of the Sun JDK API for ResultSet.getRow(), and if the author had just written it according to their own javadoc there would be no problem.
> See spec here:
>  http://java.sun.com/j2se/1.3/docs/api/java/sql/ResultSet.html#getRow()
> The Sun Javadoc does not spell out any exceptions to the required behaviour. getRow should always work regardless of whether the result set is scrollable because the issue of scrollability is whether the client can *reposition* the cursor to change the current row of the result set. Simply *reading* the current row number is different and unrelated to changing the current row number.
> Therefore getRow() should work for FORWARD_ONLY ResultSets, but Derby currently fails on this.
> Quite aside from the spec nonconformance, it's a bit annoying that my application has worked fine on Oracle, MS SQL, the JDBC-LDAP bridge driver, and even MS Access MDB files via the Sun JDBC-ODBC bridge driver, but then the Derby engine falls over just trying to run a simple Select statement on a non-updatable forward-only result set - the simplest kind of all!
> I haven't looked at what " theResults.getRowNumber() " does, but I hope the fix for this bug is as simple as removing the call to checkScrollCursor;

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-4251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12715383#action_12715383 ]

Kristian Waagan commented on DERBY-4251:
----------------------------------------

I'm not saying this can't/shouldn't be fixed, I'd just like to comment that the Java API JavaDoc has been changed in the latest version. The following additional sentence is present in the Java SE 6 docs:
"Note:Support for the getRow method is optional for ResultSets with a result set type of TYPE_FORWARD_ONLY"

> Derby EmbedResultSet does not conform to JDBC API for getRow()
> --------------------------------------------------------------
>
>                 Key: DERBY-4251
>                 URL: https://issues.apache.org/jira/browse/DERBY-4251
>             Project: Derby
>          Issue Type: Bug
>          Components: JDBC
>    Affects Versions: 10.4.2.0, 10.5.1.1
>         Environment: Windows XP, and Java 6 or Java 1.4.2
>            Reporter: Andrew McRae
>   Original Estimate: 2h
>  Remaining Estimate: 2h
>
> According to source of the Derby version I first found it in, and the version just released today, here is the offending code:
> https://svn.apache.org/repos/asf/db/derby/code/tags/10.5.1.1/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java
> {quote}
> public int getRow() throws SQLException {
> // getRow() is only allowed on scroll cursors
> checkScrollCursor("getRow()");
> /*
> * * We probably needn't bother getting the text of * the underlying
> * statement but it is better to be * consistent and we aren't
> * particularly worried * about performance of getRow().
> */
> return theResults.getRowNumber();
> }
> {quote}
> The comment and the check for scrollability is incorrect. The Javadoc comment for this getRow is a duplicate of the Sun JDK API for ResultSet.getRow(), and if the author had just written it according to their own javadoc there would be no problem.
> See spec here:
>  http://java.sun.com/j2se/1.3/docs/api/java/sql/ResultSet.html#getRow()
> The Sun Javadoc does not spell out any exceptions to the required behaviour. getRow should always work regardless of whether the result set is scrollable because the issue of scrollability is whether the client can *reposition* the cursor to change the current row of the result set. Simply *reading* the current row number is different and unrelated to changing the current row number.
> Therefore getRow() should work for FORWARD_ONLY ResultSets, but Derby currently fails on this.
> Quite aside from the spec nonconformance, it's a bit annoying that my application has worked fine on Oracle, MS SQL, the JDBC-LDAP bridge driver, and even MS Access MDB files via the Sun JDBC-ODBC bridge driver, but then the Derby engine falls over just trying to run a simple Select statement on a non-updatable forward-only result set - the simplest kind of all!
> I haven't looked at what " theResults.getRowNumber() " does, but I hope the fix for this bug is as simple as removing the call to checkScrollCursor;

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-4251?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Dag H. Wanvik updated DERBY-4251:
---------------------------------

    Issue Type: Improvement  (was: Bug)
       Summary: Make Derby EmbedResultSet support  getRow() for FORWARD_ONLY result sets  (was: Derby EmbedResultSet does not conform to JDBC API for getRow())

Triaged for 10.5.2, since this is optional in the new API docs, I change this into
an improvement issue, and make the title reflect that.

> Make Derby EmbedResultSet support  getRow() for FORWARD_ONLY result sets
> ------------------------------------------------------------------------
>
>                 Key: DERBY-4251
>                 URL: https://issues.apache.org/jira/browse/DERBY-4251
>             Project: Derby
>          Issue Type: Improvement
>          Components: JDBC
>    Affects Versions: 10.4.2.0, 10.5.1.1
>         Environment: Windows XP, and Java 6 or Java 1.4.2
>            Reporter: Andrew McRae
>   Original Estimate: 2h
>  Remaining Estimate: 2h
>
> According to source of the Derby version I first found it in, and the version just released today, here is the offending code:
> https://svn.apache.org/repos/asf/db/derby/code/tags/10.5.1.1/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java
> {quote}
> public int getRow() throws SQLException {
> // getRow() is only allowed on scroll cursors
> checkScrollCursor("getRow()");
> /*
> * * We probably needn't bother getting the text of * the underlying
> * statement but it is better to be * consistent and we aren't
> * particularly worried * about performance of getRow().
> */
> return theResults.getRowNumber();
> }
> {quote}
> The comment and the check for scrollability is incorrect. The Javadoc comment for this getRow is a duplicate of the Sun JDK API for ResultSet.getRow(), and if the author had just written it according to their own javadoc there would be no problem.
> See spec here:
>  http://java.sun.com/j2se/1.3/docs/api/java/sql/ResultSet.html#getRow()
> The Sun Javadoc does not spell out any exceptions to the required behaviour. getRow should always work regardless of whether the result set is scrollable because the issue of scrollability is whether the client can *reposition* the cursor to change the current row of the result set. Simply *reading* the current row number is different and unrelated to changing the current row number.
> Therefore getRow() should work for FORWARD_ONLY ResultSets, but Derby currently fails on this.
> Quite aside from the spec nonconformance, it's a bit annoying that my application has worked fine on Oracle, MS SQL, the JDBC-LDAP bridge driver, and even MS Access MDB files via the Sun JDBC-ODBC bridge driver, but then the Derby engine falls over just trying to run a simple Select statement on a non-updatable forward-only result set - the simplest kind of all!
> I haven't looked at what " theResults.getRowNumber() " does, but I hope the fix for this bug is as simple as removing the call to checkScrollCursor;

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-4251?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Dag H. Wanvik updated DERBY-4251:
---------------------------------

    Bug behavior facts:   (was: [Deviation from standard])

> Make Derby EmbedResultSet support  getRow() for FORWARD_ONLY result sets
> ------------------------------------------------------------------------
>
>                 Key: DERBY-4251
>                 URL: https://issues.apache.org/jira/browse/DERBY-4251
>             Project: Derby
>          Issue Type: Improvement
>          Components: JDBC
>    Affects Versions: 10.4.2.0, 10.5.1.1
>         Environment: Windows XP, and Java 6 or Java 1.4.2
>            Reporter: Andrew McRae
>   Original Estimate: 2h
>  Remaining Estimate: 2h
>
> According to source of the Derby version I first found it in, and the version just released today, here is the offending code:
> https://svn.apache.org/repos/asf/db/derby/code/tags/10.5.1.1/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java
> {quote}
> public int getRow() throws SQLException {
> // getRow() is only allowed on scroll cursors
> checkScrollCursor("getRow()");
> /*
> * * We probably needn't bother getting the text of * the underlying
> * statement but it is better to be * consistent and we aren't
> * particularly worried * about performance of getRow().
> */
> return theResults.getRowNumber();
> }
> {quote}
> The comment and the check for scrollability is incorrect. The Javadoc comment for this getRow is a duplicate of the Sun JDK API for ResultSet.getRow(), and if the author had just written it according to their own javadoc there would be no problem.
> See spec here:
>  http://java.sun.com/j2se/1.3/docs/api/java/sql/ResultSet.html#getRow()
> The Sun Javadoc does not spell out any exceptions to the required behaviour. getRow should always work regardless of whether the result set is scrollable because the issue of scrollability is whether the client can *reposition* the cursor to change the current row of the result set. Simply *reading* the current row number is different and unrelated to changing the current row number.
> Therefore getRow() should work for FORWARD_ONLY ResultSets, but Derby currently fails on this.
> Quite aside from the spec nonconformance, it's a bit annoying that my application has worked fine on Oracle, MS SQL, the JDBC-LDAP bridge driver, and even MS Access MDB files via the Sun JDBC-ODBC bridge driver, but then the Derby engine falls over just trying to run a simple Select statement on a non-updatable forward-only result set - the simplest kind of all!
> I haven't looked at what " theResults.getRowNumber() " does, but I hope the fix for this bug is as simple as removing the call to checkScrollCursor;

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-4251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12727232#action_12727232 ]

Andrew McRae commented on DERBY-4251:
-------------------------------------

The most relevant fact is that the vast majority of Java database code already running in the real world was written according to JDBC spec 3.0 or earlier versions. What I would like you to think about and ask yourself is "What is the most relevant JDBC specification for Derby?"

Consider that Derby still (thankfully) runs on Java 1.4 - thus allowing it to work in the widest possible range of Java runtimes already out there. The current JDBC api version at the time JRE 1.4 was released (2003) until Java 6 came out (2008) was JDBC 3.0 as defined in the  JDBC 3.0 spec (December 2001) and the Java 1.4 API Javadoc.

Here's what the spec says about compliance:
--8<------8<----
JDBC 3.0 API Compliance
A driver that is compliant with the JDBC 3.0 API must do the following:
*  Comply with the JDBC 2.0 API requirements
*  Include the following required interfaces:
     java.sql.ParameterMetaData
     java.sql.Savepoint
...
--8<------8<----

Here's the Java 1.4 API:
http://java.sun.com/j2se/1.4.2/docs/api/java/sql/ResultSet.html#getRow()
--8<------8<----
getRow

public int getRow()
           throws SQLException

    Retrieves the current row number. The first row is number 1, the second number 2, and so on.

    Returns:
        the current row number; 0 if there is no current row
    Throws:
        SQLException - if a database access error occurs
    Since:
        1.2
--8<------8<----

This is the behaviour required by over 5 years of software developed throughout the IT industry. There is no point in claiming to support later versions of the JDBC specification if Derby does not support the JDBC 2.0 API.

Here is what the JDBC 4.0 spec says about the goals of the 4.0 specification:
--8<------8<----
8. Maintain backward compatibility with existing applications and drivers
     Existing JDBC technology-enabled drivers ( JDBC drivers) and the applications
     that use them must continue to work in an implementation of the Java virtual
     machine that supports the JDBC 4.0 API. Applications that use only features
     defined in earlier releases of the JDBC API will not require changes to continue
     running. It should be straightforward for existing applications to migrate to
     JDBC 4.0 technology.
--8<------8<----

The latest JDBC documentation has added a new comment saying getRow() is optional for the special case of forward-only result sets, but clearly by the above reasoning this is self-contradictory to the goals of JDBC 4.0 as stated by Sun, and so should be ignored. The getRow() method has never been optional and by the chain of logic going back to JDBC 2 it cannot become optional.

Let me explain this in a different way in case it has not become clear.

Specification compliance is not a matter of being cool, it's a matter of being practically useful due to fully functional interoperability. The more recent the spec the software supports then the more features it will support and the cooler it will be, but not at the expense of breaking compatibility with the older specifications because that would make it useless to most existing software. (That would also be very uncool.)

This issue is not a request for a feature enhancement as you have now incorrectly recategorised it.

This is a bug, it is a nonconformance to the JDBC 2.0 API, and Derby must comply with that API for three reasons:
1) to allow all existing Java software to use Derby (assuming you do actually want people to use Derby), and
2) to merit compliance with BOTH the JDBC 3.0 and JDBC 4.0 specifications, and
3) to be cool.

This makes sense to me. What does everyone else think?

> Make Derby EmbedResultSet support  getRow() for FORWARD_ONLY result sets
> ------------------------------------------------------------------------
>
>                 Key: DERBY-4251
>                 URL: https://issues.apache.org/jira/browse/DERBY-4251
>             Project: Derby
>          Issue Type: Improvement
>          Components: JDBC
>    Affects Versions: 10.4.2.0, 10.5.1.1
>         Environment: Windows XP, and Java 6 or Java 1.4.2
>            Reporter: Andrew McRae
>   Original Estimate: 2h
>  Remaining Estimate: 2h
>
> According to source of the Derby version I first found it in, and the version just released today, here is the offending code:
> https://svn.apache.org/repos/asf/db/derby/code/tags/10.5.1.1/java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java
> {quote}
> public int getRow() throws SQLException {
> // getRow() is only allowed on scroll cursors
> checkScrollCursor("getRow()");
> /*
> * * We probably needn't bother getting the text of * the underlying
> * statement but it is better to be * consistent and we aren't
> * particularly worried * about performance of getRow().
> */
> return theResults.getRowNumber();
> }
> {quote}
> The comment and the check for scrollability is incorrect. The Javadoc comment for this getRow is a duplicate of the Sun JDK API for ResultSet.getRow(), and if the author had just written it according to their own javadoc there would be no problem.
> See spec here:
>  http://java.sun.com/j2se/1.3/docs/api/java/sql/ResultSet.html#getRow()
> The Sun Javadoc does not spell out any exceptions to the required behaviour. getRow should always work regardless of whether the result set is scrollable because the issue of scrollability is whether the client can *reposition* the cursor to change the current row of the result set. Simply *reading* the current row number is different and unrelated to changing the current row number.
> Therefore getRow() should work for FORWARD_ONLY ResultSets, but Derby currently fails on this.
> Quite aside from the spec nonconformance, it's a bit annoying that my application has worked fine on Oracle, MS SQL, the JDBC-LDAP bridge driver, and even MS Access MDB files via the Sun JDBC-ODBC bridge driver, but then the Derby engine falls over just trying to run a simple Select statement on a non-updatable forward-only result set - the simplest kind of all!
> I haven't looked at what " theResults.getRowNumber() " does, but I hope the fix for this bug is as simple as removing the call to checkScrollCursor;

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.