Home » Database » DB_DataObject » Bug #3773

[2005-03-11 13:24 UTC] ej dot grace at imperial dot ac dot uk

Description:
------------
[PREFIX]
This appears to have been mentioned in bug #57 Auto Sequence problem with PostgreSQL. This bug report does however contain a workaround which may be useful.

There appears to be a bug in DB::DataObject which causes the autoincrement of primary keys to use a different table to the default table created by PostgreSQL when specifying the SERIAL type.

I suspect that the function ->keys(), ->sequenceKey() et al. does not return the correct name of the primary key. For this reason the internal bits of DB::DataObject determine the sequence table to be [TABLENAME]_seq rather than the PostgreSQL default [TABLENAME]_[KEYNAME]_seq.

This results in TWO sequence tables, one that is consulted when doing an explicit INSERT query, the other when using the ->insert() member function.

Needless to say these can (and do) clash.

The workaround is as follows:

Instead of using SERIAL, which implies

colname integer DEFAULT nextval('tablename_colname_seq') NOT NULL

Specify the key as:

id BIGINT DEFAULT nextval('idclash_seq') NOT NULL

This will mean ->insert() and the pgsql INSERT command will use the same sequence key table.

Reproduce code:
---------------
Starting from a blank database:

CREATE TABLE idclash (
id SERIAL,
PRIMARY KEY id
);

Then at the psql command line do:

INSERT INTO idclash (id) VALUES (default);
INSERT INTO idclash (id) VALUES (default);

In a script call the insert member function:
->insert()
->insert()
->insert()

Finally at the psql command line do

INSERT INTO idclash (id) VALUES (default);
SELECT id FROM idclash;

Expected result:
----------------
id |
----|
1 |
2 |
3 |
4 |
5 |
6 |
(6 rows)

Actual result:
--------------
id |
----
1 |
2 |
3 |
(3 rows)

[2005-03-14 19:06 UTC] ej dot grace at imperial dot ac dot uk

> the official fix for this is to define sequenceKey:

That follows the advice given to me by Justin Patrin a developer on DataObject::FormBuilder. I think something is seriously broken as neither sequenceKey redefinitions work, DB::DataObject is still trying to get its sequence from the wrong sequence table.

o.k. create table from scratch, dropping any sequences if they are defined

DROP TABLE idclash;
DROP SEQUENCE idclash_seq;
DROP SEQUENCE idclash_id_seq;

CREATE TABLE idclash (
id SERIAL,
PRIMARY KEY (id)
);

Overload sequenceKey in Idclash.php such that it follows the same form as below:

> // for native postgres sequences.
> function sequenceKey() { return array('colname', true); }

function sequenceKey() {
return array('id',true);
}

Result of three ->insert() operations

ojdb=# SELECT last_value FROM idclash_seq ;
last_value
------------
3
(1 row)

ojdb=# SELECT last_value FROM idclash_id_seq;
last_value
------------
1
(1 row)

In other words it still creates the tablename_seq sequence and ignores the existing one.

Likewise if I define it as:

> // for DB compatible postgres sequences.
> function sequenceKey() { return array('colname', false,
>'tablename_colname_seq'); }

function sequenceKey() {
return array('id',false,'idclash_id_seq');
}

This gives the same (wrong) result:

ojdb=# SELECT last_value FROM idclash_seq ;
last_value
------------
3
(1 row)

ojdb=# SELECT last_value FROM idclash_id_seq;
last_value
------------
1
(1 row)

Version info:

DB 1.7.2 stable
DB_DataObject 1.7.7 stable

Surely the sequenceKey() method should enquire from the database what the correct sequence is and use that?

In case it is of any use the DataObject looks like this:

<?php
/**
* Table Definition for idclash
*/
require_once 'DB/DataObject.php';

class DataObjects_Idclash extends DB_DataObject
{
###START_AUTOCODE
/* the code below is auto generated do not remove the above tag */

var $__table = 'idclash'; // table name
var $id; // int4(4) not_null default_nextval%28public.idclash_id_seq%29 primary_key

/* ZE2 compatibility trick*/
function __clone() { return $this;}

/* Static get */
function staticGet($k,$v=NULL) { return DB_DataObject::staticGet('DataObjects_Idclash',$k,$v); }

/* the code above is auto generated do not remove the tag below */
###END_AUTOCODE

function sequenceKey() {
return array('id',false,'idclash_id_seq');
}

}

Regards,

-ed

[2005-03-16 09:20 UTC] ej dot grace at imperial dot ac dot uk

