Re: Patch for Statement.getGeneratedKeys()

On Fri, 14 Dec 2007, Ken Johanson wrote:

> Kris, please try to apply the attached and let me know what errors if
> any you get.

This patch is completely busted. It uses backslashes instead of forward
slashes, which is relatively easily fixed, but it also has wrong line
numbers. Consider this section of the patch:

***************
*** 159,172 ****
*/
public int executeUpdate(String sql, int columnIndexes[]) throws
SQLException
{
! if (columnIndexes.length == 0)
return executeUpdate(sql);
!
! throw new PSQLException(GT.tr("Returning autogenerated keys is
not supported."), PSQLState.NOT_IMPLEMENTED);
}

/**
--- 166,206 ----

Here it claims to have lines 159 to 172, but it only has 10 lines of text.
Perhaps you need a Netbeans upgrade or you need to use some other
CVS client.

Reading through the patch I have the following comments:

1) Why does executeUpdateGetResults have support for isFunction? Doesn't
that require preparedQuery != null? Even if not, shouldn't the
replaceProcessing be before the isFunction check?

2) Shouldn't the result for a generated key result be stored in some
place more specific? Right now can't you issue executeQuery() and then
call getGeneratedKeys()?

3) Generated keys should work for more than just insert, at least in
postgres' design. RETURNING works for all of INSERT/UPDATE/DELETE.

4) As discussed previously on -general the code in executeUpdate(String,
int[]) shouldn't be using information_schema because that has additional
permission requirements. Also it looks like it's got sql injection
problems.

5) There's no need to check connection instanceof AbstractJdbc2Connection
in executeUpdate(String, String[]). That will always be true.

6) There's no need to split this up for translation purposes, just
make it one string:

throw new PSQLException(GT.tr("Server version does not support
returning generated keys.")+" (< "+"8.2"+")", PSQLState.NOT_IMPLEMENTED);

7) executeUpdate(String, String[]) does not correctly escape the
columnNames provided if the values have embedded quotes.

8) Utils.needsQuoted is unused and should be removed.

9) Utils.getInsertIds doesn't look right. Looks like it will return
"into" for something like "insert into xxx (...)". It doesn't look
like it will work for names with quotes in them like "x""y". Also
the requirement that a query uses a completely qualified name
database.schema.table is quite onerous. Additionally the fact that this
requirement is not checked will result in many
ArrayIndexOutOfBoundsExceptions.

10) What's the purpose of Utils.position? How is this better than
String.indexOf with lowercase strings?

Kris Jurka