Thread: Synthesize support for Statement.getGeneratedKeys()?

Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

14 December 2006, 04:52:14

Hello,

I've just come to realize that Statement.getGeneratedKeys() is not
supported in the current PG and/or JDBC driver. Does someone know if
this is a limitation of PG, or its protocol, or just not yet implemented
in the JDBC driver? I'm just wondering where, if at all (if I have
enough brain cells that is :), I could try to offer a patch or ideas..

Then question 2; I did see a discussion where it was suggested that we
could get roughly the same effect by issuing a
SELECT currval('<sequence-name>'); after the DML...
http://archives.postgresql.org/pgsql-jdbc/2005-10/msg00035.php
Would it then be feasible internal to the JDBC driver to (create an
option that would enable) always implicitly append that query to the
(String)sql arg of Statment.executeUpdate(String sql, String[]
columnNames)? I mean, just internally attempt to create the same
behavior as what this method should be doing?

Question 3 is, it seems like option two would only return the last
created key, not set of keys, in the case where multiple rows were
inserted.. is this accurate?

Unfortunately if I cant find a way to make my target-app work with PG
(without adding PG-specific modifications for getting keys), I'm
probably not going to be able to make the switch to PG unfortunately -
the code I'm working with makes really, really extensive use of
retrieved keys..

Thank you,
Ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Michael Paesold

Date:

14 December 2006, 06:50:13

Ken Johanson wrote:
> Hello,
>
> I've just come to realize that Statement.getGeneratedKeys() is not
> supported in the current PG and/or JDBC driver. Does someone know if
> this is a limitation of PG, or its protocol, or just not yet implemented
> in the JDBC driver? I'm just wondering where, if at all (if I have
> enough brain cells that is :), I could try to offer a patch or ideas..

It would be possible to implement that using the RETURNING clause
supported in recent versions of the server.

See here:
http://archives.postgresql.org/pgsql-jdbc/2006-10/msg00035.php
http://archives.postgresql.org/pgsql-jdbc/2006-10/msg00037.php

Unfortunately, all my available spare time went into implementing
support for standards_conforming_strings in the 8.2 release cycle.

> Then question 2; I did see a discussion where it was suggested that we
> could get roughly the same effect by issuing a
> SELECT currval('<sequence-name>'); after the DML...
> http://archives.postgresql.org/pgsql-jdbc/2005-10/msg00035.php
> Would it then be feasible internal to the JDBC driver to (create an
> option that would enable) always implicitly append that query to the
> (String)sql arg of Statment.executeUpdate(String sql, String[]
> columnNames)? I mean, just internally attempt to create the same
> behavior as what this method should be doing?

As was discussed before, a solution just for sequences is not general
enough, therefore the RETURNING thing.

> Question 3 is, it seems like option two would only return the last
> created key, not set of keys, in the case where multiple rows were
> inserted.. is this accurate?

Seems correct. And that's one of the objections to the idea.

> Unfortunately if I cant find a way to make my target-app work with PG
> (without adding PG-specific modifications for getting keys), I'm
> probably not going to be able to make the switch to PG unfortunately -
> the code I'm working with makes really, really extensive use of
> retrieved keys..

Could you comment on my mail above (the second one) -- what API-methods
does your software use, actually?

Best Regards
Michael Paesold

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

14 December 2006, 14:38:49

Michael Paesold wrote:
..
> It would be possible to implement that using the RETURNING clause
> supported in recent versions of the server.
>
> See here:
> http://archives.postgresql.org/pgsql-jdbc/2006-10/msg00035.php
> http://archives.postgresql.org/pgsql-jdbc/2006-10/msg00037.php
>
> Unfortunately, all my available spare time went into implementing
> support for standards_conforming_strings in the 8.2 release cycle.
>

And that is a feature which I VERY much appreciate having... thank you, btw!

>
> Could you comment on my mail above (the second one) -- what API-methods
> does your software use, actually?
>

Yes, of the four executeUpdates() that you listed in your email
discussion, it uses executeUpdate(String sql, String[] columnNames)
exclusively (the easy one :-)).

Aside from that, if there were a way to get the backend to support the
others (for example executeUpdate(String sql, int[] columnIdx)), that
would be a best investment of everyone's time. But my vote right now
would be for an implementation of (..,String[] columnNames) so that I
can get bootstrapped with PG :)

Either way, I will be very happy, of course, to do some beta testing.

> Best Regards
> Michael Paesold
>
>
>
>

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Michael Paesold

Date:

23 December 2006, 08:23:15

Ken Johanson wrote:
> Michael Paesold wrote:
>> Could you comment on my mail above (the second one) -- what
>> API-methods does your software use, actually?
>>
>
> Yes, of the four executeUpdates() that you listed in your email
> discussion, it uses executeUpdate(String sql, String[] columnNames)
> exclusively (the easy one :-)).
>
> Aside from that, if there were a way to get the backend to support the
> others (for example executeUpdate(String sql, int[] columnIdx)), that
> would be a best investment of everyone's time. But my vote right now
> would be for an implementation of (..,String[] columnNames) so that I
> can get bootstrapped with PG :)
>
> Either way, I will be very happy, of course, to do some beta testing.

I might be able to find some time to get this done during Christmas
holidays. I will report back next week.

Best Regards
Michael Paesold

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

29 December 2006, 03:03:18

Michael Paesold wrote:
>
> I might be able to find some time to get this done during Christmas
> holidays. I will report back next week.
>
> Best Regards
> Michael Paesold
>

Michael, sorry for my late response. Just to say thank you, and I'm back
online to test this as soon as you'd like. Seems like we both have taken
a break from the software world during Christmas :)

Ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

08 January 2007, 16:18:23

Michael Paesold wrote:
> Ken Johanson wrote:
>> Michael Paesold wrote:
>>> Could you comment on my mail above (the second one) -- what
>>> API-methods does your software use, actually?
>>>
>>
>> Yes, of the four executeUpdates() that you listed in your email
>> discussion, it uses executeUpdate(String sql, String[] columnNames)
>> exclusively (the easy one :-)).
>>

Michael, just wondering if you'd found some time to look into
implementing this; and if it is in CVS please let me know; I'm anxious
to test it out.

Thank you,
ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

20 January 2007, 04:44:06

Michael Paesold wrote:
> Ken Johanson wrote:
>> Hello,
>>
>> I've just come to realize that Statement.getGeneratedKeys() is not
>> supported in the current PG and/or JDBC driver. Does someone know if
>> this is a limitation of PG, or its protocol, or just not yet
>> implemented in the JDBC driver? I'm just wondering where, if at all
>> (if I have enough brain cells that is :), I could try to offer a patch
>> or ideas..
>
> It would be possible to implement that using the RETURNING clause
> supported in recent versions of the server.
>
> See here:
> http://archives.postgresql.org/pgsql-jdbc/2006-10/msg00035.php
> http://archives.postgresql.org/pgsql-jdbc/2006-10/msg00037.php
>
> Unfortunately, all my available spare time went into implementing
> support for standards_conforming_strings in the 8.2 release cycle.
>

Michael, does it look like you might have the time for this in the next
couple months? If not and no one else more familar/rofiecient with the
private API will have time, then I giuess I should dig in and try...
even if I need to ramp up to the server protocol to do it.

If someone has pointers to where the augmentaion code should be placed
(org.tgresql.jdbc3.Jdbc3ResultSetMetadata I presume), and also the
protocol (separate query to the server?, or append RETURNING clause to
the DML?).

Also I cant remember - is it true that when inserting multuple VALUES,
that only the first (or last) SEQUENCE can be retrived, or can I
populate a resultset using the RETURNING data from by the server?

Thanks again,
Ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

20 January 2007, 04:59:42

>
> If someone has pointers to where the augmentaion code should be placed
> (org.tgresql.jdbc3.Jdbc3ResultSetMetadata I presume), and also the
> protocol (separate query to the server?, or append RETURNING clause to
> the DML?).
>
> Also I cant remember - is it true that when inserting multuple VALUES,
> that only the first (or last) SEQUENCE can be retrived, or can I
> populate a resultset using the RETURNING data from by the server?
>

Answering my own question here... sort of :)
http://www.postgresql.org/docs/current/static/sql-insert.html

In the case of inserting multiple values (say 3 rows), is there a clever
way to get the last 3 sequences via native SQL? Or would the JDBC code
need to synthesize them?: (very simplified)

...exec
rs.next();//single key returned
Object id = rs.getObject();
Object[] keys = new Object[insertedRowCount];
for (; keys.length; i++)
   keys[i] = increment(id);
myPGResultSet.populate(userDeclaredKeyNameOrIdx, keys);//hypothetical -
i didn't look at the API yet, sorry

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Michael Paesold

Date:

20 January 2007, 10:08:49

Ken Johanson wrote:
> Michael Paesold wrote:
>> Ken Johanson wrote:
>>> Hello,
>>>
>>> I've just come to realize that Statement.getGeneratedKeys() is not
>>> supported in the current PG and/or JDBC driver. Does someone know if
>>> this is a limitation of PG, or its protocol, or just not yet
>>> implemented in the JDBC driver? I'm just wondering where, if at all
>>> (if I have enough brain cells that is :), I could try to offer a
>>> patch or ideas..
>>
>> It would be possible to implement that using the RETURNING clause
>> supported in recent versions of the server.
>>
>> See here:
>> http://archives.postgresql.org/pgsql-jdbc/2006-10/msg00035.php
>> http://archives.postgresql.org/pgsql-jdbc/2006-10/msg00037.php
>>
>> Unfortunately, all my available spare time went into implementing
>> support for standards_conforming_strings in the 8.2 release cycle.
>>
>
> Michael, does it look like you might have the time for this in the next
> couple months? If not and no one else more familar/rofiecient with the
> private API will have time, then I giuess I should dig in and try...
> even if I need to ramp up to the server protocol to do it.

Unfortunately I did not find time to work on this over Christmas
holidays and I will be quite busy at work for the next two or three
months or so. Thus, if you have time to work on this yourself, please go
ahead!

> If someone has pointers to where the augmentaion code should be placed
> (org.tgresql.jdbc3.Jdbc3ResultSetMetadata I presume), and also the
> protocol (separate query to the server?, or append RETURNING clause to
> the DML?).

Right now, the server protocol does not have a separate facility for the
RETURNING functionality. Therefore you have to append a RETURNING clause
   to the query string.

> Also I cant remember - is it true that when inserting multuple VALUES,
> that only the first (or last) SEQUENCE can be retrived, or can I
> populate a resultset using the RETURNING data from by the server?

I suppose it should work with multiple VALUES, but you can simple try. ;-)

Best Regards
Michael Paesold

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

20 January 2007, 16:43:59

>> If someone has pointers to where the augmentaion code should be placed
>> (org.tgresql.jdbc3.Jdbc3ResultSetMetadata I presume), and also the
>> protocol (separate query to the server?, or append RETURNING clause to
>> the DML?).
>
> Right now, the server protocol does not have a separate facility for the
> RETURNING functionality. Therefore you have to append a RETURNING clause
>   to the query string.
>
>> Also I cant remember - is it true that when inserting multuple VALUES,
>> that only the first (or last) SEQUENCE can be retrived, or can I
>> populate a resultset using the RETURNING data from by the server?
>
> I suppose it should work with multiple VALUES, but you can simple try. ;-)
>


For the PG & SQL masters out there, I am brain storming this...

1) It seems implicit that I will have to scan the input query for an
existing RETURNING clause, and replace it if it exists; is there any
pre-built query parsing helpers that would help with this? Or for
efficiency I would need to do an indexOf that scans from the end of the
CharSequence... thoughts?

2) Any queries where simply appending a RETURNING can somehow spoof
functionality (UPDATEs, etc), or cause unintended results? Is it always
permissible for RETURNING to be last? (i.e can it appear after other
user clauses, LIMIT, etc)

3) Is there a) an efficient RETURNING clause to pre-populate the
generated-keys result set, or b) should I synthesize it.

4) If 3b is required, then besides incrementing (by one) sequences, any
suggestions on how to correctly synthesize other increment values? Other
key types (OIDs, etc?)

5) To be absolutely sure, inserting multiple values then getting the
sequence back (RETURNING) will happen atomically in the server, correct?
(to avoid getting another transaction's keys).

If someone wants to write up a spec on how this should work then I'll
try to implement it in code. I can work without a spec but I'll make
more incorrect assumptions (not being PG proficient yet).

Thanks,
Ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Dave Cramer

Date:

20 January 2007, 17:06:29

Hi Ken
On 20-Jan-07, at 12:43 PM, Ken Johanson wrote:

>
>>> If someone has pointers to where the augmentaion code should be
>>> placed (org.tgresql.jdbc3.Jdbc3ResultSetMetadata I presume), and
>>> also the protocol (separate query to the server?, or append
>>> RETURNING clause to the DML?).
>> Right now, the server protocol does not have a separate facility
>> for the RETURNING functionality. Therefore you have to append a
>> RETURNING clause   to the query string.
>>> Also I cant remember - is it true that when inserting multuple
>>> VALUES, that only the first (or last) SEQUENCE can be retrived,
>>> or can I populate a resultset using the RETURNING data from by
>>> the server?
>> I suppose it should work with multiple VALUES, but you can simple
>> try. ;-)
>

We've pretty much avoided trying to parse the SQL up until now, and
I'm still not sure it's a good idea. My sense is that this will
burden the driver for everyone. How many people actually want this
implemented ?
>
> For the PG & SQL masters out there, I am brain storming this...
>
> 1) It seems implicit that I will have to scan the input query for
> an existing RETURNING clause, and replace it if it exists; is there
> any pre-built query parsing helpers that would help with this? Or
> for efficiency I would need to do an indexOf that scans from the
> end of the CharSequence... thoughts?

There is some parsing already done to break the query up into pieces,
perhaps just adding it there ?
>
> 2) Any queries where simply appending a RETURNING can somehow spoof
> functionality (UPDATEs, etc), or cause unintended results? Is it
> always permissible for RETURNING to be last? (i.e can it appear
> after other user clauses, LIMIT, etc)
>
> 3) Is there a) an efficient RETURNING clause to pre-populate the
> generated-keys result set, or b) should I synthesize it.

I take it from this that you are intending on returning all generated
columns?, just generated columns that have keys ?
>
> 4) If 3b is required, then besides incrementing (by one) sequences,
> any suggestions on how to correctly synthesize other increment
> values? Other key types (OIDs, etc?)

Are you suggesting here that you are planning on incrementing the
sequences by 1 ? Why not just let the insert occur and get the
currval of the sequence ?
>
> 5) To be absolutely sure, inserting multiple values then getting
> the sequence back (RETURNING) will happen atomically in the server,
> correct?
> (to avoid getting another transaction's keys).
You don't want to return the sequence, as it can be changed by
another transaction, you want to return the value in the row that was
inserted.
>
> If someone wants to write up a spec on how this should work then
> I'll try to implement it in code. I can work without a spec but
> I'll make more incorrect assumptions (not being PG proficient yet).

Dave
> Thanks,
> Ken
>
>
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 7: You can help support the PostgreSQL project by donating at
>
>                http://www.postgresql.org/about/donate
>

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

21 January 2007, 18:07:04

Dave Cramer wrote:
>>
>> 3) Is there a) an efficient RETURNING clause to pre-populate the
>> generated-keys result set, or b) should I synthesize it.
>
> I take it from this that you are intending on returning all generated
> columns?, just generated columns that have keys ?
>>