My apologies. Repeating the tests again I see I made an error:

array('id',false,'idclash_id_seq');

Gives the correct behaviour.

Just to reiterate, after a fresh recreation of the table and dropping of sequence tables, 6 ->insert() opterations give the following with:

return array('id',true), the result is bool(false). idclash_id_seq.lastvalue = 1, idclash_seq does not exist:

return array('id',false,'idclash_id_seq'), the result is string ("1"), ("2") etc.. idclash_id_seq.lastvalue=6, idclash_seq does not exist.

return array('id',true,'idclash_id_seq'), the result is
bool(false). idclash_id_seq.lastvalue=1, idclash_seq does not exist.

So, to reaffirm

function sequenceKey() {
return array('id',false,'idclash_id_seq');
}

*DOES* give the correct behaviour.

Can this not be made the default behaviour for dataobjects when using postgres? It seems silly to use the createTables.php script, only to have to overload each and every DO with the same type of function.

[2005-03-21 17:13 UTC] phil at africacomputing dot org

With the official fix to define sequenceKey with postgresql :

function sequenceKey() { return array('colname', true); }

insert works fine but there is now a problem with update operations :

- without the fix : update OK, insert KO
- with the fix : update KO, insert OK

ERROR: update: trying to perform an update without the key set, and argument to update is not DB_DATAOBJECT_WHEREADD_ONLY

An idea ?

[2005-03-21 17:20 UTC] phil at africacomputing dot org

My apologies.

I did an error in the colname : it's why insert operations are working and not update operations !

;-)

[2005-05-10 16:44 UTC] ej dot grace at imperial dot ac dot uk

In my opinion the original bug is not fixed. Surely sequenceKey() and its relatives should work out of the box for postgres?

[2005-06-27 08:40 UTC] ej dot grace at imperial dot ac dot uk

> postgres has 2 ways to define sequences/serial etc.
>
> id SERIAL, -which doesnt work
>
> id default nextval('my_sequence') not null; - which works.

Indeed. SERIAL is in fact an alias for

DEFAULT nextval('tablename_colname_seq') not null

> If we support SERIAL, how do you get the last inserted id back from the
> insert() call?

I imagine in the same manner as it is done at the moment.

$pgsql_key = $DB->getOne("SELECT last_value FROM ".$seq);

I think the point is that it is getting the last_value from the wrong SEQUENCE!

A more detailed (and possibly redundant) explanation follows..

=======================================

Ok, I should begin by saying I don't thoroughly know the inner workings of DataObject.php or its design philosophy, however I think the following stands to reason without this knowledge.

What I think is going wrong is that the case for pgsql is not functioning correctly in ->insert() where the


// nativeSequences or Sequences..

// big check for using sequences

section occurs. For this reason I think it picks up the wrong sequence name, e.g. from :

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;
}

My reason for this line of thought is the following:

If I define a table key as SERIAL the DataObject code (incorrectly) creates a sequence table named:

table_seq

e.g. person_seq

it if does not exist which DataObject uses to keep track of sequences, whereas for SERIAL postgres defines the sequence:

table_column_seq

e.g. person_id_seq

which *it* uses to keep track of IDs. The fact that there can be two separate sequences relating to the same table and key is what causes the mismatch between ->insert() and INSERT.

If one can make the DataObject use the correct sequence, i.e. table_column_seq instead of table_seq then it will behave in a consistent manner with the SQL INSERT.

This is why defining the automatically incrementing key with:

CREATE TABLE tablename (
id BIGINT DEFAULT nextval('tablename_seq') NOT NULL,
..
..
..
);

works. Since it forces Postgres to generate a sequence called tablename_seq, which is the same sequence DataObjects will use to keep track of keys. Rather than tablename_id_seq which is what would be generated using SERIAL as the type.

After all SERIAL is just an alias for

BIGINT DEFAULT nextval('tablename_colname_seq') NOT NULL

in postgres.

-ed

[2006-10-13 12:21 UTC] mcraig at leadehealth dot com

I've been working with sequences in PostgreSQL >=7.4.7 and a modified DataObject for about 2 years without problems. The whole problem is in DB with the sequence code. (Sorry I never got the fix in. I submitted a change once but the version had bumped up a few times and my patch was no longer usable.)

Regardless of using "SERIAL" or "nextval(...)" in the create table command, Postgres keeps the string "DEFAULT nextval(...)" in the schema definition and it is easy for DataObject to use it correctly.

I will submit a patch for this against the most current version of DataObject because there is also a fix in there for a race condition against selecting nextval from the sequence.