drupal_write_record() fails on Postgres (Patch passed on MySQL)

replicated, and looking at it.

first attempt results in :

arning: pg_query(): Query failed: ERROR: currval of sequence "node_nid_seq" is not yet defined in this session in /home/brenda/projects/drupalcore/HEAD/includes/database.pgsql.inc on line 155.
user warning: query: SELECT currval('node_nid_seq') in /home/brenda/projects/drupalcore/HEAD/includes/database.pgsql.inc on line 248.
warning: pg_query(): Query failed: ERROR: currval of sequence "node_revisions_vid_seq" is not yet defined in this session in /home/brenda/projects/drupalcore/HEAD/includes/database.pgsql.inc on line 155.
user warning: query: SELECT currval('node_revisions_vid_seq') in /home/brenda/projects/drupalcore/HEAD/includes/database.pgsql.inc on line 248.
The node could not be saved.

subsequent attempts results in:

warning: pg_query(): Query failed: ERROR: duplicate key violates unique constraint "node_pkey" in /home/brenda/projects/drupalcore/HEAD/includes/database.pgsql.inc on line 155.
user warning: query: INSERT INTO node (nid, vid, type, language, title, uid, status, created, changed, comment, promote, moderate, sticky, tnid, translate) VALUES (0, 0, 'story', '', 'adfds', 1, 1, 1192248666, 1192248666, 2, 1, 0, 0, 0, 0) in /home/brenda/projects/drupalcore/HEAD/includes/common.inc on line 3185.
warning: pg_query(): Query failed: ERROR: currval of sequence "node_nid_seq" is not yet defined in this session in /home/brenda/projects/drupalcore/HEAD/includes/database.pgsql.inc on line 155.
user warning: query: SELECT currval('node_nid_seq') in /home/brenda/projects/drupalcore/HEAD/includes/database.pgsql.inc on line 248.
warning: pg_query(): Query failed: ERROR: duplicate key violates unique constraint "node_revisions_pkey" in /home/brenda/projects/drupalcore/HEAD/includes/database.pgsql.inc on line 155.
user warning: query: INSERT INTO node_revisions (nid, vid, uid, title, body, teaser, log, timestamp, format) VALUES (0, 0, 1, 'adfds', 'asdfads', 'asdfads', '', 1192248666, 1) in /home/brenda/projects/drupalcore/HEAD/includes/common.inc on line 3185.
warning: pg_query(): Query failed: ERROR: currval of sequence "node_revisions_vid_seq" is not yet defined in this session in /home/brenda/projects/drupalcore/HEAD/includes/database.pgsql.inc on line 155.
user warning: query: SELECT currval('node_revisions_vid_seq') in /home/brenda/projects/drupalcore/HEAD/includes/database.pgsql.inc on line 248.
warning: pg_query(): Query failed: ERROR: duplicate key violates unique constraint "node_comment_statistics_pkey" in /home/brenda/projects/drupalcore/HEAD/includes/database.pgsql.inc on line 155.
user warning: query: INSERT INTO node_comment_statistics (nid, last_comment_timestamp, last_comment_name, last_comment_uid, comment_count) VALUES (0, 1192248666, NULL, 1, 0) in /home/brenda/projects/drupalcore/HEAD/modules/comment/comment.module on line 588.
The node could not be saved.

