db_last_insert_id returns watchdog (wid) if comment (cid) is selected

There is nothing to fix here. db_last_insert_id() can only return... the last inserted id, even if, for implementation reason, you have to pass it the table and field.

You will have to grab the cid by other means.

This is only a documentation issue. db_last_insert_id() only returns the last inserted id. Period.

Drupal 7 is not affected (db_last_insert_id() is deprecated in D7). Feel free to suggest a patch clarifying that fact.

Again, the bug is in your code.

Don't use db_last_insert_id() to retrieve anything other than the last insert id of the last executed SQL statement. That behavior is consistent in every version of Drupal (< 7), and I don't believe it is worth changing it. But, still, fell free to suggest a patch to clarify the documentation.

On Drupal 7, db_insert() returns the inserted id of the query, and db_last_insert_id() is deprecated.

Damien,

According to the Drupal API documentation, there are 10 functions in core that use db_last_insert_id() in Drupal 6. Since the identifier returned by the MySQL implementation is the absolute last identifier added to the database and not bound by any transactional context, this call can potentially return false data -- particularly when the site is under load.

FWIW, we have also experienced this issue with the ID of a watchdog row being returned after creation of a row in another table. I understand what the function does and appreciate that it shouldn't be used for our purpose, but I strongly suggest that the implementation of the function is critically flawed and, since it is used in core, this issue should move back to critical priority. If the function shouldn't be used and is not used by any core code, I agree that the priority should be changed to Normal and documentation added to warn against its use and suggest workarounds.

The code in question is:

<?php
db_query('insert into {term_data} (vid, name, description) values (%d, \'%s\', \'\')', $vid, $term);
$tid = db_last_insert_id();
db_query('insert into {term_hierarchy} (tid, parent) values (%d, 0)', $tid);
?>

The tid being returned is the ID of a row being added to the watchdog table and not of the entry being added to the term_data table.

M

Sorry, we've just tried again and are unable to replicate the issue we were experiencing earlier. We attempted with one user on a breakpoint after the initial db_query() and another user creating content (and thus updating the watchdog table).

Confusing.

Once again there is no bug at all here.

From http://dev.mysql.com/doc/refman/5.0/en/information-functions.html#functi...

The ID that was generated is maintained in the server on a per-connection basis. This means that the value returned by the function to a given client is the first AUTO_INCREMENT value generated for most recent statement affecting an AUTO_INCREMENT column by that client. This value cannot be affected by other clients, even if they generate AUTO_INCREMENT values of their own. This behavior ensures that each client can retrieve its own ID without concern for the activity of other clients, and without the need for locks or transactions.

But of course, if you let another query execute (for example a watchdog query) between the query you are targeting and the call to db_last_insert_id(), for sure the last inserted id will be different. The documentation of that function clearly states:

Returns the last insert id. This function is thread safe.

For the record, you need to pass the $table and $field to that function because PostgreSQL needs that information. But still, you can only get the *last* inserted id (ie. the one potentially inserted by the very last query).

I have to agree with hass...

I need an auto_increment id from an insert to a table. If the insert succeeds i can use that id to insert records to another table that references the first.

I get the correct id when the query succeeds, but when it fails i get the id of the watchdog. On top of that, when i look for errors using db_error() i don't get an error, because the insert to watchdog succeeded...? Logic? Maybe it is logical, but I don't see it.

There is a solution to my problem however... At least I hope there is, using drupal_write_record... I am trying it out next:

table testTable:
id int(10) unsigned auto_increment not null primary key
label varchar(150) not null unique key
description mediumtext null

$record = new stdClass();
$record->label = "a label";
$record->description = "a description";
drupal_write_record("testTable", $record);

according to the API it should set the incremented value in $record->id and if the query fails it should not be set... So let's find out...

And yes it works! Pitty though because i like to use db_query() but this will be the way around for me.

I am a bit puzzled though... the drupal_write_record function uses db_query and db_last_insert_id to achieve this.

// Execute the SQL.
if (db_query($query, $values)) {
if ($serials) {
// Get last insert ids and fill them in.
foreach ($serials as $field) {
$object->$field = db_last_insert_id($table, $field);
}
}
}
else {
$return = FALSE;
}

And now I feel a little bit like an idiot =D.

A watchdog is written when an error occurs. But of course db_query() returns false if an error occurs... so if an error occurs db_last_insert_id() will return the id of the watchdog added. That is the nature of MySQL...

This is definitely not a bug.

@hass: if you have no proper hook and / or information in a submit function, you are screwed. Don't rely on the database layer to do what a hook should do. Open a feature request against those.

You can call this not a bug by logic, but this practically is and i fully quote hass. An abstraction layer should return the current query last id, not the mysql/pgsql/wtf implementation insert id. That does not abstract us from anything.

Anyway, i did hack the core the same way i did in my own cmf: I added a custom global var on the very next line after the query ran, i have no time to waste in useless turnarounds tipically cascading other issues:

database.mysqli.inc

$result = mysqli_query($active_db, $query);
$GLOBALS['brazorf']['last_insert_id'] = mysqli_insert_id($active_db);

As easy.