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