I experienced the same behavior with the development version of d6 (// $Id: CHANGELOG.txt,v 1.226 2007/10/10 10:40:13 goba Exp $), running PHP Version 5.2.1, PostGres 8.2.3, Apache 2.2.4). I don't know common.inc well enough to patch it yet, but this is what I did to get posting, at least. So maybe what I did will help others. Forgive the verbosity, I just want to make sure I understand.

The bottom line seems to be that when you do an INSERT in PostGres, and one of the fields is a SERIAL field, you shouldn't pass a value into the SERIAL field at all, because that will override the autogenerated value.

So, in common inc, when collecting the values thus:

    // Track serial fields so we can helpfully populate them after the query.
    if ($info['type'] == 'serial') {
      $serials[] = $field;
      // Ignore values for serials when inserting data. Unsupported.
      $object->$field = 'NULL';
    }

we end up with a NULL value in the "nid" field and a 0 in the "vid" field because that is what the {node} schema drives:

      'nid' => array(
        'type' => 'serial',
        'unsigned' => TRUE,
        'not null' => TRUE)...
      'vid' => array(
        'default' => 0)...

We then pass INSERT INTO {node) (nid,vid) VALUES (NULL,0) ... to db_query, which converts the NULL into a zero, so we get INSERT INTO {node) (nid,vid) VALUES (0,0). Then we come, in common.inc, to this line, which fires if we have serials (nid):

      $nid = db_last_insert_id($table, $field);

In database.pgsql.inc that translates to:

"SELECT currval ...

But currval can only return a value if nextval() has previously been called, which is not the case because we overrode the default value supplied by SERIAL sequence generator by passing in a value. And so we get the "currval of sequence 'node_nid_seq' is not yet defined in this session" error instead of the page confirmation.

By this time, Drupal has ended up populating the nid and vid values (even though the post hasn't been saved), but not auto-incrementing them (we over-rode the serial genator), so on subsequent attempts, which again pass in ..VALUES (0,0)..., we get the uniqueness constraint violations.

So, just to start inserting nodes again, I changed the default values of nid and vid in {node} to look like:

nid: nextval('node_nid_seq'::regclass) 	<-- nextval does the auto-incrementing
vid: currval('node_nid_seq'::regclass) 	 <-- same value as nid generated by nextval

In database.pgsql.inc I changed currval to a SELECT so there would be no dependency on nextval:

function db_last_insert_id($table, $field) {
  // $result = db_result(db_query("SELECT currval('%s_seq')", db_prefix_tables('{'. $table .'}') .'_'. $field));
     $result = db_result(db_query("SELECT last_value FROM %s_seq", db_prefix_tables('{'. $table .'}') .'_'. $field));
}

And then I made use of the helpfully collected serial fields in common.inc:

  foreach ($schema['fields'] as $field => $info) {
    // NEW: collect known serial fields on INSERT
    // WARNING: PostGres only. This is bad.
    if ($info['type'] == 'serial' && (in_array($field,array('nid','vid'),TRUE) === TRUE) && (count($update) == 0))  {
      $serials[]= $field;
      // the nid/vid field/values never get into $values or $placesholders, so they never get into the query at all, 
      // and then the SERIAL generators in PostGres don't get over-ridden with default values.
      continue;
    }
    // Special case -- skip serial types if we are updating.
    if ($info['type'] == 'serial' && count($update)) {
      $serials[]= $field;
      continue;
    }

Now fill in the serial fields in PostGres:

  if ($serials) {
    // Get last insert ids and fill them in.
    foreach ($serials as $field) {
      $nid = db_last_insert_id($table, $field);
      $object->$field = $nid;
   }
  }

This now words because the serials are collected on INSERT as well as UPDATE, and by the time db_last_insert is called, the {node} table has been populated by the SERIAL generators to db_last_insert_id has a nid to retrieve.

I hope this is helpful to people who know both common.inc and PostGres better than I do, and can move this issue forward.

Raised priority to critical.

The attached patch file changes the line "$object->$field = 'NULL';" in the drupal_write_record function in common.inc to "unset($object->field);". The effect is that the insert query doesn't want to set the serial field to a NULL value but instead auto-increments. It seems to solve the insert, but i don't know what else is affected, not to mention when using MySQL.

Bug is also present in the beta2 version.

Patch tested for both MySQL 5.0.32 and PostgreSQL 8.1.9 on Debian etch 4.0r1 with PHP 5.2.0-8+etch7, all passed for node creation, update with revision, and comment add/drop.

The change sounds logical, as we need not to include those auto incremental field within SQL, even though it is set as NULL, and works with lossy MySQL handling.

Patch reroll with latest CVS HEAD, and keep the patch syntax as clean. It should be RTBC.

Thanks, committed.

The patch works for me on beta2 (to which, apparently, it has not yet been rolled in?)

I guess it is committed? CVS msg: http://drupal.org/cvs?commit=85124

Yes, beta2 was released before this patch was committed.