Re: Synthesize support for Statement.getGeneratedKeys()?
Re: Synthesize support for Statement.getGeneratedKeys()?
От:
Dave Cramer <pg@fastcrypt.com>
Дата:
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: Patch for Statement.getGeneratedKeys()
От:
Tom Lane <tgl@sss.pgh.pa.us>
Дата:
Ken Johanson 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()
От:
Tom Lane <tgl@sss.pgh.pa.us>
Дата:
Ken Johanson 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()
От:
Tom Lane <tgl@sss.pgh.pa.us>
Дата:
Kris Jurka 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: Synthesize support for Statement.getGeneratedKeys()?
От:
Dave Cramer <pg@fastcrypt.com>
Дата:
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 >
Synthesize support for Statement.getGeneratedKeys()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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(''); 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()?
От:
Michael Paesold <mpaesold@gmx.at>
Дата:
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(''); 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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Michael Paesold <mpaesold@gmx.at>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
> > 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()?
От:
Michael Paesold <mpaesold@gmx.at>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
>> 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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
>>>> 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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
>> 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()?
От:
Michael Paesold <mpaesold@gmx.at>
Дата:
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()?
От:
Michael Paesold <mpaesold@gmx.at>
Дата:
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()?
От:
Vit Timchishin <tivvpgsqljdbc@gtech-ua.com>
Дата:
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 *prepareStatement*(String sql,
int[] columnIndexes)
throws SQLException
(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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
>>>> 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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
>> 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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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; iRe: Synthesize support for Statement.getGeneratedKeys()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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; iRe: Synthesize support for Statement.getGeneratedKeys()?
От:
Vit Timchishin <tivvpgsqljdbc@gtech-ua.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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) throws SQLException
***************
*** 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 (majorRe: Synthesize support for Statement.getGeneratedKeys()?
От:
Kris Jurka <books@ejurka.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
> 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) throws SQLException
***************
*** 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; i1 && 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; iRe: Synthesize support for Statement.getGeneratedKeys()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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) throws SQLException
***************
*** 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; i1 && 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; iStatement.executeUpdate(String sql, int columnIndexes[]) via RETURNING clause?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()?
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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) throws SQLException
***************
*** 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 (; j1 && 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; icatalog.schema.table 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 (; startfind in String in, 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; iRe: Patch for Statement.getGeneratedKeys()
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
> 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()
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
>> 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()
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
>> 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()
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
>> 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()
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
Tom Lane wrote: > Kris Jurka 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()
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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()
От:
tivvpgsqljdbc@gtech-ua.com
Дата:
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()
От:
Ken Johanson <pg-user@kensystem.com>
Дата:
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 catalog.schema.table 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 (; start2)
+ {
+ 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 (; jRe: Synthesize support for Statement.getGeneratedKeys()?
От:
Dave Cramer <pg@fastcrypt.com>
Дата:
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()?
От:
"Albe Laurenz" <all@adv.magwien.gv.at>
Дата:
>> 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()?
От:
Dave Cramer <pg@fastcrypt.com>
Дата:
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()?
От:
Dave Cramer <pg@fastcrypt.com>
Дата:
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()?
От:
Dave Cramer <pg@fastcrypt.com>
Дата:
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()?
От:
Dave Cramer <pg@fastcrypt.com>
Дата:
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()?
От:
Kris Jurka <books@ejurka.com>
Дата:
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()?
От:
Kris Jurka <books@ejurka.com>
Дата:
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()?
От:
Kris Jurka <books@ejurka.com>
Дата:
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()?
От:
Kris Jurka <books@ejurka.com>
Дата:
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()?
От:
Kris Jurka <books@ejurka.com>
Дата:
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()?
От:
Kris Jurka <books@ejurka.com>
Дата:
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()?
От:
Kris Jurka <books@ejurka.com>
Дата:
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()?
От:
Kris Jurka <books@ejurka.com>
Дата:
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: Patch for Statement.getGeneratedKeys()
От:
Kris Jurka <books@ejurka.com>
Дата:
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()
От:
Kris Jurka <books@ejurka.com>
Дата:
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()
От:
Kris Jurka <books@ejurka.com>
Дата:
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()
От:
Kris Jurka <books@ejurka.com>
Дата:
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()
От:
Kris Jurka <books@ejurka.com>
Дата:
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()
От:
Kris Jurka <books@ejurka.com>
Дата:
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()
От:
Kris Jurka <books@ejurka.com>
Дата:
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
