Re: [Patch] DB_DataObject and Postgres INSERT race condition obtaining last key

Justin Patrin wrote: > On 10/13/06, Alexey Borzov <borz_off@cs.msu.su> wrote: > >> Hi, >> >> Justin Patrin wrote: >> >> Currently, the actual INSERT INTO statement is completed and then >> >> later in the code the >> >> sequence itself is checked to see what the last used value of the >> >> sequence was. In between >> >> those two steps it is very easy for another INSERT INTO statement for >> >> the same table in a >> >> different object to increment the sequence a second time before the >> >> first object even checks >> >> what the value of the sequence is. The end result is that both >> >> objects return the same last >> >> insert key. One of them incorrectly, of course. >> >> >> >> This code fixes the problem by recognizing autoincrement fields for >> >> PostgreSQL and selecting a >> >> next value from the sequence to insert, rather than letting the >> >> automatic nextval() function >> >> run. This ensures that the same key inserted into the table gets >> >> returned as the last inserted >> >> key. >> >> >> >> The final section of the patch is in the joinAdd function to allow >> >> auto joins where the tables >> >> in the join share a column name given as the parameter $joinCol. This >> >> is a feature, rather >> >> than a bug fix and could be dropped, but the pgsql INSERT code is, in >> >> my estimation, essential. >> >> >> >> This code has been tested and in production for about 3 months on both >> >> a Postgres 7.4.13 and >> >> 8.0 installation. >> >> >> > >> > Good catch, but I don't think DB_DataObject is the place to fix this. >> > If there is indeed a race condition (have you seen it actually >> > happen?) then this should probably be fixed in DB's sequence handling, >> > not in DB_DO. >> >> I've had a brief look, the bug is in DB_DO, it incorrectly gets the >> last value >> of the sequence: >> $pgsql_key = $DB->getOne("SELECT last_value FROM ".$seq); >> Should be instead: >> $pgsql_key = $DB->getOne("SELECT currval('".$seq . "')"); >> >> PostgreSQL's built-in currval() function automatically takes care of >> giving the >> proper value. >> >> Of course, this fix is also a bit shorter than the proposed patch. :-D >> > > I knew there has to be a way to get the connection's last sequence > value. ;-) Nice find. > > Matt: could you try this instead of your patch and see if it fixes your > problem? > Much better fix. I will test this. A question remains in my mind about connection pooling. currval() says in the documentation "Notice that because this is returning a session-local value, it gives a predictable answer even if other sessions are executing nextval meanwhile." on http://www.postgresql.org/docs/7.4/static/functions-sequence.html With pooled connections through the web server are these pooled connections considered the same session?