Yes, ideally. I'd prefer not to try and synthesize the values from only
a single (last or first row's) key, since sequences can have increment
values other than one, and because OIDs are not predictable (are they?).
Other server generated key types also seem to make the synthetic idea
unfeasible.

>> 4) If 3b is required, then besides incrementing (by one) sequences,
>> any suggestions on how to correctly synthesize other increment values?
>> Other key types (OIDs, etc?)
>
> Are you suggesting here that you are planning on incrementing the
> sequences by 1 ? Why not just let the insert occur and get the currval
> of the sequence ?

I'm thinking what you're thinking; get the current value; however the
increment I mention is just in case I cant get more than one curval... I
don't know, can I get 3 values from RETURN if I insert three VALUEs in
one query??

>>
>> 5) To be absolutely sure, inserting multiple values then getting the
>> sequence back (RETURNING) will happen atomically in the server, correct?
>> (to avoid getting another transaction's keys).
> You don't want to return the sequence, as it can be changed by another
> transaction, you want to return the value in the row that was inserted.
>>

Agreed.

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Dave Cramer

Date:

21 January 2007, 23:21:36

On 21-Jan-07, at 2:06 PM, Ken Johanson wrote:

> Dave Cramer wrote:
>>>
>>> 3) Is there a) an efficient RETURNING clause to pre-populate the
>>> generated-keys result set, or b) should I synthesize it.
>> I take it from this that you are intending on returning all
>> generated columns?, just generated columns that have keys ?
>>>
>
> Yes, ideally. I'd prefer not to try and synthesize the values from
> only a single (last or first row's) key, since sequences can have
> increment values other than one, and because OIDs are not
> predictable (are they?). Other server generated key types also seem
> to make the synthetic idea unfeasible.

I wouldn't worry about OID's first of all, they are no longer the
default on user tables, secondly, they are not indexed.
>
>>> 4) If 3b is required, then besides incrementing (by one)
>>> sequences, any suggestions on how to correctly synthesize other
>>> increment values? Other key types (OIDs, etc?)
>> Are you suggesting here that you are planning on incrementing the
>> sequences by 1 ? Why not just let the insert occur and get the
>> currval of the sequence ?
>
> I'm thinking what you're thinking; get the current value; however
> the increment I mention is just in case I cant get more than one
> curval... I don't know, can I get 3 values from RETURN if I insert
> three VALUEs in one query??
hmmm good question, one which I doubt the Sun people thought about. I
wouldn't bother trying to return any more than the last one.
>
>>>
>>> 5) To be absolutely sure, inserting multiple values then getting
>>> the sequence back (RETURNING) will happen atomically in the
>>> server, correct?
>>> (to avoid getting another transaction's keys).
>> You don't want to return the sequence, as it can be changed by
>> another transaction, you want to return the value in the row that
>> was inserted.
>>>
>
> Agreed.
>
>
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
>               http://archives.postgresql.org
>

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

21 January 2007, 23:53:04

>>>> 4) If 3b is required, then besides incrementing (by one) sequences,
>>>> any suggestions on how to correctly synthesize other increment
>>>> values? Other key types (OIDs, etc?)
>>> Are you suggesting here that you are planning on incrementing the
>>> sequences by 1 ? Why not just let the insert occur and get the
>>> currval of the sequence ?
>>
>> I'm thinking what you're thinking; get the current value; however the
>> increment I mention is just in case I cant get more than one curval...
>> I don't know, can I get 3 values from RETURN if I insert three VALUEs
>> in one query??
> hmmm good question, one which I doubt the Sun people thought about. I
> wouldn't bother trying to return any more than the last one.
>>


Well, unless server generated keys can only be numeric (increment by
one, i.e predictable) (which is the only kind I've used), then I think
it's essential that we somehow can get each of the generated keys.

As for Sun/others, I do know that this feature works well in other
dbms's - every one I've used supports it, and with multiple rows
inserted. But I've only ever used numeric server generated keys so I
don't know if other types are supported on those DBs.

As an aside, how do PG jdbc users get the server generated keys? Or does
everyone use some kind of UUID system (which I think is generally
regarded as detrimental to indexes/memory under high load and large DB
sizes - compared to int/bigint)? Or do PG users using some standard or
server-specific (RETURNING) SQL clause?

ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Dave Cramer

Date:

22 January 2007, 01:29:51

On 21-Jan-07, at 7:52 PM, Ken Johanson wrote:

>
>>>>> 4) If 3b is required, then besides incrementing (by one)
>>>>> sequences, any suggestions on how to correctly synthesize other
>>>>> increment values? Other key types (OIDs, etc?)
>>>> Are you suggesting here that you are planning on incrementing
>>>> the sequences by 1 ? Why not just let the insert occur and get
>>>> the currval of the sequence ?
>>>
>>> I'm thinking what you're thinking; get the current value; however
>>> the increment I mention is just in case I cant get more than one
>>> curval... I don't know, can I get 3 values from RETURN if I
>>> insert three VALUEs in one query??
>> hmmm good question, one which I doubt the Sun people thought
>> about. I wouldn't bother trying to return any more than the last one.
>>>
>
>
> Well, unless server generated keys can only be numeric (increment
> by one, i.e predictable) (which is the only kind I've used), then I
> think it's essential that we somehow can get each of the generated
> keys.
>
> As for Sun/others, I do know that this feature works well in other
> dbms's - every one I've used supports it, and with multiple rows
> inserted. But I've only ever used numeric server generated keys so
> I don't know if other types are supported on those DBs.
OK, I should look at the spec before making statements above. This is
possible in postgres, you can get whatever values were auto generated
by the insert
>
> As an aside, how do PG jdbc users get the server generated keys? Or
> does everyone use some kind of UUID system (which I think is
> generally regarded as detrimental to indexes/memory under high load
> and large DB sizes - compared to int/bigint)? Or do PG users using
> some standard or server-specific (RETURNING) SQL clause?

either create the key ahead of time select nextval('sequence') and
insert it explicitly, or insert the row and  then  select currval
('sequence')

Dave
>
> ken
>
>
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
>       choose an index scan if your joining column's datatypes do not
>       match
>

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

22 January 2007, 04:21:36

>> As an aside, how do PG jdbc users get the server generated keys? Or
>> does everyone use some kind of UUID system (which I think is generally
>> regarded as detrimental to indexes/memory under high load and large DB
>> sizes - compared to int/bigint)? Or do PG users using some standard or
>> server-specific (RETURNING) SQL clause?
>
> either create the key ahead of time select nextval('sequence') and
> insert it explicitly, or insert the row and  then  select
> currval('sequence')
>


That makes sense; the sequence is retrieved and it internally increments
- regardless of whether the key was actually inserted or not. I'm
personally not used to this though, it allows for actual keys in the
database to possibly have gaps (if the key want actually used / rollback
etc). Thats trivial / innocuous I guess, but I'm just used to having
sequential keys tables. Would this require two trips to the server, or
can we handle in one excecuteUpdate?

My real question is, what about the case where multiple VALUES are
inserted; if I have 3 values should I call the sequence 3 times? What is
the most efficient was to do that? (Can I do it in a single query?)

Thank you,
ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Michael Paesold

Date:

22 January 2007, 08:16:30

Ken Johanson wrote:
>
>>> As an aside, how do PG jdbc users get the server generated keys? Or
>>> does everyone use some kind of UUID system (which I think is
>>> generally regarded as detrimental to indexes/memory under high load
>>> and large DB sizes - compared to int/bigint)? Or do PG users using
>>> some standard or server-specific (RETURNING) SQL clause?
>>
>> either create the key ahead of time select nextval('sequence') and
>> insert it explicitly, or insert the row and  then  select
>> currval('sequence')
>>
>
>
> That makes sense; the sequence is retrieved and it internally increments
> - regardless of whether the key was actually inserted or not. I'm
> personally not used to this though, it allows for actual keys in the
> database to possibly have gaps (if the key want actually used / rollback
> etc). Thats trivial / innocuous I guess, but I'm just used to having
> sequential keys tables. Would this require two trips to the server, or
> can we handle in one excecuteUpdate?
>
> My real question is, what about the case where multiple VALUES are
> inserted; if I have 3 values should I call the sequence 3 times? What is
> the most efficient was to do that? (Can I do it in a single query?)

I don't think you should use "currval" or "nextval" at all. A general
solution in the JDBC driver should even work in the case of triggers
that interfere with the value of a sequence. Or which might change the
value actually inserted into the table. Just think of an insert trigger
that uses a sequence for a second time.

There is only one way to reliably get the database generated values: the
RETURNING clause.

So my basic suggestion was to rewrite a query written as:
"INSERT INTO tab VALUES (...)"
into
"INSERT INTO tab VALUES (...) RETURNING x"

With x being either (a) what the user specified using the Java API (i.e.
any column names) or (b) the primary key column(s) (or other columns
having a "DEFAULT currval(...)").
The second case (b) I would leave for later, since it requires parsing
the query and finding the table which will be inserted into. And you
would have to use database meta data to find the columns to return.

Of course, there should be a minimum amount of parsing to detect if the
query is a valid INSERT query and does not already have a different
RETURNING clause.

Another option would be to convince backend developers to add a way to
specify a "RETURNING clause" on the protocol level, i.e. without having
to change the query string.

Best Regards
Michael Paesold

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Dave Cramer

Date:

22 January 2007, 11:28:28

On 22-Jan-07, at 12:09 AM, Ken Johanson wrote:

>
>>> As an aside, how do PG jdbc users get the server generated keys?
>>> Or does everyone use some kind of UUID system (which I think is
>>> generally regarded as detrimental to indexes/memory under high
>>> load and large DB sizes - compared to int/bigint)? Or do PG users
>>> using some standard or server-specific (RETURNING) SQL clause?
>> either create the key ahead of time select nextval('sequence') and
>> insert it explicitly, or insert the row and  then  select currval
>> ('sequence')
>
>
> That makes sense; the sequence is retrieved and it internally
> increments - regardless of whether the key was actually inserted or
> not. I'm personally not used to this though, it allows for actual
> keys in the database to possibly have gaps (if the key want
> actually used / rollback etc). Thats trivial / innocuous I guess,
> but I'm just used to having sequential keys tables. Would this
> require two trips to the server, or can we handle in one
> excecuteUpdate?

Well, this is widely debated, but in Postgresql since the sequence
cannot be rolled back (easily) you have to design for gaps. Take for
instance the case where you cache sequences for speed. If you drop
that connection you will lose all the cached values.
>
> My real question is, what about the case where multiple VALUES are
> inserted; if I have 3 values should I call the sequence 3 times?
> What is the most efficient was to do that? (Can I do it in a single
> query?)

You have to call the sequence n times.
>
> Thank you,
> ken
>
>
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
>

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Dave Cramer

Date:

22 January 2007, 11:29:59

On 22-Jan-07, at 4:16 AM, Michael Paesold wrote:

> Ken Johanson wrote:
>>>> As an aside, how do PG jdbc users get the server generated keys?
>>>> Or does everyone use some kind of UUID system (which I think is
>>>> generally regarded as detrimental to indexes/memory under high
>>>> load and large DB sizes - compared to int/bigint)? Or do PG
>>>> users using some standard or server-specific (RETURNING) SQL
>>>> clause?
>>>
>>> either create the key ahead of time select nextval('sequence')
>>> and insert it explicitly, or insert the row and  then  select
>>> currval('sequence')
>>>
>> That makes sense; the sequence is retrieved and it internally
>> increments - regardless of whether the key was actually inserted
>> or not. I'm personally not used to this though, it allows for
>> actual keys in the database to possibly have gaps (if the key want
>> actually used / rollback etc). Thats trivial / innocuous I guess,
>> but I'm just used to having sequential keys tables. Would this
>> require two trips to the server, or can we handle in one
>> excecuteUpdate?
>> My real question is, what about the case where multiple VALUES are
>> inserted; if I have 3 values should I call the sequence 3 times?
>> What is the most efficient was to do that? (Can I do it in a
>> single query?)
>
> I don't think you should use "currval" or "nextval" at all. A
> general solution in the JDBC driver should even work in the case of
> triggers that interfere with the value of a sequence. Or which
> might change the value actually inserted into the table. Just think
> of an insert trigger that uses a sequence for a second time.
>
> There is only one way to reliably get the database generated
> values: the RETURNING clause.
>
> So my basic suggestion was to rewrite a query written as:
> "INSERT INTO tab VALUES (...)"
> into
> "INSERT INTO tab VALUES (...) RETURNING x"
>
> With x being either (a) what the user specified using the Java API
> (i.e. any column names) or (b) the primary key column(s) (or other
> columns having a "DEFAULT currval(...)").
> The second case (b) I would leave for later, since it requires
> parsing the query and finding the table which will be inserted
> into. And you would have to use database meta data to find the
> columns to return.
>
Yes, agreed, Ken was just curious how it is being done now.
> Of course, there should be a minimum amount of parsing to detect if
> the query is a valid INSERT query and does not already have a
> different RETURNING clause.
>
> Another option would be to convince backend developers to add a way
> to specify a "RETURNING clause" on the protocol level, i.e. without
> having to change the query string.

Yes, this would be the best solution.
> Best Regards
> Michael Paesold
>
>
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 6: explain analyze is your friend
>

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Michael Paesold

Date:

22 January 2007, 12:38:06

Dave Cramer schrieb:
>
> On 22-Jan-07, at 4:16 AM, Michael Paesold wrote:
>
>> Ken Johanson wrote:
>>>>> As an aside, how do PG jdbc users get the server generated keys? Or
>>>>> does everyone use some kind of UUID system (which I think is
>>>>> generally regarded as detrimental to indexes/memory under high load
>>>>> and large DB sizes - compared to int/bigint)? Or do PG users using
>>>>> some standard or server-specific (RETURNING) SQL clause?
>>>>
>>>> either create the key ahead of time select nextval('sequence') and
>>>> insert it explicitly, or insert the row and  then  select
>>>> currval('sequence')
>>>>
>>> That makes sense; the sequence is retrieved and it internally
>>> increments - regardless of whether the key was actually inserted or
>>> not. I'm personally not used to this though, it allows for actual
>>> keys in the database to possibly have gaps (if the key want actually
>>> used / rollback etc). Thats trivial / innocuous I guess, but I'm just
>>> used to having sequential keys tables. Would this require two trips
>>> to the server, or can we handle in one excecuteUpdate?
>>> My real question is, what about the case where multiple VALUES are
>>> inserted; if I have 3 values should I call the sequence 3 times? What
>>> is the most efficient was to do that? (Can I do it in a single query?)
>>
>> I don't think you should use "currval" or "nextval" at all. A general
>> solution in the JDBC driver should even work in the case of triggers
>> that interfere with the value of a sequence. Or which might change the
>> value actually inserted into the table. Just think of an insert
>> trigger that uses a sequence for a second time.
>>
>> There is only one way to reliably get the database generated values:
>> the RETURNING clause.
>>
>> So my basic suggestion was to rewrite a query written as:
>> "INSERT INTO tab VALUES (...)"
>> into
>> "INSERT INTO tab VALUES (...) RETURNING x"
>>
>> With x being either (a) what the user specified using the Java API
>> (i.e. any column names) or (b) the primary key column(s) (or other
>> columns having a "DEFAULT currval(...)").
>> The second case (b) I would leave for later, since it requires parsing
>> the query and finding the table which will be inserted into. And you
>> would have to use database meta data to find the columns to return.
>>
> Yes, agreed, Ken was just curious how it is being done now.

You are right, sorry. I wanted to be sure that he did not try to do the
same thing for the JDBC driver.

Best Regards
Michael Paesold

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Vit Timchishin

Date:

22 January 2007, 13:20:09

Hello.

I have few thoughts I'd like to share. Everything below is my IMHO.
First of all, parsing input DDL (or requiring protocol support) is
needed to get full support for all range of DDLS. But now PostgreSQL
does not support this feature for any DLLs.
Then you can always detect is the support is needed on the first user
call - depending on which function is called to prepare/execute the
statement.
Why don't you want to do the next:
For

PreparedStatement <http://java.sun.com/javase/6/docs/api/java/sql/PreparedStatement.html> *prepareStatement*(String
<http://java.sun.com/javase/6/docs/api/java/lang/String.html>sql, 
                                   int[] columnIndexes)
                                   throws SQLException
