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

Bryan Pendleton edited comment on DERBY-6849 at 11/20/16 10:07 PM:
-------------------------------------------------------------------

After a long delay, I've been thinking about this problem some more.

I'm starting from the premise that Derby's current getGeneratedKeys()
implementation is incorrect, as it violates the documented behavior of
Statement.getGeneratedKeys:
{quote}
 If this Statement object did not generate any keys, an empty ResultSet object 
is returned.
{quote}

The current implementation of Statement.getGeneratedKeys() in
EmbedStatement.java is, as we've discussed earlier, hard-wired to
implement getGeneratedKeys() by compiling and executing the
SQL statement "VALUES IDENTITY_VAL_LOCAL()":

{code}
        public final java.sql.ResultSet getGeneratedKeys() throws SQLException  
{
                checkStatus();
                if (autoGeneratedKeysResultSet == null)
                        return null;
                else {
                        execute("VALUES IDENTITY_VAL_LOCAL()", true, false, 
Statement.NO_GENERATED_KEYS, null, null);
                        return results;
                }
        }
{code}

This, in turn, is wired deep in the system to generate a ResultSet with exactly 
one row,
because that's basically what it means to issue a SQL "VALUES" statement against
the result of the IDENTITY_VAL_LOCAL() function.

>From a code point of view, that trace looks like this:

{quote}
        at 
org.apache.derby.impl.sql.execute.RowResultSet.<init>(RowResultSet.java:84)
        at 
org.apache.derby.impl.sql.execute.GenericResultSetFactory.getRowResultSet(GenericResultSetFactory.java:464)
        at 
org.apache.derby.exe.acf81e0010x0158x83aax78bbx000006ff41201.createResultSet(Unknown
 Source)
        at 
org.apache.derby.impl.sql.execute.CursorActivation.decorateResultSet(CursorActivation.java:63)
        at 
org.apache.derby.impl.sql.execute.BaseActivation.execute(BaseActivation.java:288)
        at 
org.apache.derby.impl.sql.GenericActivationHolder.execute(GenericActivationHolder.java:337)
        at 
org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:470)
        at 
org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:351)
        at 
org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1339)
        at 
org.apache.derby.impl.jdbc.EmbedPreparedStatement.executeStatement(EmbedPreparedStatement.java:1709)
        at 
org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:705)
        at 
org.apache.derby.impl.jdbc.EmbedStatement.getGeneratedKeys(EmbedStatement.java:1246)
{quote}

So I don't think that there's a straightforward, or even a desirable, way to 
change the
implementation of
{code}
VALUES IDENTITY_VAL_LOCAL()
{code}
so that it sometimes returns a single-row result set, and sometimes returns an 
empty result set.

However, it seems to me that we could possibly address this problem in the
implementation of EmbedStatement.getGeneratedKeys() by changing it so that
it reads something like:

{code}
        public final java.sql.ResultSet getGeneratedKeys() throws SQLException  
{
                checkStatus();
                if (autoGeneratedKeysResultSet == null)
                        return null;
                else if( autoGeneratedKeysResultSet.noKeysWereGenerated() )
                        return EmptyResultSet();
                else {
                        execute("VALUES IDENTITY_VAL_LOCAL()", true, false, 
Statement.NO_GENERATED_KEYS, null, null);
                        return results;
                }
        }
{code}

Obviously, we'd need to implement this new "noKeysWereGenerated()" method,
and we'd have to properly return a "EmptyResultSet".

So that requires a bit of coding.

But possibly it's a way forward, so I wanted to record that idea for others to
think about and react to.



was (Author: bryanpendleton):
After a long delay, I've been thinking about this problem some more.

I'm starting from the premise that Derby's current getGeneratedKeys()
implementation is incorrect, as it violates the documented behavior of
Statement.getGeneratedKeys:
{blockquote}
 If this Statement object did not generate any keys, an empty ResultSet object 
is returned.
{blockquote}

The current implementation of Statement.getGeneratedKeys() in
EmbedStatement.java is, as we've discussed earlier, hard-wired to
implement getGeneratedKeys() by compiling and executing the
SQL statement "VALUES IDENTITY_VAL_LOCAL()":

{code}
        public final java.sql.ResultSet getGeneratedKeys() throws SQLException  
{
                checkStatus();
                if (autoGeneratedKeysResultSet == null)
                        return null;
                else {
                        execute("VALUES IDENTITY_VAL_LOCAL()", true, false, 
Statement.NO_GENERATED_KEYS, null, null);
                        return results;
                }
        }
{code}

