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

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