Re: Patch for Statement.getGeneratedKeys()

On Mon, 7 Jan 2008, Ken Johanson wrote:

>> 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()?
>
> Again I am not familiar with all the use cases but presume you mean,
> allowing for calling a query (non-DML?) and then expect the keys to be
> available?..

What I'm saying is that since the "result" variable is set for every
ResultSet, someone can do executeQuery("SELECT ...") and then if they
call getGeneratedKeys it will return a reference to that ResultSet.
getGenereatedKeys should fail if it was not immediately preceded by a
call that created a ResultSet of generated keys.

>> 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.
>
> I am waiting on this and #9.b; for an answer to a prior question about:
> is it is possible to determine the connection's db and schema names to
> normalize those array elements (in the case where just the
> [schema]tablename is provided). Is it possible without a round trip to
> know what these should be?

You can tell the connection's database via Connection.getCatalog, but
there is no such thing as a connection's schema. Each connection has a
search_path that specifies the order it looks through schemas to find a
table. So you can't tell what schema a table is in without looking
through the list.

>> 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);
>>
>
> Unless the code will not work I will elect to keep support for translation
> should someone want to enter these.

I'm not saying it shouldn't be translatable I'm just saying put the "
< 8.2" part into the message string.

>> 10) What's the purpose of Utils.position? How is this better than
>> String.indexOf with lowercase strings?
>
> It save a potential buffer allocation should String.class find a case-fold is
> necessary (esp in large query).
>
> query.toLowerCase().indexOf(s);
>
> Utils.position is not strictly needed and I will remove it if you prefer.
>

Let's take it out.

Kris Jurka