This, in turn, is wired deep in the system to generate a ResultSet with exactly 
one row,
because that's basically what it means to issue a SQL "VALUES" statement against
the result of the IDENTITY_VAL_LOCAL() function.

>From a code point of view, that trace looks like this:

{blockquote}
        at 
org.apache.derby.impl.sql.execute.RowResultSet.<init>(RowResultSet.java:84)
        at 
org.apache.derby.impl.sql.execute.GenericResultSetFactory.getRowResultSet(GenericResultSetFactory.java:464)
        at 
org.apache.derby.exe.acf81e0010x0158x83aax78bbx000006ff41201.createResultSet(Unknown
 Source)
        at 
org.apache.derby.impl.sql.execute.CursorActivation.decorateResultSet(CursorActivation.java:63)
        at 
org.apache.derby.impl.sql.execute.BaseActivation.execute(BaseActivation.java:288)
        at 
org.apache.derby.impl.sql.GenericActivationHolder.execute(GenericActivationHolder.java:337)
        at 
org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:470)
        at 
org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:351)
        at 
org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1339)
        at 
org.apache.derby.impl.jdbc.EmbedPreparedStatement.executeStatement(EmbedPreparedStatement.java:1709)
        at 
org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:705)
        at 
org.apache.derby.impl.jdbc.EmbedStatement.getGeneratedKeys(EmbedStatement.java:1246)
{blockquote}

So I don't think that there's a straightforward, or even a desirable, way to 
change the
implementation of
{code}
VALUES IDENTITY_VAL_LOCAL()
{code}
so that it sometimes returns a single-row result set, and sometimes returns an 
empty result set.

However, it seems to me that we could possibly address this problem in the
implementation of EmbedStatement.getGeneratedKeys() by changing it so that
it reads something like:

{code}
        public final java.sql.ResultSet getGeneratedKeys() throws SQLException  
{
                checkStatus();
                if (autoGeneratedKeysResultSet == null)
                        return null;
                else if( autoGeneratedKeysResultSet.noKeysWereGenerated() )
                        return EmptyResultSet();
                else {
                        execute("VALUES IDENTITY_VAL_LOCAL()", true, false, 
Statement.NO_GENERATED_KEYS, null, null);
                        return results;
                }
        }
{code}

Obviously, we'd need to implement this new "noKeysWereGenerated()" method,
and we'd have to properly return a "EmptyResultSet".

So that requires a bit of coding.

But possibly it's a way forward, so I wanted to record that idea for others to
think about and react to.


> Statement.RETURN_GENERATED_KEYS returns a 1 row result set even if there are 
> no auto-generated fields
> -----------------------------------------------------------------------------------------------------
>
>                 Key: DERBY-6849
>                 URL: https://issues.apache.org/jira/browse/DERBY-6849
>             Project: Derby
>          Issue Type: Bug
>          Components: JDBC
>    Affects Versions: 10.9.1.0
>            Reporter: John Hendrikx
>         Attachments: DERBY6849Repro.java
>
>
> If:
> 1) A JDBC INSERT statement is executed, with Statement.RETURN_GENERATED_KEYS 
> enabled, and
> 2) A call is then made to Statement.getGeneratedKeys, and
> 3) The table which was inserted into has *NO* generated columns,
> then getGeneratedKeys() returns a ResultSet object with a single row in it.
> This behavior seems incorrect; it seems that the correct behavior
> would be to return a ResultSet object which has *NO* rows in it, so
> that ResultSet.next() returns FALSE the first time it is called.
>  
> I have a very simple table:
> {noformat}
>     CREATE TABLE images (
>       url varchar(1000) NOT NULL,
>       image blob NOT NULL,
>   
>       CONSTRAINT images_url PRIMARY KEY (url)
>     );
> {noformat}
> No auto-generated fields.  However when I do an insert, JDBC tells me there 
> are auto-generated keys (rs.next() does not return false and a LONG value is 
> returned):
> {noformat}
>       try(PreparedStatement statement = connection.prepareStatement(sql, 
> Statement.RETURN_GENERATED_KEYS)) {
>         setParameters(parameterValues, statement);
>         statement.execute();
>         try(ResultSet rs = statement.getGeneratedKeys()) {
>           if(rs.next()) {
>             return rs.getObject(1);
>           }
>           return null;
>         }
>       }
>       catch(SQLException e) {
>         throw new DatabaseException(this, sql + ": " + parameters, e);
>       }
> {noformat}
> This sounds like a bug to me.  For comparison, PostgreSQL does not have the 
> same behaviour.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to