<http://java.sun.com/javase/6/docs/api/java/sql/SQLException.html>

(ans similar Statement call)
simply call sql + " RETURNING *"
and then filter out column indexes
Of course, this won't handle the case when statement already contains
RETURNING, cases for non-INSERT and so on (I am not an expert to
describe all possible cases).
But now NO cases are working and this change will make some work
properly and I, personally, don't see a problem in others giving some
other error message (say, incorrect SQL text for statements already
containing RETURNING) then they are doing now.
When the protocol will be changed (extended), you can use that but what
does prevent you to implement the most widely used (as for me) case,
given it will be implemented properly?

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

23 January 2007, 06:01:59

>>>> My real question is, what about the case where multiple VALUES are
>>>> inserted; if I have 3 values should I call the sequence 3 times?
>>>> What is the most efficient was to do that? (Can I do it in a single
>>>> query?)
>>>
>>> I don't think you should use "currval" or "nextval" at all. A general
>>> solution in the JDBC driver should even work in the case of triggers
>>> that interfere with the value of a sequence. Or which might change
>>> the value actually inserted into the table. Just think of an insert
>>> trigger that uses a sequence for a second time.
>>>
>>> There is only one way to reliably get the database generated values:
>>> the RETURNING clause.
>>>
>>> So my basic suggestion was to rewrite a query written as:
>>> "INSERT INTO tab VALUES (...)"
>>> into
>>> "INSERT INTO tab VALUES (...) RETURNING x"
>>>
>>> With x being either (a) what the user specified using the Java API
>>> (i.e. any column names) or (b) the primary key column(s) (or other
>>> columns having a "DEFAULT currval(...)").
>>> The second case (b) I would leave for later, since it requires
>>> parsing the query and finding the table which will be inserted into.
>>> And you would have to use database meta data to find the columns to
>>> return.
>>>


I think that, given everyone's input (including Vit's, thanks) and
mention of possible variation on query, possible need to parse for
table/column names, and/or need to call database metadata / or result
set metadata (to get keys?) (which require another trip to the
server?)... this might be out of my league. Well, even if I did get it
working, it likely would not work in every case (triggers etc), and
would eventually be replaced when V4 protocol comes around.

Unless one of the PG folks can prescribe, in exact terms, the very best
way to execute this (after which I would build out the actual patch)...
then I may have to bow out of this (it's complex / error prone enough to
frighten lil'ol me, and time is a bit short on my end too I'm afraid).

Perhaps it's better for everyone if we lobby to have the
backend/protocol to add this natively (as you all have suggested). So..

Does anyone know if the actual server core natively has the ability to
build created-keys resultsets (without having to modify the query /
RETURNS), or is this truly a protocl bottleneck?...

Thanks,
Ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Dave Cramer

Date:

23 January 2007, 10:32:39

On 23-Jan-07, at 2:00 AM, Ken Johanson wrote:

