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

I've used the currval() fix - can you check this works and let me know. Thanks Alan Matt Craig wrote: > DB_DataObject has a race condition in the PostgreSQL insert() + last inserted key code. > > 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. > > matt > > <ESC>:wq > > ------------------------------------------------------------------------ > > ? DataObject.diff > Index: DataObject.php > =================================================================== > RCS file: /repository/pear/DB_DataObject/DataObject.php,v > retrieving revision 1.419 > diff -u -r1.419 DataObject.php > --- DataObject.php 4 Sep 2006 02:53:20 -0000 1.419 > +++ DataObject.php 13 Oct 2006 12:40:07 -0000 > @@ -937,6 +937,44 @@ > } > $this->$key = $keyvalue; > } > + else if ($key && $useNative) { > + // postgresql has a race condition if you try to get the last_value from > + // a sequence in the middle of multiple transactions inserting into the same table > + // so in this case we will select the nextval before doing the insert and use it > + if($dbtype === 'pgsql') { > + // by this point we know this is a nextval field in postgres > + $defs = $DB->tableInfo($this->__table); > + foreach($defs as $def){ > + if($def['name'] == $key) { > + $f = rawurldecode($def['flags']); > + if (preg_match("/nextval\((.*)\)/i", $f, $seqs)) { > + // gotta put some quotes around this since the table description > is no helpful > + // I'd much rather use $seqs[0][ > + $seq = "nextval('{$seqs[1]}')"; > + } > + break; > + } > + } > + if (!$seq) { > + $seq = $DB->getSequenceName($this->__table ); > + } > + > + $pgsql_key = $DB->getOne("SELECT $seq"); > + if (PEAR::isError($pgsql_key)) { > + $this->raiseError("error in sequence: $seq"); > + return false; > + } > + else if(intval($pgsql_key)!=$pgsql_key){ > + // we have a real problem here > + $this->raiseError('postgres seq value is not an integer'); > + return false; > + } > + $this->$key = $pgsql_key; > + } > + } > + > + > + > > > > @@ -944,7 +982,10 @@ > > // if we are using autoincrement - skip the column... > if ($key && ($k == $key) && $useNative) { > - continue; > + if($dbtype!=='pgsql'){ > + // we already fetched an key for pgsql and it needs to be added to the > fields=values > + continue; > + } > } > > > @@ -1064,15 +1105,8 @@ > break; > > case 'pgsql': > - if (!$seq) { > - $seq = $DB->getSequenceName($this->__table ); > - } > - $pgsql_key = $DB->getOne("SELECT last_value FROM ".$seq); > - if (PEAR::isError($pgsql_key)) { > - $this->raiseError($r); > - return false; > - } > - $this->$key = $pgsql_key; > + // break right out of this. We got the sequence above and we'll > use that instance > + // TODO: this code should be removed entirely > break; > > case 'ifx': > @@ -2022,7 +2056,7 @@ > // technically postgres native here... > // we need to get the new improved tabledata sorted out first. > > - if ( in_array($dbtype , array( 'mysql', 'mysqli', > 'mssql', 'ifx')) && > + if ( in_array($dbtype , array( 'mysql', 'mysqli', > 'mssql', 'ifx', 'pgsql')) && > ($table[$usekey] & DB_DATAOBJECT_INT) && > isset($realkeys[$usekey]) && ($realkeys[$usekey] == 'N') > ) { > @@ -3112,6 +3146,13 @@ > } > } > > + // finally if these two table have column names that match do a join by default on > them > + if($ofield === false && $joinCol) { > + $ofield = $joinCol; > + $tfield = $joinCol; > + } > + > + > /* did I find a conneciton between them? */ > > if ($ofield === false) { > >