>>>>> My real question is, what about the case where multiple VALUES
>>>>> are inserted; if I have 3 values should I call the sequence 3
>>>>> times? What is the most efficient was to do that? (Can I do it
>>>>> in a single query?)
>>>>
>>>> I don't think you should use "currval" or "nextval" at all. A
>>>> general solution in the JDBC driver should even work in the case
>>>> of triggers that interfere with the value of a sequence. Or
>>>> which might change the value actually inserted into the table.
>>>> Just think of an insert trigger that uses a sequence for a
>>>> second time.
>>>>
>>>> There is only one way to reliably get the database generated
>>>> values: the RETURNING clause.
>>>>
>>>> So my basic suggestion was to rewrite a query written as:
>>>> "INSERT INTO tab VALUES (...)"
>>>> into
>>>> "INSERT INTO tab VALUES (...) RETURNING x"
>>>>
>>>> With x being either (a) what the user specified using the Java
>>>> API (i.e. any column names) or (b) the primary key column(s) (or
>>>> other columns having a "DEFAULT currval(...)").
>>>> The second case (b) I would leave for later, since it requires
>>>> parsing the query and finding the table which will be inserted
>>>> into. And you would have to use database meta data to find the
>>>> columns to return.
>>>>
>
>
> I think that, given everyone's input (including Vit's, thanks) and
> mention of possible variation on query, possible need to parse for
> table/column names, and/or need to call database metadata / or
> result set metadata (to get keys?) (which require another trip to
> the server?)... this might be out of my league. Well, even if I did
> get it working, it likely would not work in every case (triggers
> etc), and would eventually be replaced when V4 protocol comes around.
>
> Unless one of the PG folks can prescribe, in exact terms, the very
> best way to execute this (after which I would build out the actual
> patch)... then I may have to bow out of this (it's complex / error
> prone enough to frighten lil'ol me, and time is a bit short on my
> end too I'm afraid).

If we can implement the one which does specify the keys that would be
useful. Statement.getGeneratedKeys( columns )
>
> Perhaps it's better for everyone if we lobby to have the backend/
> protocol to add this natively (as you all have suggested). So..
>
> Does anyone know if the actual server core natively has the ability
> to build created-keys resultsets (without having to modify the
> query / RETURNS), or is this truly a protocl bottleneck?...
>
I don't know for absolute certainty but I highly suspect that it
does, since it can return the columns returned via insert returning

Dave
> Thanks,
> Ken
>
>
>

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

26 January 2007, 02:54:46

In the RETURNING clause that I'll be appending to the query, to get
gen'd keys, anyone have any advise for best practice for when to quote
the column names? Are there any pre-built util methods in the driver
that I can use to scan for identifiers that need quoting? (whitespace
char, reserved words, etc)?

Thanks,

Ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

26 January 2007, 03:43:04

In implementing Statement.executeUpdate(String sql,String[]
columnNames), does any know how I could go about sending the sql and
subsequent RETURNING clause directly to a stream? Just so I can avoid
double-buffering the query (allocated in the sql, and second in the
StringBuffer I'd be appending the RETURNING clause to). I do see that
there are ASCII and Unicode char streams and I'm also  not sure how to
delegate to those or if a highr level method (network) would do that..

gratsi,

ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

26 January 2007, 06:12:29

http://onnet.cc/AbstractJdbc3Statement.patch
http://onnet.cc/AbstractJdbc2Statement.patch

For some reason I'm getting an empty result set with this.. the javadocs
are a tad sparse so I may not be able to resolve this for a day or two..
anyone better at with this api? I'm also not sure if this quick and
dirty change will correctly handle PreparedStmts.


ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Dave Cramer

Date:

26 January 2007, 11:52:04

Ken,

Do you have a test case ? What do the server logs show ?

Dave
On 26-Jan-07, at 2:12 AM, Ken Johanson wrote:

> http://onnet.cc/AbstractJdbc3Statement.patch
> http://onnet.cc/AbstractJdbc2Statement.patch
>
> For some reason I'm getting an empty result set with this.. the
> javadocs are a tad sparse so I may not be able to resolve this for
> a day or two.. anyone better at with this api? I'm also not sure if
> this quick and dirty change will correctly handle PreparedStmts.
>
>
> ken
>
>
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
>       choose an index scan if your joining column's datatypes do not
>       match
>

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

27 January 2007, 03:33:58

Dave Cramer wrote:
> Ken,
>
> Do you have a test case ? What do the server logs show ?
>

Hi Dave,

My test case is:

String url = "jdbc:postgresql://127.0.0.1:5432/test?stringtype=unspecified";
String clz = "org.postgresql.Driver";
String dml = "insert into contact (rowid,logonname) values (DEFAULT,'bar')";
String[] keys = {"rowid"};
Connection con = DriverManager.getConnection(url, "user", "pass");
Statement stmt = con.createStatement();
int affected = stmt.executeUpdate(dml, keys);
ResultSet rs = stmt.getGeneratedKeys();
if (rs.next())
    out.println(rs.getObject(1));
else
    out.println("NO ROWS, INSERTED "+affected);

A ResultSet is returned but has no rows. The insert does succeed and a
sequence generated key increments in the 'rowid' column.

I think the issue is just that my code is not calling through the
correct private methods that would populate a result.

Also I noticed that so far my mods have not implemented
Stmt.NO_GENERATED_KEYS & RETURN_GENERATED_KEYS;

On a related note, do you know how I could call though a Stream method
instead of the String/Charsequence execs?

Thanks,
Ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Kris Jurka

Date:

27 January 2007, 03:51:01

On Thu, 25 Jan 2007, Ken Johanson wrote:

> In implementing Statement.executeUpdate(String sql,String[] columnNames),
> does any know how I could go about sending the sql and subsequent RETURNING
> clause directly to a stream? Just so I can avoid double-buffering the query
> (allocated in the sql, and second in the StringBuffer I'd be appending the
> RETURNING clause to).

Don't worry about this minor overhead.  So much other manipulation and
object creation happens (even forgetting the network trip) that this will
be in the noise.

> I do see that there are ASCII and Unicode char streams
> and I'm also  not sure how to delegate to those or if a highr level method
> (network) would do that..
>

These are for sending data (parameters) to the server for things like
large text fields.

Kris Jurka

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Kris Jurka

Date:

27 January 2007, 03:58:03

On Fri, 26 Jan 2007, Ken Johanson wrote:

> http://onnet.cc/AbstractJdbc3Statement.patch
> http://onnet.cc/AbstractJdbc2Statement.patch

I can't get the first patch to apply cleanly at all, erroring with

patching file AbstractJdbc3Statement.java
patch: **** Premature `---' at line 33; check line numbers at line 21

And even to get to that point I had to change from windows \ to unix /
directory separators.  Could we get a patch that is easy to apply?

> For some reason I'm getting an empty result set with this.. the javadocs are
> a tad sparse so I may not be able to resolve this for a day or two.. anyone
> better at with this api? I'm also not sure if this quick and dirty change
> will correctly handle PreparedStmts.
>

I wouldn't be surprised if the driver is confused by an INSERT command
completion tag also having a result set.  If you can produce a working
patch I'll take a closer look.

Kris Jurka

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

28 January 2007, 14:51:27

>> In implementing Statement.executeUpdate(String sql,String[]
>> columnNames), does any know how I could go about sending the sql and
>> subsequent RETURNING clause directly to a stream? Just so I can avoid
>> double-buffering the query (allocated in the sql, and second in the
>> StringBuffer I'd be appending the RETURNING clause to).
>
> Don't worry about this minor overhead.  So much other manipulation and
> object creation happens (even forgetting the network trip) that this
> will be in the noise.
>


Hmm, this might be an area I'd enjoy contributing code for.. if there
are places now where there is avoidable double or triple allocation
(like in my patch) (where it could instead be handled by streams and/or
finer grained chunks), and since the driver (apparently) already has
some stream based utils... maybe I could try to improve those.

I know from experience with other tools that unnec allocation can play
havoc under high load and/or very large data sets, using (resizeable)
buffers (like StringBuffer). When that code gets optimized the
application becomes much more responsive.

If you know of any places in the driver that could benefit from this,
please let me know.

ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

30 January 2007, 03:38:06

Hi Kris, were you able to look at this? If time wont permit that I'll
dig back in; though I'd prefer not to duplicate any work your're doing,
of course.

Thank,
ken

Kris Jurka wrote:
>
>
> On Fri, 26 Jan 2007, Ken Johanson wrote:
>
>> http://onnet.cc/AbstractJdbc3Statement.patch
>> http://onnet.cc/AbstractJdbc2Statement.patch
>
> I can't get the first patch to apply cleanly at all, erroring with
>
> patching file AbstractJdbc3Statement.java
> patch: **** Premature `---' at line 33; check line numbers at line 21
>
> And even to get to that point I had to change from windows \ to unix /
> directory separators.  Could we get a patch that is easy to apply?
>
>
>> For some reason I'm getting an empty result set with this.. the
>> javadocs are a tad sparse so I may not be able to resolve this for a
>> day or two.. anyone better at with this api? I'm also not sure if this
>> quick and dirty change will correctly handle PreparedStmts.
>>
>
> I wouldn't be surprised if the driver is confused by an INSERT command
> completion tag also having a result set.  If you can produce a working
> patch I'll take a closer look.
>
> Kris Jurka

Kris,

Strange about the patch. Aside from the import stmts, only the method
below is changed in that source file:

public int executeUpdate(String sql, String columnNames[]) throws
SQLException
{
   //fix-me "The driver will ignore the array if the SQL statement is
not an INSERT statement"
   //fix me : impl NO_GENERATED_KEYS & RETURN_GENERATED_KEYS
   if (columnNames==null || columnNames.length==0)
          return executeUpdate(sql);
   //should never reallocate
   StringBuffer s = new
StringBuffer(12+sql.length()+(columnNames.length*32));
   s.append(sql);
   s.append('\n');
   s.append("RETURNING");
   s.append(' ');
   for (int i=0; i<columnNames.length; i++)
   {
       if (i!=0)
           s.append(',');
       s.append(columnNames[i]);
   }
   return executeUpdateGetResults(s.toString());
      //throw new PSQLException(GT.tr("Returning autogenerated keys is
not supported."), PSQLState.NOT_IMPLEMENTED);
}

ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Kris Jurka

Date:

01 February 2007, 17:26:56

On Mon, 29 Jan 2007, Ken Johanson wrote:

> Hi Kris, were you able to look at this? If time wont permit that I'll dig
> back in; though I'd prefer not to duplicate any work your're doing, of
> course.

Taking another look the obvious problem is that you haven't touched
AbstractJdbc3Statement.getGeneratedKeys.  It still reads:

     public ResultSet getGeneratedKeys() throws SQLException
     {
         return createDriverResultSet(new Field[0], new Vector());
     }

So an empty ResultSet is not surprising.

Kris Jurka

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

13 February 2007, 22:38:34

Kris Jurka wrote:
>
>
> On Mon, 29 Jan 2007, Ken Johanson wrote:
>
>> Hi Kris, were you able to look at this? If time wont permit that I'll
>> dig back in; though I'd prefer not to duplicate any work your're
>> doing, of course.
>
> Taking another look the obvious problem is that you haven't touched
> AbstractJdbc3Statement.getGeneratedKeys.  It still reads:
>
>     public ResultSet getGeneratedKeys() throws SQLException
>     {
>         return createDriverResultSet(new Field[0], new Vector());
>     }
>
> So an empty ResultSet is not surprising.
>
> Kris Jurka

Kris, sorry my response is late. Sometimes I get distracted by life :-)

Thanks for pointing that out. I missed the obvious. The following
replacement for that methods works; I can get the generated keys (in
conjunction with my previous patches).

I have not tested this exhaustively, nor given though about the
correctness of the null-normalization below; is this enough for you or
someone to commit the changes?

Thanks,
Ken

     public ResultSet getGeneratedKeys() throws SQLException
     {
         return result==null ?
            createDriverResultSet(new Field[0], new Vector())
             : result.getResultSet();
     }

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Kris Jurka

Date:

14 February 2007, 17:04:57

On Tue, 13 Feb 2007, Ken Johanson wrote:

> I have not tested this exhaustively, nor given though about the correctness
> of the null-normalization below; is this enough for you or someone to commit
> the changes?
>

No, you've provided the sketch of a solution, but it's not appropriate for
the driver until you've resolved things like quoting column names, putting
in a version check for a server that supports RETURNING, and added some
test cases.  Some or all of this can be done by the committer, but the
more work the committer must do the less likely it's going to happen.

Kris Jurka

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

15 February 2007, 03:31:54

Kris Jurka wrote:
>
>
> On Tue, 13 Feb 2007, Ken Johanson wrote:
>
>> I have not tested this exhaustively, nor given though about the
>> correctness of the null-normalization below; is this enough for you or
>> someone to commit the changes?
>>
>
> No, you've provided the sketch of a solution, but it's not appropriate
> for the driver until you've resolved things like quoting column names,
> putting in a version check for a server that supports RETURNING, and
> added some test cases.  Some or all of this can be done by the
> committer, but the more work the committer must do the less likely it's
> going to happen.
>
> Kris Jurka
>

This patch adds server version checking and only basic quoting to
AbstractJdbc3Statement. No test cases.

It only checks for server version 8 or newer as I couldn't easily find
the threshold for the RETURNING support. It also only quotes the key
array if it contains spaces.

Doe anyone know if there is a scanner method to check for quotable
identifiers? Did server 8 first implement RETURNING?

What other concerns should I apply to this?

Thanks,
ken
# This patch file was generated by NetBeans IDE
# Following Index: paths are relative to: C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc3
# This patch can be applied using context Tools: Patch action on respective folder.
# It uses platform neutral UTF-8 encoding and \n newlines.
# Above lines and this line are ignored by the patching process.
Index: AbstractJdbc3Statement.java
*** C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc3\AbstractJdbc3Statement.java Base (1.21)
--- C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc3\AbstractJdbc3Statement.java Locally Modified (Based On 1.21)
***************
*** 19,24 ****
--- 19,26 ----
  import org.postgresql.core.QueryExecutor;
  import org.postgresql.core.Field;
  import org.postgresql.core.BaseConnection;
+ import org.postgresql.jdbc2.AbstractJdbc2Connection;
+ import org.postgresql.jdbc2.AbstractJdbc2Statement.StatementResultHandler;
  import org.postgresql.util.GT;

  /**
***************
*** 106,112 ****
       */
      public ResultSet getGeneratedKeys() throws SQLException
      {
!         return createDriverResultSet(new Field[0], new Vector());
      }

      /**
--- 108,116 ----
       */
      public ResultSet getGeneratedKeys() throws SQLException
      {
!         return result==null ?
!             createDriverResultSet(new Field[0], new Vector())
!             : result.getResultSet();
      }

      /**
***************
*** 135,141 ****
      {
          if (autoGeneratedKeys == Statement.NO_GENERATED_KEYS)
              return executeUpdate(sql);
!
          throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
      }

--- 139,145 ----
      {
          if (autoGeneratedKeys == Statement.NO_GENERATED_KEYS)
              return executeUpdate(sql);
!         //fix me : impl NO_GENERATED_KEYS & RETURN_GENERATED_KEYS
          throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
      }

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

      /**
       * Executes the given SQL statement, which may return multiple results,
--- 188,230 ----
       */
      public int executeUpdate(String sql, String columnNames[]) throws SQLException
      {
!         //fix me : columnNames only quoted if contain 0x20
!         //fix me : if super changes, will break (need interface to get server verion):
!         String prefix = sql.substring(0,10).toLowerCase();
!         if (prefix.indexOf("insert")==-1)
!             return executeUpdateGetResults(sql);
!         if (!(connection instanceof AbstractJdbc2Connection))
!         {
!             throw new PSQLException(GT.tr("Driver version does not support returning generated keys.")+"
"+connection.getClass().getName(),PSQLState.NOT_IMPLEMENTED); 
!         }
!         AbstractJdbc2Connection con = (AbstractJdbc2Connection)connection;
!         int major = con.getServerMajorVersion();
!         if (major<8)
!             throw new PSQLException(GT.tr("Server version does not support returning generated keys.")+" ("+major+" <
"+8+")",PSQLState.NOT_IMPLEMENTED); 
!         if (columnNames==null || columnNames.length==0)
              return executeUpdate(sql);
!         StringBuffer s = new StringBuffer(sql.length()+(columnNames.length*32));
!         s.append(sql);
!         s.append('\n');
!         s.append("RETURNING");
!         s.append(' ');
!         boolean needsQuote;
!         for (int i=0; i<columnNames.length; i++)
!         {
!             if (i!=0)
!                 s.append(',');
!             needsQuote = columnNames[i].indexOf(' ')!=-1;
!             if (needsQuote)
!                 s.append('"');
!             s.append(columnNames[i]);
!             if (needsQuote)
!                 s.append('"');
          }
+         return executeUpdateGetResults(s.toString());
+         //throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."),
PSQLState.NOT_IMPLEMENTED);
+     }

+
      /**
       * Executes the given SQL statement, which may return multiple results,
       * and signals the driver that any

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Vit Timchishin

Date:

15 February 2007, 09:13:56

Ken Johanson wrote:
> Kris Jurka wrote:
>>
>>
>> On Tue, 13 Feb 2007, Ken Johanson wrote:
>>
>>> I have not tested this exhaustively, nor given though about the
>>> correctness of the null-normalization below; is this enough for you
>>> or someone to commit the changes?
>>>
>>
>> No, you've provided the sketch of a solution, but it's not
>> appropriate for the driver until you've resolved things like quoting
>> column names, putting in a version check for a server that supports
>> RETURNING, and added some test cases.  Some or all of this can be
>> done by the committer, but the more work the committer must do the
>> less likely it's going to happen.
>>
>> Kris Jurka
>>
>
> This patch adds server version checking and only basic quoting to
> AbstractJdbc3Statement. No test cases.
>
> It only checks for server version 8 or newer as I couldn't easily find
> the threshold for the RETURNING support.

8.1.5 does not have it. I suppose it is 8.2 feature. You can check by
looking into 8.1 and 8.2 documentation, and, I suppose, "what's new"
for  8.2

Re: Synthesize support for Statement.getGeneratedKeys()?

From

"Albe Laurenz"

Date:

15 February 2007, 10:16:20

>> It only checks for server version 8 or newer as I couldn't easily
find
>> the threshold for the RETURNING support.
>
> 8.1.5 does not have it. I suppose it is 8.2 feature. You can check by
> looking into 8.1 and 8.2 documentation, and, I suppose, "what's new"
> for  8.2

Yes, it is new in 8.2.

Yours,
Laurenz Albe

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

15 February 2007, 22:40:33

This patch adds the Major and Minor version checking, for server >=8.2.

If there is a more correct method of checking for quote-able
identifiers, or anything else I'm missing, please let me know.

Thanks,
Ken
# This patch file was generated by NetBeans IDE
# Following Index: paths are relative to: C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc3
# This patch can be applied using context Tools: Patch action on respective folder.
# It uses platform neutral UTF-8 encoding and \n newlines.
# Above lines and this line are ignored by the patching process.
Index: AbstractJdbc3Statement.java
*** C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc3\AbstractJdbc3Statement.java Base (1.21)
--- C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc3\AbstractJdbc3Statement.java Locally Modified (Based On 1.21)
***************
*** 19,24 ****
--- 19,26 ----
  import org.postgresql.core.QueryExecutor;
  import org.postgresql.core.Field;
  import org.postgresql.core.BaseConnection;
+ import org.postgresql.jdbc2.AbstractJdbc2Connection;
+ import org.postgresql.jdbc2.AbstractJdbc2Statement.StatementResultHandler;
  import org.postgresql.util.GT;

  /**
***************
*** 28,33 ****
--- 30,38 ----
   */
  public abstract class AbstractJdbc3Statement extends org.postgresql.jdbc2.AbstractJdbc2Statement
  {
+     private static final int SUPPORTED_RETURNING_MAJOR = 8;
+     private static final int SUPPORTED_RETURNING_MINOR = 8;
+
      private final int rsHoldability;

      public AbstractJdbc3Statement (AbstractJdbc3Connection c, int rsType, int rsConcurrency, int rsHoldability)
throwsSQLException 
***************
*** 106,112 ****
       */
      public ResultSet getGeneratedKeys() throws SQLException
      {
!         return createDriverResultSet(new Field[0], new Vector());
      }

      /**
--- 111,119 ----
       */
      public ResultSet getGeneratedKeys() throws SQLException
      {
!         return result==null ?
!             createDriverResultSet(new Field[0], new Vector())
!             : result.getResultSet();
      }

      /**
***************
*** 135,141 ****
      {
          if (autoGeneratedKeys == Statement.NO_GENERATED_KEYS)
              return executeUpdate(sql);
!
          throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
      }

--- 142,148 ----
      {
          if (autoGeneratedKeys == Statement.NO_GENERATED_KEYS)
              return executeUpdate(sql);
!         //fix me : impl NO_GENERATED_KEYS & RETURN_GENERATED_KEYS
          throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
      }

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

      /**
       * Executes the given SQL statement, which may return multiple results,
--- 191,235 ----
       */
      public int executeUpdate(String sql, String columnNames[]) throws SQLException
      {
!         //fix me : columnNames only quoted if contain 0x20
!         //fix me : if super changes, will break (need interface to get server verion):
!         String prefix = sql.substring(0,10).toLowerCase();
!         if (columnNames==null || prefix.indexOf("insert")==-1)
!             return executeUpdateGetResults(sql);
!         if (!(connection instanceof AbstractJdbc2Connection))
!         {
!             throw new PSQLException(GT.tr("Driver version does not support returning generated keys.")+"
"+connection.getClass().getName(),PSQLState.NOT_IMPLEMENTED); 
!         }
!         AbstractJdbc2Connection con = (AbstractJdbc2Connection)connection;
!         int args = columnNames.length;
!         int major = con.getServerMajorVersion();
!         int minor = con.getServerMinorVersion();
!         if (major<SUPPORTED_RETURNING_MAJOR && minor<SUPPORTED_RETURNING_MINOR)
!             throw new PSQLException(GT.tr("Server version does not support returning generated keys.")+"
("+major+"."+minor+"< "+SUPPORTED_RETURNING_MAJOR+"."+SUPPORTED_RETURNING_MINOR+")", PSQLState.NOT_IMPLEMENTED); 
!         if (args==0)
              return executeUpdate(sql);
!         StringBuffer s = new StringBuffer(sql.length()+(args*32));
!         s.append(sql);
!         s.append('\n');
!         s.append("RETURNING");
!         s.append(' ');
!         boolean needsQuote;
!         for (int i=0; i<args; i++)
!         {
!             if (i!=0)
!                 s.append(',');
!             needsQuote = columnNames[i].indexOf(' ')!=-1;
!             if (needsQuote)
!                 s.append('"');
!             s.append(columnNames[i]);
!             if (needsQuote)
!                 s.append('"');
          }
+         return executeUpdateGetResults(s.toString());
+         //throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."),
PSQLState.NOT_IMPLEMENTED);
+     }

+
      /**
       * Executes the given SQL statement, which may return multiple results,
       * and signals the driver that any

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Kris Jurka

Date:

16 February 2007, 02:33:07

Ken Johanson wrote:
> Do you know what built-in method I can choose to determine what
> identifiers need quoting?
>

There isn't one, but I was thinking about it some more and I think we
might want to quote everything.  Otherwise we won't be able to
distinguish between columns that are created with quotes and those
without.  Consider a table like:

CREATE TABLE tab(a int, "A" int);

Without parsing the query and doing some metadata lookups we won't know
what the columns are.  I'd rather not try to do this and the behavior
matches our existing handling of DatabaseMetaData methods.  Although we
might get some complaints, I'm OK with it.

Right now the only place we quote identifiers in
jdbc3/PSQLSavepoint#getPGName.  I would suggest creating a method in
org.postgresql.core.Utils named appendEscapedIdentifier that works like
appendEscapedString and using it in both places.

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Kris Jurka

Date:

16 February 2007, 02:34:31

On Thu, 15 Feb 2007, Ken Johanson wrote:

> This patch adds the Major and Minor version checking, for server >=8.2.
>

The correct version check should be written:
if (connection.haveMinimumServerVersion("8.2"))

I'm also not sure about the getGeneratedKeys method.  Should we throw an
Exception instead of returning an empty ResultSet if there weren't any
generated keys?  Also do we need to do better tracking of what comes from
generated keys vs just regular results?  Once people start using this
functionality we'll need some better error checking.

Kris Jurka

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

16 February 2007, 03:31:00

Kris Jurka wrote:
>
> The correct version check should be written:
> if (connection.haveMinimumServerVersion("8.2"))
>

I will correct this.

> I'm also not sure about the getGeneratedKeys method.  Should we throw an
> Exception instead of returning an empty ResultSet if there weren't any
> generated keys?

I checked and the spec says:
"If this Statement object did not generate any keys, an empty ResultSet
  object is returned."
If there is an implied rules elsewhere that it is not this simple, I
dont know. Otherwise it seems correct.

   Also do we need to do better tracking of what comes
> from generated keys vs just regular results?

I believe you are correct; if that method is called not-after
executeUpdate(String sql, ?), it should throw and exception.. or
something... the spec does not seem to elaborate on this.

   Once people start using
> this functionality we'll need some better error checking.

Agreed in full.

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

16 February 2007, 03:39:52

Kris Jurka wrote:
>
>
> Right now the only place we quote identifiers in
> jdbc3/PSQLSavepoint#getPGName.  I would suggest creating a method in
> org.postgresql.core.Utils named appendEscapedIdentifier that works like
> appendEscapedString and using it in both places.
>


Would you care to code this up; I think you have a clearly understanding
of how it should work.

Thanks,
Ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Kris Jurka

Date:

02 April 2007, 06:57:04


On Thu, 15 Feb 2007, Ken Johanson wrote:

> Kris Jurka wrote:
>>
>> Right now the only place we quote identifiers in
>> jdbc3/PSQLSavepoint#getPGName.  I would suggest creating a method in
>> org.postgresql.core.Utils named appendEscapedIdentifier that works like
>> appendEscapedString and using it in both places.
>
> Would you care to code this up; I think you have a clearly understanding of
> how it should work.
>

I have added the appendEscapedIdentifier method discussed above.

Kris Jurka

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

06 December 2007, 05:57:19

> The previous discussion does detail the remaining steps needed to get
> something ready to be committed: proper quoting, error checking, test
> cases, ...  If you have the time and skill to work on these that would
> be appreciated.
>


Kris (and folks), I did actually find some justification for the time to
work on this. The attached patch should follow on from our last state in
Feb. Essentially the only thing I added was error checking of the
arguments to 'executeUpdate's columnNames. The checking is done in a new
Utils.needsQuoted(String in) (see javadoc) (feel free to rename this
method).

I opted to use the Quoting mechanism I already had in executeUpdate for
now, since the string validation (no 0x00 && no nested quotes) is being
done in needsQuoted (in the same loop that validates quotes and scans
for whitespace).

Let me know in detail what else needs to be implemented in terms of
error checking or methods. I did not add any unit-tests (that will be a
learning curve; seeking volunteers).

Questions:

-is whitespace the sole determinator for needing quoting? And other chars?

-is it fine to leave the string un-quoted if it contains no ws, vs
always quoting it (my feeling is yes).

-is '"' the only legal quoting chars? (I cant remember for having
dabbled with too many non-spec databases)

-my needsQuoted method throws if the identifier contains nested quotes
(foo"bar or "foo"bar"); is there a legal quote-escaping mechanism
similar to apostrophe doubling? eg: how or would one pass foo"bar (I
imagine quotes are never allowed in identifiers but don't have an SQL
spec handy)

Ken

PS - Kris, I recall you said the backslashes in the patch were
troublesome; did you find any fix for your patch tool aside from
translating them to '/'? If not I will translate them from hereto forth.
# This patch file was generated by NetBeans IDE
# Following Index: paths are relative to: C:\dev\java\proj\pgjdbc\pgjdbc
# This patch can be applied using context Tools: Patch action on respective folder.
# It uses platform neutral UTF-8 encoding and \n newlines.
# Above lines and this line are ignored by the patching process.
Index: org/postgresql/jdbc2/AbstractJdbc2Statement.java
*** C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc2\AbstractJdbc2Statement.java Base (1.104)
--- C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc2\AbstractJdbc2Statement.java Locally Modified (Based On 1.104)
***************
*** 286,291 ****
--- 286,318 ----
      }

      /*
+      * Execute a SQL INSERT, UPDATE or DELETE statement.  In addition
+      * SQL statements that return nothing such as SQL DDL statements
+      * can be executed
+      *
+      * @param sql a SQL statement
+      * @return either a row count, or 0 for SQL commands
+      * @exception SQLException if a database access error occurs
+      */
+     protected int executeUpdateGetResults(String p_sql) throws SQLException
+     {
+         if (preparedQuery != null)
+             throw new PSQLException(GT.tr("Can''t use query methods that take a query string on a
PreparedStatement."),
+                                     PSQLState.WRONG_OBJECT_TYPE);
+         if( isFunction )
+         {
+             executeWithFlags(p_sql, 0);
+             return 0;
+         }
+         checkClosed();
+         p_sql = replaceProcessing(p_sql);
+         Query simpleQuery = connection.getQueryExecutor().createSimpleQuery(p_sql);
+         execute(simpleQuery, null, 0);
+         this.lastSimpleQuery = simpleQuery;
+         return getUpdateCount();
+     }
+
+     /*
       * Execute a SQL INSERT, UPDATE or DELETE statement.  In addition,
       * SQL statements that return nothing such as SQL DDL statements can
       * be executed.
Index: org/postgresql/jdbc3/AbstractJdbc3Statement.java
*** C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc3\AbstractJdbc3Statement.java Base (1.21)
--- C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc3\AbstractJdbc3Statement.java Locally Modified (Based On 1.21)
***************
*** 19,24 ****
--- 19,27 ----
  import org.postgresql.core.QueryExecutor;
  import org.postgresql.core.Field;
  import org.postgresql.core.BaseConnection;
+ import org.postgresql.core.Utils;
+ import org.postgresql.jdbc2.AbstractJdbc2Connection;
+ import org.postgresql.jdbc2.AbstractJdbc2Statement.StatementResultHandler;
  import org.postgresql.util.GT;

  /**
***************
*** 28,33 ****
--- 31,37 ----
   */
  public abstract class AbstractJdbc3Statement extends org.postgresql.jdbc2.AbstractJdbc2Statement
  {
+
      private final int rsHoldability;

      public AbstractJdbc3Statement (AbstractJdbc3Connection c, int rsType, int rsConcurrency, int rsHoldability)
throwsSQLException 
***************
*** 106,112 ****
       */
      public ResultSet getGeneratedKeys() throws SQLException
      {
!         return createDriverResultSet(new Field[0], new Vector());
      }

      /**
--- 110,118 ----
       */
      public ResultSet getGeneratedKeys() throws SQLException
      {
!         return result==null ?
!             createDriverResultSet(new Field[0], new Vector())
!             : result.getResultSet();
      }

      /**
***************
*** 135,141 ****
      {
          if (autoGeneratedKeys == Statement.NO_GENERATED_KEYS)
              return executeUpdate(sql);
!
          throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
      }

--- 141,147 ----
      {
          if (autoGeneratedKeys == Statement.NO_GENERATED_KEYS)
              return executeUpdate(sql);
!         //fix me : impl NO_GENERATED_KEYS & RETURN_GENERATED_KEYS
          throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
      }

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

      /**
       * Executes the given SQL statement, which may return multiple results,
--- 190,236 ----
       */
      public int executeUpdate(String sql, String columnNames[]) throws SQLException
      {
!         //fix me : columnNames only quoted if contain 0x20
!         String prefix = sql.substring(0,10).toLowerCase();
!         if (columnNames==null || prefix.indexOf("insert")==-1)
!             return executeUpdateGetResults(sql);
!         if (!(connection instanceof AbstractJdbc2Connection))
!         {
!             throw new PSQLException(GT.tr("Driver version does not support returning generated keys.")+"
"+connection.getClass().getName(),PSQLState.NOT_IMPLEMENTED); 
!         }
!         AbstractJdbc2Connection con = (AbstractJdbc2Connection)connection;
!         int args = columnNames.length;
!         if (!connection.haveMinimumServerVersion("8.2"))
!             throw new PSQLException(GT.tr("Server version does not support returning generated keys.")+" (<
"+"8.2"+")",PSQLState.NOT_IMPLEMENTED); 
!         if (args==0)
              return executeUpdate(sql);
!         StringBuffer s = new StringBuffer(sql.length()+(args*32));
!         s.append(sql);
!         s.append('\n');
!         s.append("RETURNING");
!         s.append(' ');
!         boolean needsQuote;
!         for (int i=0; i<args; i++)
!         {
!             String arg = columnNames[i];
!             if (arg==null)
!                 //throw new NullPointerException("executeUpdate: null columnName at index "+i);
!                 throw new PSQLException(GT.tr("Null value in columnNames"), PSQLState.INVALID_PARAMETER_VALUE);
!             if (i!=0)
!                 s.append(',');
!             needsQuote = Utils.needsQuoted(arg);
!             if (needsQuote)
!                 s.append('"');
!             s.append(arg);
!             if (needsQuote)
!                 s.append('"');
          }
+         return executeUpdateGetResults(s.toString());
+         //throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."),
PSQLState.NOT_IMPLEMENTED);
+     }

+
+
      /**
       * Executes the given SQL statement, which may return multiple results,
       * and signals the driver that any
Index: org/postgresql/core/Utils.java
*** C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\core\Utils.java Base (1.6)
--- C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\core\Utils.java Locally Modified (Based On 1.6)
***************
*** 146,152 ****
--- 146,186 ----

          return sbuf;
      }
+     /**
+      * return true if the string contains whitespace and is not already quoted, false otherwise
+      *
+      * @param in
+      * @return true if the string contains whitespace and is not already quoted
+      * @throws java.sql.SQLException if the string contains quotes inside its value
+      * (foo"bar or "foor"bar"), or contains char 0x00.
+      */
+     public static final boolean needsQuoted(String in) throws SQLException
+     {
+         int len = in.length();
+         //quoted and non-empty quotes:
+         boolean already = len>1 && in.charAt(0)=='"' && in.charAt(len-1)=='"';
+         if (already && len==2)
+             throw new PSQLException(GT.tr("Empty quoted value"), PSQLState.INVALID_PARAMETER_VALUE);
+         int end = len-1;
+         for (int i=1; i<end; i++)
+         {//scan for legal
+             char c = in.charAt(i);
+             if (c=='"')
+                 throw new PSQLException(GT.tr("Invalid quotes found inside argument"),
PSQLState.INVALID_PARAMETER_VALUE);    
+             if (c=='\0')
+                 throw new PSQLException(GT.tr("Null bytes may not occur in identifiers."),
PSQLState.INVALID_PARAMETER_VALUE);
          }
+         for (int i=1; i<end; i++)
+         {
+             char c = in.charAt(i);
+             if (Character.isWhitespace(c))
+                 return !already;
+         }
+         return false;
+     }
+
+ }

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

06 December 2007, 06:07:51

Version with 4-space instead of tabs..
# This patch file was generated by NetBeans IDE
# Following Index: paths are relative to: C:\dev\java\proj\pgjdbc\pgjdbc
# This patch can be applied using context Tools: Patch action on respective folder.
# It uses platform neutral UTF-8 encoding and \n newlines.
# Above lines and this line are ignored by the patching process.
Index: org/postgresql/jdbc2/AbstractJdbc2Statement.java
*** C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc2\AbstractJdbc2Statement.java Base (1.104)
--- C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc2\AbstractJdbc2Statement.java Locally Modified (Based On 1.104)
***************
*** 286,291 ****
--- 286,318 ----
      }

      /*
+      * Execute a SQL INSERT, UPDATE or DELETE statement.  In addition
+      * SQL statements that return nothing such as SQL DDL statements
+      * can be executed
+      *
+      * @param sql a SQL statement
+      * @return either a row count, or 0 for SQL commands
+      * @exception SQLException if a database access error occurs
+      */
+     protected int executeUpdateGetResults(String p_sql) throws SQLException
+     {
+         if (preparedQuery != null)
+             throw new PSQLException(GT.tr("Can''t use query methods that take a query string on a
PreparedStatement."),
+                                     PSQLState.WRONG_OBJECT_TYPE);
+         if( isFunction )
+         {
+             executeWithFlags(p_sql, 0);
+             return 0;
+         }
+         checkClosed();
+         p_sql = replaceProcessing(p_sql);
+         Query simpleQuery = connection.getQueryExecutor().createSimpleQuery(p_sql);
+         execute(simpleQuery, null, 0);
+         this.lastSimpleQuery = simpleQuery;
+         return getUpdateCount();
+     }
+
+     /*
       * Execute a SQL INSERT, UPDATE or DELETE statement.  In addition,
       * SQL statements that return nothing such as SQL DDL statements can
       * be executed.
Index: org/postgresql/jdbc3/AbstractJdbc3Statement.java
*** C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc3\AbstractJdbc3Statement.java Base (1.21)
--- C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc3\AbstractJdbc3Statement.java Locally Modified (Based On 1.21)
***************
*** 19,24 ****
--- 19,27 ----
  import org.postgresql.core.QueryExecutor;
  import org.postgresql.core.Field;
  import org.postgresql.core.BaseConnection;
+ import org.postgresql.core.Utils;
+ import org.postgresql.jdbc2.AbstractJdbc2Connection;
+ import org.postgresql.jdbc2.AbstractJdbc2Statement.StatementResultHandler;
  import org.postgresql.util.GT;

  /**
***************
*** 28,33 ****
--- 31,37 ----
   */
  public abstract class AbstractJdbc3Statement extends org.postgresql.jdbc2.AbstractJdbc2Statement
  {
+
      private final int rsHoldability;

      public AbstractJdbc3Statement (AbstractJdbc3Connection c, int rsType, int rsConcurrency, int rsHoldability)
throwsSQLException 
***************
*** 106,112 ****
       */
      public ResultSet getGeneratedKeys() throws SQLException
      {
!         return createDriverResultSet(new Field[0], new Vector());
      }

      /**
--- 110,118 ----
       */
      public ResultSet getGeneratedKeys() throws SQLException
      {
!         return result==null ?
!             createDriverResultSet(new Field[0], new Vector())
!             : result.getResultSet();
      }

      /**
***************
*** 135,141 ****
      {
          if (autoGeneratedKeys == Statement.NO_GENERATED_KEYS)
              return executeUpdate(sql);
!
          throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
      }

--- 141,147 ----
      {
          if (autoGeneratedKeys == Statement.NO_GENERATED_KEYS)
              return executeUpdate(sql);
!         //fix me : impl NO_GENERATED_KEYS & RETURN_GENERATED_KEYS
          throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
      }

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

      /**
       * Executes the given SQL statement, which may return multiple results,
--- 190,236 ----
       */
      public int executeUpdate(String sql, String columnNames[]) throws SQLException
      {
!         //fix me : columnNames only quoted if contain 0x20
!         String prefix = sql.substring(0,10).toLowerCase();
!         if (columnNames==null || prefix.indexOf("insert")==-1)
!             return executeUpdateGetResults(sql);
!         if (!(connection instanceof AbstractJdbc2Connection))
!         {
!             throw new PSQLException(GT.tr("Driver version does not support returning generated keys.")+"
"+connection.getClass().getName(),PSQLState.NOT_IMPLEMENTED); 
!         }
!         AbstractJdbc2Connection con = (AbstractJdbc2Connection)connection;
!         int args = columnNames.length;
!         if (!connection.haveMinimumServerVersion("8.2"))
!             throw new PSQLException(GT.tr("Server version does not support returning generated keys.")+" (<
"+"8.2"+")",PSQLState.NOT_IMPLEMENTED); 
!         if (args==0)
              return executeUpdate(sql);
!         StringBuffer s = new StringBuffer(sql.length()+(args*32));
!         s.append(sql);
!         s.append('\n');
!         s.append("RETURNING");
!         s.append(' ');
!         boolean needsQuote;
!         for (int i=0; i<args; i++)
!         {
!             String arg = columnNames[i];
!             if (arg==null)
!                 //throw new NullPointerException("executeUpdate: null columnName at index "+i);
!                 throw new PSQLException(GT.tr("Null value in columnNames"), PSQLState.INVALID_PARAMETER_VALUE);
!             if (i!=0)
!                 s.append(',');
!             needsQuote = Utils.needsQuoted(arg);
!             if (needsQuote)
!                 s.append('"');
!             s.append(arg);
!             if (needsQuote)
!                 s.append('"');
          }
+         return executeUpdateGetResults(s.toString());
+         //throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."),
PSQLState.NOT_IMPLEMENTED);
+     }

+
+
      /**
       * Executes the given SQL statement, which may return multiple results,
       * and signals the driver that any
Index: org/postgresql/core/Utils.java
*** C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\core\Utils.java Base (1.6)
--- C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\core\Utils.java Locally Modified (Based On 1.6)
***************
*** 146,152 ****
--- 146,186 ----

          return sbuf;
      }
+     /**
+      * return true if the string contains whitespace and is not already quoted, false otherwise
+      *
+      * @param in
+      * @return true if the string contains whitespace and is not already quoted
+      * @throws java.sql.SQLException if the string contains quotes inside its value
+      * (foo"bar or "foor"bar"), or contains char 0x00.
+      */
+     public static final boolean needsQuoted(String in) throws SQLException
+     {
+         int len = in.length();
+         //quoted and non-empty quotes:
+         boolean already = len>1 && in.charAt(0)=='"' && in.charAt(len-1)=='"';
+         if (already && len==2)
+             throw new PSQLException(GT.tr("Empty quoted value"), PSQLState.INVALID_PARAMETER_VALUE);
+         int end = len-1;
+         for (int i=1; i<end; i++)
+         {//scan for legal
+             char c = in.charAt(i);
+             if (c=='"')
+                 throw new PSQLException(GT.tr("Invalid quotes found inside argument"),
PSQLState.INVALID_PARAMETER_VALUE);    
+             if (c=='\0')
+                 throw new PSQLException(GT.tr("Null bytes may not occur in identifiers."),
PSQLState.INVALID_PARAMETER_VALUE);
          }
+         for (int i=1; i<end; i++)
+         {
+             char c = in.charAt(i);
+             if (Character.isWhitespace(c))
+                 return !already;
+         }
+         return false;
+     }
+
+ }

Statement.executeUpdate(String sql, int columnIndexes[]) via RETURNING clause?

From

Ken Johanson

Date:

06 December 2007, 06:27:07

Does anyone have knowledge of how to implement:

Statement.executeUpdate(String sql, int columnIndexes[])

This would be in the context of using the server's RETURNING clause. For
named-identifiers this is straightforward but I do not know how the
'returning' clause could support numbers... hopefully it can.

Something like?:

... RETURNING [1],[2]

Ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

13 December 2007, 05:55:53

Kris, do you what token parsers utils exists in the current JDBC
package? E.g the most tried and true way to get the schema and table
name from:

INSERT INTO foo (col1, col2..) VALUES ..
INSERT INTO foo VALUES ..
INSERT INTO "foo" VALUES ..
INSERT INTO mydb."foo" VALUES ..
etc.

Thanks,
Ken

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Kris Jurka

Date:

13 December 2007, 06:06:02

On Wed, 12 Dec 2007, Ken Johanson wrote:

> Kris, do you what token parsers utils exists in the current JDBC
> package? E.g the most tried and true way to get the schema and table
> name from:
>
> INSERT INTO foo (col1, col2..) VALUES ..

Most of the parsing code in the driver is focused on finding placeholders
and escape sequences and doesn't care what the query is actually doing.
Deriving the base tables of a query happens in only one place, updatable
resultset support.  See
org.postgresql.jdbc2.AbstractJdbc2ResultSet#parseQuery.  That said, the
current implementation is terrible and is fooled by many queries.  It just
looks for the first " FROM " and takes anything after that as the table
the select is based on.  Clearly this doesn't work for SELECT col AS from
FROM tab; or SELECT /* FROM */ * FROM tab; or SELECT (SELECT col FROM tab)
FROM tab2; and many other ways.  So I doubt modelling new code on it is a
good idea.

Kris Jurka

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Kris Jurka

Date:

13 December 2007, 15:49:24

On Wed, 5 Dec 2007, Ken Johanson wrote:

> I opted to use the Quoting mechanism I already had in executeUpdate for now,
> since the string validation (no 0x00 && no nested quotes) is being done in
> needsQuoted (in the same loop that validates quotes and scans for
> whitespace).
>
> -is whitespace the sole determinator for needing quoting? And other chars?

Any keywords would need quoting:  If you had a column named "user" it must
be quoted.

jurka=# create temp table zz(a int, "user" text);
CREATE TABLE

jurka=# insert into zz values(1,'a') returning a, user, "user";
  a | current_user | user
---+--------------+------
  1 | jurka        | a

> -is it fine to leave the string un-quoted if it contains no ws, vs always
> quoting it (my feeling is yes).

I was thinking about this some more and I think we should quote everything
regardless of whether it needs it or not.  This forces the caller to
provide the column in the correct case because it won't be folded any
more, but that's something we're already doing in DatabaseMetaData.  If we
don't do this there will be no way for the user to indicate that he has
case-sensitive column names.  (Unless of course we implemented
getGeneratedKeys with column names similar to how we might implemented it
for interger column indexes.  If we used RETURNING * and only did the
extraction once it got back to the driver, then we have some more
flexibility in handling names.)

> -is '"' the only legal quoting chars? (I cant remember for having dabbled
> with too many non-spec databases)

Yes.

> -my needsQuoted method throws if the identifier contains nested quotes
> (foo"bar or "foo"bar"); is there a legal quote-escaping mechanism similar to
> apostrophe doubling? eg: how or would one pass foo"bar (I imagine quotes are
> never allowed in identifiers but don't have an SQL spec handy)

Nested quotes are legal and escaped just like apostrophe doubling:

create table "abc""def" ( """" int);

> PS - Kris, I recall you said the backslashes in the patch were troublesome;
> did you find any fix for your patch tool aside from translating them to '/'?
> If not I will translate them from hereto forth.
>

I haven't looked at it since then.  Let's get another draft or two and
I'll see what needs to be done.

Kris Jurka

Re: Synthesize support for Statement.getGeneratedKeys()?

From

Ken Johanson

Date:

14 December 2007, 01:02:58

Kris Jurka wrote:

>
> Any keywords would need quoting:  If you had a column named "user" it
> must be quoted.
>

Enough said. I will quote all IDs and provide a diff tonight hopefully.

Patch for Statement.getGeneratedKeys()

From

Ken Johanson

Date:

14 December 2007, 06:36:40

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

All ids are now quoted in: executeUpdate(String sql, String
columnIndexes[]), and: int executeUpdate(String sql, int
columnIndexes[]) is implemented, however it currently will work ONLY if
the fully qualified table is set in the insert:

INSERT INTO foocatalog.fooschema.tbl .....(quoted or not)

It will support normalizing the not-supplied catalog and schema names --
after I find out how to extract these from the Connection (hopefully
this would not require an additional round trip). Any suggestions on this?

Ken
# This patch file was generated by NetBeans IDE
# Following Index: paths are relative to: C:\dev\java\proj\pgjdbc\pgjdbc
# This patch can be applied using context Tools: Patch action on respective folder.
# It uses platform neutral UTF-8 encoding and \n newlines.
# Above lines and this line are ignored by the patching process.
Index: org/postgresql/jdbc2/AbstractJdbc2Statement.java
*** C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc2\AbstractJdbc2Statement.java Base (1.104)
--- C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc2\AbstractJdbc2Statement.java Locally Modified (Based On 1.104)
***************
*** 286,291 ****
--- 286,318 ----
      }

      /*
+      * Execute a SQL INSERT, UPDATE or DELETE statement.  In addition
+      * SQL statements that return nothing such as SQL DDL statements
+      * can be executed
+      *
+      * @param sql a SQL statement
+      * @return either a row count, or 0 for SQL commands
+      * @exception SQLException if a database access error occurs
+      */
+     protected int executeUpdateGetResults(String p_sql) throws SQLException
+     {
+         if (preparedQuery != null)
+             throw new PSQLException(GT.tr("Can''t use query methods that take a query string on a
PreparedStatement."),
+                                     PSQLState.WRONG_OBJECT_TYPE);
+         if( isFunction )
+         {
+             executeWithFlags(p_sql, 0);
+             return 0;
+         }
+         checkClosed();
+         p_sql = replaceProcessing(p_sql);
+         Query simpleQuery = connection.getQueryExecutor().createSimpleQuery(p_sql);
+         execute(simpleQuery, null, 0);
+         this.lastSimpleQuery = simpleQuery;
+         return getUpdateCount();
+     }
+
+     /*
       * Execute a SQL INSERT, UPDATE or DELETE statement.  In addition,
       * SQL statements that return nothing such as SQL DDL statements can
       * be executed.
Index: org/postgresql/jdbc3/AbstractJdbc3Statement.java
*** C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc3\AbstractJdbc3Statement.java Base (1.21)
--- C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\jdbc3\AbstractJdbc3Statement.java Locally Modified (Based On 1.21)
***************
*** 11,16 ****
--- 11,17 ----

  import java.math.BigDecimal;
  import java.sql.*;
+ import java.util.ArrayList;
  import java.util.Calendar;
  import java.util.Vector;

***************
*** 19,24 ****
--- 20,28 ----
  import org.postgresql.core.QueryExecutor;
  import org.postgresql.core.Field;
  import org.postgresql.core.BaseConnection;
+ import org.postgresql.core.Utils;
+ import org.postgresql.jdbc2.AbstractJdbc2Connection;
+ import org.postgresql.jdbc2.AbstractJdbc2Statement.StatementResultHandler;
  import org.postgresql.util.GT;

  /**
***************
*** 28,33 ****
--- 32,38 ----
   */
  public abstract class AbstractJdbc3Statement extends org.postgresql.jdbc2.AbstractJdbc2Statement
  {
+
      private final int rsHoldability;

      public AbstractJdbc3Statement (AbstractJdbc3Connection c, int rsType, int rsConcurrency, int rsHoldability)
throwsSQLException 
***************
*** 106,112 ****
       */
      public ResultSet getGeneratedKeys() throws SQLException
      {
!         return createDriverResultSet(new Field[0], new Vector());
      }

      /**
--- 111,119 ----
       */
      public ResultSet getGeneratedKeys() throws SQLException
      {
!         return result==null ?
!             createDriverResultSet(new Field[0], new Vector())
!             : result.getResultSet();
      }

      /**
***************
*** 135,141 ****
      {
          if (autoGeneratedKeys == Statement.NO_GENERATED_KEYS)
              return executeUpdate(sql);
!
          throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
      }

--- 142,148 ----
      {
          if (autoGeneratedKeys == Statement.NO_GENERATED_KEYS)
              return executeUpdate(sql);
!         //fix me : impl NO_GENERATED_KEYS & RETURN_GENERATED_KEYS
          throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
      }

***************
*** 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 ----
       */
      public int executeUpdate(String sql, int columnIndexes[]) throws SQLException
      {
!         if (columnIndexes==null || columnIndexes.length == 0)
              return executeUpdate(sql);
!         String prefix = sql.substring(0,10).toLowerCase();
!         if (columnIndexes==null || prefix.indexOf("insert")==-1)
!         {
!             return executeUpdateGetResults(sql);
          }
+         int start = Utils.position(sql, "INTO", 0);
+         ArrayList args = Utils.getInsertIds(sql, start);
+         String pgCols =
+             "SELECT column_name "+
+             "FROM information_schema.columns "+
+             "WHERE table_catalog='"+args.get(0)+"' AND table_schema='"+args.get(1)+"' AND
table_name='"+args.get(2)+"'"+ 
+             "ORDER BY ordinal_position";
+         ResultSet rs = null;
+         String[] columnNames = new String[columnIndexes.length];
+         try {
+             rs = this.executeQuery(pgCols);
+         } catch (SQLException ex) {
+             throw new PSQLException(GT.tr("Could not translate column name indexes.")+" "+ex,
PSQLState.UNEXPECTED_ERROR);
+         } finally {
+             if (rs!=null) rs.close();
+         }
+         int j=0;
+         try {
+             for (; j<columnNames.length; j++)
+             {
+                 rs.absolute(columnIndexes[j]);
+                 columnNames[j] = rs.getString(1);
+             }
+         } catch (SQLException ex) {//invalid column index provided
+             throw new PSQLException(GT.tr("Column index out of bounds.")+" "+columnIndexes[j],
PSQLState.UNEXPECTED_ERROR);
+         }
+         return executeUpdate(sql, columnNames);
+     }

      /**
       * Executes the given SQL statement and signals the driver that the
***************
*** 184,198 ****
       */
      public int executeUpdate(String sql, String columnNames[]) throws SQLException
      {
!         if (columnNames.length == 0)
              return executeUpdate(sql);
!
!         throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
      }

      /**
       * Executes the given SQL statement, which may return multiple results,
--- 221,262 ----
       */
      public int executeUpdate(String sql, String columnNames[]) throws SQLException
      {
!         String prefix = sql.substring(0,10).toLowerCase();
!         if (columnNames==null || prefix.indexOf("insert")==-1)
!             return executeUpdateGetResults(sql);
!         if (!(connection instanceof AbstractJdbc2Connection))
!         {
!             throw new PSQLException(GT.tr("Driver version does not support returning generated keys.")+"
"+connection.getClass().getName(),PSQLState.NOT_IMPLEMENTED); 
!         }
!         AbstractJdbc2Connection con = (AbstractJdbc2Connection)connection;
!         int args = columnNames.length;
!         if (!connection.haveMinimumServerVersion("8.2"))
!             throw new PSQLException(GT.tr("Server version does not support returning generated keys.")+" (<
"+"8.2"+")",PSQLState.NOT_IMPLEMENTED); 
!         if (args==0)
              return executeUpdate(sql);
!         StringBuffer s = new StringBuffer(sql.length()+(args*32));
!         s.append(sql);
!         s.append('\n');
!         s.append("RETURNING");
!         s.append(' ');
!         for (int i=0; i<args; i++)
!         {
!             String arg = columnNames[i];
!             if (arg==null)
!                 //throw new NullPointerException("executeUpdate: null columnName at index "+i);
!                 throw new PSQLException(GT.tr("Null value in columnNames"), PSQLState.INVALID_PARAMETER_VALUE);
!             if (i!=0)
!                 s.append(',');
!             s.append('"');
!             s.append(arg);
!             s.append('"');
          }
+         return executeUpdateGetResults(s.toString());
+         //throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."),
PSQLState.NOT_IMPLEMENTED);
+     }

+
+
      /**
       * Executes the given SQL statement, which may return multiple results,
       * and signals the driver that any


Index: org/postgresql/core/Utils.java
*** C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\core\Utils.java Base (1.6)
--- C:\dev\java\proj\pgjdbc\pgjdbc\org\postgresql\core\Utils.java Locally Modified (Based On 1.6)
***************
*** 12,17 ****
--- 12,18 ----
  package org.postgresql.core;

  import java.sql.SQLException;
+ import java.util.ArrayList;

  import org.postgresql.util.GT;
  import org.postgresql.util.PSQLException;
***************
*** 146,152 ****
--- 147,286 ----

          return sbuf;
      }
+     /**
+      * return true if the string contains whitespace and is not already quoted, false otherwise
+      *
+      * @param in
+      * @return true if the string contains whitespace and is not already quoted
+      * @throws java.sql.SQLException if the string contains quotes inside its value
+      * (foo"bar or "foor"bar"), or contains char 0x00.
+      */
+     public static final boolean needsQuoted(String in) throws SQLException
+     {
+         int len = in.length();
+         //quoted and non-empty quotes:
+         boolean already = len>1 && in.charAt(0)=='"' && in.charAt(len-1)=='"';
+         if (already && len==2)
+             throw new PSQLException(GT.tr("Empty quoted value"), PSQLState.INVALID_PARAMETER_VALUE);
+         int end = len-1;
+         for (int i=1; i<end; i++)
+         {//scan for legal
+             char c = in.charAt(i);
+             if (c=='"')
+                 throw new PSQLException(GT.tr("Invalid quotes found inside argument"),
PSQLState.INVALID_PARAMETER_VALUE);    
+             if (c=='\0')
+                 throw new PSQLException(GT.tr("Null bytes may not occur in identifiers."),
PSQLState.INVALID_PARAMETER_VALUE);
          }
+         for (int i=1; i<end; i++)
+         {
+             char c = in.charAt(i);
+             if (Character.isWhitespace(c))
+                 return !already;
+         }
+         return false;
+     }
+
+     /**
+      * Return an ArrayList of Strings representing the table identifiers, quoted or not.
+     * Any number of id may exists; no attempt is made to validate the maximum number of IDs.
+      * @param sql INSERT INTO stmt
+      * @param start - index of the INTO keyword, after which the <code>catalog.schema.table</code> identifiers appear
+      * @return ArrayList who first element is the left-most identifiers, and right-most is the table name.
+     * @author Ken Johanon ken2006@onnet.cc
+      */
+     public static ArrayList getInsertIds(String sql, int start)
+     {
+         if (start<0)
+             throw new IllegalArgumentException("getInsertIds: invalid start index: "+start);
+         start += 4;
+         //advance to first alnum
+         for (; start<sql.length(); start++)
+             if (Character.isLetterOrDigit(sql.charAt(start)))
+                 break;
+         //advance to first non-quoted, non-alnum
+         ArrayList ar = new ArrayList(4);
+         int end = start;
+         int pos = start;
+         boolean inQuote = sql.charAt(end-1)=='"';
+         for (; end<sql.length(); end++)
+         {
+             char c = sql.charAt(end);
+             if (inQuote)
+             {
+                 if (c=='"')
+                 {
+                     ar.add(sql.substring(pos, end));
+                     end++;
+                     pos = end+1;
+                     inQuote = false;
+                 }
+             }
+             else
+             {
+                 if (c=='"')
+                 {
+                     inQuote = true;
+                     pos = end+1;
+                 }
+                 else if (c=='.')
+                 {
+                     ar.add(sql.substring(pos, end));
+                     pos = end+1;
+                 }
+             }
+
+             if (c=='(' || (!inQuote && Character.isSpaceChar(c)))
+             {
+                 if (pos!=end)
+                     ar.add(sql.substring(pos, end));
+                 break;
+             }
+         }
+         return ar;
+     }
+
+     /**
+      * Search for and return the location of <code>find</code> in String <code>in</code>, case insensitive.
+      * If in is empty, return -1. If find is empty, return 0.
+      * @param in
+      * @param find - string to find
+      * @param pos - starting position to search
+      * @return int location, or -1 if not found
+     * @author Ken Johanon ken2006@onnet.cc
+      */
+     public static int position(CharSequence in, String find, int pos)
+     {
+         boolean c = in==null || in.length()==0;
+         boolean d = find==null || find.length()==0;
+         if (d || c && d)
+             return 0;
+         if (c)
+             return -1;
+         int a = in.length();
+         int b = find.length();
+         int count = 0;
+         //if (pos>a-b)
+         //    return -1;
+         char c1, c2;
+         for (int i=pos; i<a; i++)
+         {
+             c1 = in.charAt(i);
+             c2 = find.charAt(count);
+             if (c1==c2 || c1==Character.toLowerCase(c2) || c1==Character.toUpperCase(c2))
+                 count++;
+             else
+             {
+                 i -= count;
+                 count = 0;
+             }
+             if (count==b)
+                 return i-b+1;
+         }
+         return -1;
+     }
+
+ }

Re: Patch for Statement.getGeneratedKeys()

From

Ken Johanson

Date:

19 December 2007, 04:30:21

Kris, were you able to apply the last patch I sent? Let me know what
you'd like. I would like to proceed so I can close this project out.

Re: Patch for Statement.getGeneratedKeys()

From

Kris Jurka

Date:

19 December 2007, 04:36:44

On Tue, 18 Dec 2007, Ken Johanson wrote:

> Kris, were you able to apply the last patch I sent? Let me know what
> you'd like. I would like to proceed so I can close this project out.
>

Sorry, I have not had time to look at it and probably won't be able to
until next year.  I've got a vacation coming up and people who pay me that
want stuff done before I leave.

Kris Jurka

Re: Patch for Statement.getGeneratedKeys()

From

Ken Johanson

Date:

03 January 2008, 05:22:36

> On Tue, 18 Dec 2007, Ken Johanson wrote:
>
>> Kris, were you able to apply the last patch I sent? Let me know what
>> you'd like. I would like to proceed so I can close this project out.
>>
>
> Sorry, I have not had time to look at it and probably won't be able to
> until next year.  I've got a vacation coming up and people who pay me
> that want stuff done before I leave.
>

Just a friendly reminder. Whenever you can try the patch let me know.
k

Re: Patch for Statement.getGeneratedKeys()

From

Kris Jurka

Date:

07 January 2008, 05:24:32

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

Re: Patch for Statement.getGeneratedKeys()

From

Ken Johanson

Date:

08 January 2008, 04:29:16

Kris,
Incomplete / short response below:
>
>
> 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.

I am upgrading my NB now and we'll see how the next one comes out..

>
> 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?

I'm going to let you suggest code for this method, as I don't even
recall coding it (a long time ago perhaps and lazy copy-paste). I do not
know the driver innards enough to answer this (I am a relative novice
even at the formal JDBC API level).

>
> 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?..

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

Will add these unless you suggest code first.

> 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?

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

OK

>
> 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.

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

Thank you, I will apply appendEscapedIdentifier().

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

OK

>
> 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".

It requires a context-sensitive (int)start argument, to position the
search start onto the first keyword, i.e INTO for the case of INSERTs. I
tested quoted quote-less Ids. As stated earlier it is incomplete pending
a way to pass-in the current DB & schema names.


9.b):
  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.

(see above). I believe that a fully qualified value may be needed in the
  case where the table is fully qualified but (for some reason) not in
the same context as the RETURNING... this can happen, yes? Or does
RETURNING always refer to the acted-on table?

>
> 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.

Ken

Re: Patch for Statement.getGeneratedKeys()

From

Ken Johanson

Date:

10 January 2008, 03:37:34

Kris Jurka wrote:
>
> 9) Utils.getInsertIds
....
> the requirement that a query uses a completely qualified name
> database.schema.table is quite onerous.
>

What are your thoughts on this? Should it be possible/safe going forward
to not supply the fully qualified table in the case of:

Statement.executeUpdate(String sql, int columnIndexes[])

If you can recommend a simpler strategy I will try it. If we can avoid
the round trip also that would be great.

Ken

Re: Patch for Statement.getGeneratedKeys()

From

Kris Jurka

Date:

11 January 2008, 04:29:22

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

Re: Patch for Statement.getGeneratedKeys()

From

Kris Jurka

Date:

11 January 2008, 04:29:25

On Wed, 9 Jan 2008, Ken Johanson wrote:

>> the requirement that a query uses a completely qualified name
>> database.schema.table is quite onerous.
>>
>
> What are your thoughts on this? Should it be possible/safe going forward to
> not supply the fully qualified table in the case of:
>
> Statement.executeUpdate(String sql, int columnIndexes[])
>
> If you can recommend a simpler strategy I will try it. If we can avoid the
> round trip also that would be great.
>

Writing "RETURNING *" will return more data than you need, but it has the
plus of not requiring you to know anything about the underlying table
columns.  Once the full result is returned you would need to strip out
only the parts specified in columnIndexes and either create a new
ResultSet or a wrapper around the returned ResultSet.

Kris Jurka

Re: Patch for Statement.getGeneratedKeys()

From

Ken Johanson

Date:

15 January 2008, 04:30:24

>> 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.
>

That makes sense to me now, thanks. In any case do you agree that we
still need to parse the fully qualified table, in case of input like:

insert into postgres.public.test ...

I don think my earlier question about getting the current/default
catalog name is relevant since the query can specify other ones. True?

Re: Patch for Statement.getGeneratedKeys()

From

Ken Johanson

Date:

15 January 2008, 04:37:55

>> What are your thoughts on this? Should it be possible/safe going
>> forward to not supply the fully qualified table in the case of:
>>
>> Statement.executeUpdate(String sql, int columnIndexes[])
>>
>> If you can recommend a simpler strategy I will try it. If we can avoid
>> the round trip also that would be great.
>>
>
> Writing "RETURNING *" will return more data than you need, but it has
> the plus of not requiring you to know anything about the underlying
> table columns.  Once the full result is returned you would need to strip
> out only the parts specified in columnIndexes and either create a new
> ResultSet or a wrapper around the returned ResultSet.
>

"RETURNING *" is indeed simpler although based on on real word
experience on slow/congested links or hi transaction volume, and use
cases with large inserts (multi-row, many columns, or LOBs) (
memory/scalability issues), the round trip seems to preferable. I'd
rather muddle though getting that to work (I did already and code to
follow) instead of risking more serious performance issues, if you don't
object.

Ken

Re: Patch for Statement.getGeneratedKeys()

From

Kris Jurka

Date:

15 January 2008, 04:45:33

On Mon, 14 Jan 2008, Ken Johanson wrote:

> That makes sense to me now, thanks. In any case do you agree that we still
> need to parse the fully qualified table, in case of input like:
>
> insert into postgres.public.test ...

That depends what your approach is for non-qualified tables as it would be
odd to do it differently for the two cases.  (Just got your other email).
Since you don't like "RETURNING *", you will need to be able to parse a
fully qualified name, but you also must be able to parse and then qualify
a non-fully qualified name.

> I don think my earlier question about getting the current/default catalog
> name is relevant since the query can specify other ones. True?
>

Not really, you can only specify the current database.  If you try
it with something else you get:

jurka=# select * from jurka.schema.tab;
ERROR:  schema "schema" does not exist

jurka=# select * from otherdb.schema.tab;
ERROR:  cross-database references are not implemented:
"otherdb.schema.tab"

Kris Jurka

Re: Patch for Statement.getGeneratedKeys()

From

Ken Johanson

Date:

15 January 2008, 06:08:24

>> I don think my earlier question about getting the current/default
>> catalog name is relevant since the query can specify other ones. True?
>>
>
> Not really, you can only specify the current database.  If you try it
> with something else you get:
>
> jurka=# select * from jurka.schema.tab;
> ERROR:  schema "schema" does not exist
>
> jurka=# select * from otherdb.schema.tab;
> ERROR:  cross-database references are not implemented: "otherdb.schema.tab"
>

Hmm. I do know that I can select the fully qualified name (select * from
postgres.public.test), so I presume your first example is the case where
"schema" does not exist.

For int executeUpdate(String sql, int columnIndexes[]) I'll still need
get the fully qual'd name to filter the information_schema (or pg_
tables). Since we can get the table name and catalog, that leaves
schema. Do you know how we can get the current schema name? I presumed
it might appear in SHOW ALL, but not so.

Ken

Re: Patch for Statement.getGeneratedKeys()

From

Ken Johanson

Date:

15 January 2008, 06:18:29

Ken Johanson wrote:
> Do you know how we can get the current schema name? I presumed
> it might appear in SHOW ALL, but not so.
>

Answer: select current_schema()  :-)

Re: Patch for Statement.getGeneratedKeys()

From

Kris Jurka

Date:

15 January 2008, 06:27:53

On Tue, 15 Jan 2008, Ken Johanson wrote:

> Ken Johanson wrote:
>> Do you know how we can get the current schema name? I presumed it might
>> appear in SHOW ALL, but not so.
>>
>
> Answer: select current_schema()  :-)
>

False, as I explained earlier there is no current schema.  Have
a read of this:

http://www.postgresql.org/docs/8.2/static/ddl-schemas.html#DDL-SCHEMAS-PATH

You need to look through the schemas in the search_path in order and see
which one a table with the given name appears in first.

Kris Jurka

Re: Patch for Statement.getGeneratedKeys()

From

Tom Lane

Date:

15 January 2008, 14:32:18

Kris Jurka <books@ejurka.com> writes:
> You need to look through the schemas in the search_path in order and see
> which one a table with the given name appears in first.

I've lost track of the context in which this needs to be done, but in
some cases a cast to or from regclass offers a painless way to
disambiguate table names.  Just a suggestion ...

            regards, tom lane

Re: Patch for Statement.getGeneratedKeys()

From

Ken Johanson

Date:

16 January 2008, 06:08:50

Tom Lane wrote:
> Kris Jurka <books@ejurka.com> writes:
>> You need to look through the schemas in the search_path in order and see
>> which one a table with the given name appears in first.
>
> I've lost track of the context in which this needs to be done, but in
> some cases a cast to or from regclass offers a painless way to
> disambiguate table names.  Just a suggestion ...
>

Tom, can you offer an example of this and how the overall goal might be
achieved? Kris, please jump in where I'm missing anything:

#Overview:
We need to implement:
Statement.executeUpdate(String sql, int columnIndexes[])

Current strategy is to find the natural column order (ordinal positions
for columnIndexes[]) and extract those names, passing them through to:
Statement.executeUpdate(String sql, String columnIndexes[])

To get the column names, I need to look in [the pg_* table equiv to
information_schema] tables, and of course this means knowing which table
is being referenced for modification. We are already parsing the table
name (fully or partially qualified) from the DML; now we need to search
[information_schema], finding the matching catalog, schema, and table,
and searching schema in the order of the schema search-path.

#History
Most interesting probably is that Kris mentioned it would work to just
do a INSERT.. RETURNING * to get the keys, however I'm electing to try
the extra-mile / "hard way" to save returning LOBs or entire multi-row
inserts. Ideally I's like to do everything in one extra query to
[information_schema] or better yet in RETURNING.

#Now
I'm a bit perplexed as to how I could get the current-ref'd schema using
one query. I think it might involve passing a subselect of "SHOW
search_path" as the arg to the [information_schema] query, but using
that var as a list and filling the $user var is not familiar ground...

#Questions:

-would the regclass-cast technique (I have no experience with it) work
directly in the RETURNING or need to be in the [information_schema]
query? Can you point me to examples?

-would it be feasible to modify RETURNING in new server versions to
accept indexes as args? That would obviate this whole discussion.

Thanks,
Ken

Re: Patch for Statement.getGeneratedKeys()

From

Kris Jurka

Date:

16 January 2008, 06:20:43

On Wed, 16 Jan 2008, Ken Johanson wrote:

>> I've lost track of the context in which this needs to be done, but in
>> some cases a cast to or from regclass offers a painless way to
>> disambiguate table names.  Just a suggestion ...
>
> Tom, can you offer an example of this and how the overall goal might be
> achieved? Kris, please jump in where I'm missing anything:

Regclass is actually exactly what you need.  This let's us skip all kinds
of parsing, deducing, ...

SELECT 'database.schema.table'::regclass::oid;
SELECT 'schema.table'::regclass::oid;
SELECT 'table'::regclass::oid;
SELECT '"database".schema."table"'::regclass::oid;

will all return the same thing, the oid for the qualified table, or if
unqualified, the first matching table on the search path.  The oid will be
pg_class.oid which can then easily be used to lookup the columns in
pg_attribute as people have explained on -general.

> #Questions:
>
> -would the regclass-cast technique (I have no experience with it) work
> directly in the RETURNING or need to be in the [information_schema] query?
> Can you point me to examples?

Needs to be in a separate query.

> -would it be feasible to modify RETURNING in new server versions to accept
> indexes as args? That would obviate this whole discussion.
>

Not really, RETURNING is an arbitrary SELECT list, so you can say things
like RETURNING 1, 2+columnA, f(columnB).  You could potentially add some
kind of keyword like RETURNING INDEXES 1,2,7, but I doubt the server
people have a great desire to support something this braindead for just
one client API to use.

Kris Jurka

Re: Patch for Statement.getGeneratedKeys()

From

Tom Lane

Date:

16 January 2008, 06:38:42

Ken Johanson <pg-user@kensystem.com> writes:
> Tom Lane wrote:
>> I've lost track of the context in which this needs to be done, but in
>> some cases a cast to or from regclass offers a painless way to
>> disambiguate table names.  Just a suggestion ...

> Tom, can you offer an example of this and how the overall goal might be
> achieved?

Well, most of the point is that regclass can substitute for an explicit
understanding of search_path disambiguation.  If you see "s.t" in the
query then it's clear that this is table t in schema s, but if you see
just "t" then you aren't so sure which schema it is in.  Resolving that
in a pure-SQL query is theoretically possible but it seems mighty ugly.

> To get the column names, I need to look in [the pg_* table equiv to
> information_schema] tables, and of course this means knowing which table
> is being referenced for modification.

Right.  I suggest using regclass to obtain the OID of the referenced
table, which then allows direct lookup in pg_attribute and other
relevant catalogs.  Something along the line of

    select attname from pg_attribute where attrelid = 't'::regclass

which also works for

    select attname from pg_attribute where attrelid = 's.t'::regclass

This is oversimplified because it doesn't consider any quoting issues
for funny characters in table names, and the query itself needs some
refinements like checking for attisdropped, but hopefully the point
is clear.

> -would it be feasible to modify RETURNING in new server versions to
> accept indexes as args? That would obviate this whole discussion.

I'm not clear what you're hoping for there.  It seems to me that any way
you slice it, you'll need to know which columns of the result are what.
"RETURNING *" is inadequate for that reason, but so would be "RETURNING
some-index-columns-but-I-aint-saying-which".  ISTM you've got to parse
things out enough to understand which columns you want and why; once
you know that, asking for them by name doesn't seem like much trouble.

            regards, tom lane

Re: Patch for Statement.getGeneratedKeys()

From

Ken Johanson

Date:

16 January 2008, 07:21:11

Tom Lane wrote:
> Right.  I suggest using regclass to obtain the OID of the referenced
> table, which then allows direct lookup in pg_attribute and other
> relevant catalogs.  Something along the line of
>
>     select attname from pg_attribute where attrelid = 't'::regclass
>

Here is what I have so far (not sure of my conditionals yet or if I need
any joins for them):

select attname from pg_attribute
where attrelid = 'postgres.public.test'::regclass
and attstattarget=-1
and attisdropped='f'
order by attnum asc

Very simple and elegant, you guys rule.

Couple things I want to verify - will this regclass method have any
cases where security would make it less reliable than say, using pg_*?
Also compatibility issues?: are there any server or session/driver modes
that would somehow prevent this from working?

ken

Re: Patch for Statement.getGeneratedKeys()

From

tivvpgsqljdbc@gtech-ua.com

Date:

16 January 2008, 08:30:25

BTW: How about next way:
extract table name as it is
parse + describe "select * from tableName".
optionally close unnamed statement (Protocol says it will still be
closed by next unnamed statement).
Read column names from there.
Call insert with returning column names.
It still has round-trip (or two, dunno if parse + describe needs two
round-trips), but it will take any insert statement.

P.S. Sorry if message will come twice - used wrong From: for the first one.

Re: Patch for Statement.getGeneratedKeys()

From

Tom Lane

Date:

16 January 2008, 14:02:54

Ken Johanson <pg-user@kensystem.com> writes:
> Here is what I have so far (not sure of my conditionals yet or if I need
> any joins for them):

> select attname from pg_attribute
> where attrelid = 'postgres.public.test'::regclass
> and attstattarget=-1
> and attisdropped='f'
> order by attnum asc

I'd do "where attrelid = ... and attnum > 0 and not attisdropped".
Not sure what you think the attstattarget condition would accompllish,
but it doesn't seem like anything that client-side code ought to be
touching.

            regards, tom lane

Re: Patch for Statement.getGeneratedKeys()

From

Ken Johanson

Date:

17 January 2008, 05:13:47

Kris, all: please see the changes in AbstractJdbc3Statement, the
regclass technique suggested by Tom is implemented and working. However
I have not made any attempt to manage the separate generated-keys
resultset nor state (RETURN_GENERATED_KEYS).

Please consider making these changes yourself, or tell me what needs to
be done (again I am not proficient with the spec or state mgmt for the
calls). I think your deeper knowledge of the driver and spec will
produce better quality code. I am losing some cycles starting next week
so won't be able to work on this for several weeks or more.

Thanks,
Ken
# This patch file was generated by NetBeans IDE
# This patch can be applied using context Tools: Apply Diff Patch action on respective folder.
# It uses platform neutral UTF-8 encoding.
# Above lines and this line are ignored by the patching process.
Index: pgjdbc/org/postgresql/core/Utils.java
--- pgjdbc/org/postgresql/core/Utils.java Base (1.6)
+++ pgjdbc/org/postgresql/core/Utils.java Locally Modified (Based On 1.6)
@@ -12,6 +12,7 @@
 package org.postgresql.core;

 import java.sql.SQLException;
+import java.util.ArrayList;

 import org.postgresql.util.GT;
 import org.postgresql.util.PSQLException;
@@ -146,4 +147,65 @@

         return sbuf;
     }
+
+
+    /**
+     * Return an ArrayList of Strings representing the table identifiers, quoted or not.
+     * Any number of ids may exists; no attempt is made to validate the maximum number of IDs.
+     * @param sql INSERT INTO stmt
+     * @param start - end-index of the keyword (INTO, FROM, UPDATE etc) preceding
+     * the table reference, after which the <code>catalog.schema.table</code> identifiers appear
+     * @return ArrayList who first element is the left-most identifiers,
+     * and right-most is the table name.
+     * @author Ken Johanon - ken2006 at onnet.cc
+     */
+    public static ArrayList getTableIdentifiers(String sql, int start)
+    {
+        if (start<0)//assertion
+            throw new IllegalArgumentException("getInsertIds: invalid start index: -1");
+        //advance to first alnum
+        for (; start<sql.length(); start++)
+            if (Character.isLetterOrDigit(sql.charAt(start)))
+                break;
+        //advance to first non-quoted, non-alnum
+        ArrayList ar = new ArrayList(3);
+        int end = start;
+        int pos = start;
+        boolean inQuote = sql.charAt(end-1)=='"';
+        for (; end<sql.length(); end++)
+        {
+            char c = sql.charAt(end);
+            if (inQuote)
+            {
+                if (c=='"')
+                {
+                    ar.add(sql.substring(pos, end));
+                    end++;
+                    pos = end+1;
+                    inQuote = false;
 }
+            }
+            else
+            {
+                if (c=='"')
+                {
+                    inQuote = true;
+                    pos = end+1;
+                }
+                else if (c=='.')
+                {
+                    ar.add(sql.substring(pos, end));
+                    pos = end+1;
+                }
+            }
+
+            if (c=='(' || (!inQuote && Character.isSpaceChar(c)))
+            {
+                if (pos!=end)
+                    ar.add(sql.substring(pos, end));
+                break;
+            }
+        }
+        return ar;
+    }
+}
Index: pgjdbc/org/postgresql/jdbc2/AbstractJdbc2Statement.java
--- pgjdbc/org/postgresql/jdbc2/AbstractJdbc2Statement.java Base (1.104)
+++ pgjdbc/org/postgresql/jdbc2/AbstractJdbc2Statement.java Locally Modified (Based On 1.104)
@@ -286,6 +286,33 @@
     }

     /*
+     * Execute a SQL INSERT, UPDATE or DELETE statement.  In addition
+     * SQL statements that return nothing such as SQL DDL statements
+     * can be executed
+     *
+     * @param sql a SQL statement
+     * @return either a row count, or 0 for SQL commands
+     * @exception SQLException if a database access error occurs
+     */
+    protected int executeUpdateGetResults(String p_sql) throws SQLException
+    {
+        if (preparedQuery != null)
+            throw new PSQLException(GT.tr("Can''t use query methods that take a query string on a
PreparedStatement."),
+                                    PSQLState.WRONG_OBJECT_TYPE);
+        if( isFunction )
+        {
+            executeWithFlags(p_sql, 0);
+            return 0;
+        }
+        checkClosed();
+        p_sql = replaceProcessing(p_sql);
+        Query simpleQuery = connection.getQueryExecutor().createSimpleQuery(p_sql);
+        execute(simpleQuery, null, 0);
+        this.lastSimpleQuery = simpleQuery;
+        return getUpdateCount();
+    }
+
+    /*
      * Execute a SQL INSERT, UPDATE or DELETE statement.  In addition,
      * SQL statements that return nothing such as SQL DDL statements can
      * be executed.
Index: pgjdbc/org/postgresql/jdbc3/AbstractJdbc3Statement.java
--- pgjdbc/org/postgresql/jdbc3/AbstractJdbc3Statement.java Base (1.22)
+++ pgjdbc/org/postgresql/jdbc3/AbstractJdbc3Statement.java Locally Modified (Based On 1.22)
@@ -11,6 +11,7 @@

 import java.math.BigDecimal;
 import java.sql.*;
+import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.Vector;

@@ -19,6 +20,9 @@
 import org.postgresql.core.QueryExecutor;
 import org.postgresql.core.Field;
 import org.postgresql.core.BaseConnection;
+import org.postgresql.core.Utils;
+import org.postgresql.jdbc2.AbstractJdbc2Connection;
+import org.postgresql.jdbc2.AbstractJdbc2Statement.StatementResultHandler;
 import org.postgresql.util.GT;

 /**
@@ -28,6 +32,7 @@
  */
 public abstract class AbstractJdbc3Statement extends org.postgresql.jdbc2.AbstractJdbc2Statement
 {
+
     private final int rsHoldability;

     public AbstractJdbc3Statement (AbstractJdbc3Connection c, int rsType, int rsConcurrency, int rsHoldability) throws
SQLException
@@ -106,7 +111,9 @@
      */
     public ResultSet getGeneratedKeys() throws SQLException
     {
-        return createDriverResultSet(new Field[0], new Vector());
+        return result==null ?
+            createDriverResultSet(new Field[0], new Vector())
+            : result.getResultSet();
     }

     /**
@@ -135,7 +142,7 @@
     {
         if (autoGeneratedKeys == Statement.NO_GENERATED_KEYS)
             return executeUpdate(sql);
-
+        //fix me : impl NO_GENERATED_KEYS & RETURN_GENERATED_KEYS
         throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
     }

@@ -159,11 +166,85 @@
      */
     public int executeUpdate(String sql, int columnIndexes[]) throws SQLException
     {
-        if (columnIndexes.length == 0)
+        if (columnIndexes==null || columnIndexes.length==0)
             return executeUpdate(sql);
-
-        throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
+        String preamble = sql.substring(0, Math.min(sql.length()-1,100)).toUpperCase();
+        int start = -1;
+        String subparse =
+            (start = preamble.indexOf("INSERT"))!=-1 ? "INTO" :
+            (start = preamble.indexOf("DELETE"))!=-1 ? "FROM" :
+            null;
+        if (subparse==null)
+        {//UPDATE
+            start = preamble.indexOf("UPDATE");
+            if (start!=-1) start += 6;
     }
+        else
+        {//INTO or FROM
+            start = preamble.indexOf(subparse, start);
+            if (start!=-1) start += 4;
+        }
+        if (start==-1)//not one of INSERT,DELETE,UPDATE
+            return executeUpdate(sql);
+        ArrayList args = Utils.getTableIdentifiers(sql, start);
+        boolean standardConformingStrings = connection.getStandardConformingStrings();
+        int argLen = args.size();
+        if (argLen==0)
+            throw new PSQLException(GT.tr("Assertion failed: table reference zero elements"),
PSQLState.UNEXPECTED_ERROR);
+        StringBuffer sb = new StringBuffer(argLen*20);
+        if (argLen>2)
+        {
+            sb.append('"');
+            Utils.appendEscapedLiteral(sb,args.get(argLen-3).toString(), standardConformingStrings);
+            sb.append('"');
+            sb.append('.');
+        }
+        if (argLen>1)
+        {
+            sb.append('"');
+            Utils.appendEscapedLiteral(sb,args.get(argLen-2).toString(), standardConformingStrings);
+            sb.append('"');
+            sb.append('.');
+        }
+        sb.append('"');
+        Utils.appendEscapedLiteral(sb,args.get(argLen-1).toString(), standardConformingStrings);
+        sb.append('"');
+        String tblRef = sb.toString();
+        String pgCols =
+            "SELECT attname "+
+            "FROM pg_attribute "+
+            "WHERE attrelid = '"+tblRef+"'::regclass "+
+            "AND attnum > 0 "+
+            "AND NOT attisdropped "+
+            "ORDER BY attnum ASC";
+        String[] columnNames = new String[columnIndexes.length];
+        boolean isOutOfBoundsEx = false;
+        ResultSet rs = null;
+        try {
+            rs = this.executeQuery(pgCols);
+            int j=0;
+            try {
+                for (; j<columnNames.length; j++)
+                {
+                    rs.absolute(columnIndexes[j]);
+                    columnNames[j] = rs.getString(1);
+                }
+            } catch (SQLException ex) {//invalid column-index provided
+                if (rs.getRow()==0)
+                    throw new PSQLException(GT.tr("Table OID not found")+": "+tblRef, PSQLState.INVALID_NAME);
+                isOutOfBoundsEx = true;
+                throw new PSQLException(GT.tr("Column index out of bounds")+": "+columnIndexes[j],
PSQLState.INVALID_PARAMETER_VALUE);
+            }
+        } catch (SQLException ex) {
+            if (isOutOfBoundsEx)
+                throw ex;
+            //in executeQuery:
+            throw new PSQLException(GT.tr("Cannot translate column name indexes"), PSQLState.UNEXPECTED_ERROR, ex);
+        } finally {
+            if (rs!=null) rs.close();
+        }
+        return executeUpdate(sql, columnNames);
+    }

     /**
      * Executes the given SQL statement and signals the driver that the
@@ -184,12 +265,33 @@
      */
     public int executeUpdate(String sql, String columnNames[]) throws SQLException
     {
-        if (columnNames.length == 0)
+        if (columnNames==null || columnNames.length == 0)
             return executeUpdate(sql);
-
-        throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
+        int args = columnNames.length;
+        if (!connection.haveMinimumServerVersion("8.2"))
+            throw new PSQLException(GT.tr("Server version does not support returning generated keys: < 8.2"),
PSQLState.NOT_IMPLEMENTED);
+        if (args==0)
+            return executeUpdate(sql);
+        StringBuffer s = new StringBuffer(sql.length()+(args*32));
+        s.append(sql);
+        s.append('\n');
+        s.append("RETURNING");
+        s.append(' ');
+        for (int i=0; i<args; i++)
+        {
+            String arg = columnNames[i];
+            if (arg==null)
+                throw new PSQLException(GT.tr("Null value in columnNames"), PSQLState.INVALID_PARAMETER_VALUE);
+            if (i!=0)
+                s.append(',');
+            s.append(Utils.appendEscapedIdentifier(null,arg));
     }
+        return executeUpdateGetResults(s.toString());
+        //throw new PSQLException(GT.tr("Returning autogenerated keys is not supported."), PSQLState.NOT_IMPLEMENTED);
+    }

+
+
     /**
      * Executes the given SQL statement, which may return multiple results,
      * and signals the driver that any