On Thu, 2007-05-03 at 12:25 -0400, Barry Jaspan wrote:
The most recent thread on using auto-incrementing columns in Drupal seems to have led to the decision that it is okay to use them (acknowledging that both our mysql and pgsql tables already do use them), and the upcoming database schema data structure supports them. However, while in my waking-up haze this morning, out of nowhere I realized one of the reasons they were probably previously avoided.
On an auto-incrementing column, a race condition exists if db_next_id() is sometimes used and sometimes not used. The problem is that while db_next_id() is atomic it does not alter MySQL's idea of the next id to assign. Suppose at T0 the maximum nid in the node table is 10 and the sequences table reflects that. At T1, one Drupal process calls db_next_id('{node}_nid'). The sequences table is atomically updated to 11 and the caller gets 11. At T2, a second Drupal process inserts a new row into the node table without providing a nid; since the maximum nid is 10, the new row is assigned nid 11. At T3, the first process tries to insert a row using the nid 11 it got from db_next_id() and fails.
This problem does not occur with pgsql because the db_next_id() and the auto-incrementing column both use the same underlying 'sequence' object.
I see two ways to solve this problem:
1. Disallow auto-incrementing columns everywhere in Drupal. Note that they are used on numerous core tables both with mysql and pgsql. To remove them, we would have to verify that every insert into a table uses db_next_id() to generate the new id.
2. Just don't use db_next_id() and auto-increment on the same columns, ever. We could just declare this a bug and fix it when discovered. However, now that we (will soon) have a database schema, we could also make db_next_id() actively warn about it:
function db_next_id($name) { list($table, $col) = preg_split_table_and_column_name($name); $schema = drupal_get_schema(); if ($schema[$table]['cols'][$col]['type'] == 'serial') { // log an error here }
// rest of db_next_id() here }
Thoughts?
Barry
I'd say 2. I think best practice is to decide if you want to use auto_increment or sequences for your table and stick with one. However, just because both Postgres has something auto_increment like doesn't mean they're equivalent. Auto increment is bad for anyone tinkering with multi-master environments. I've seen the light. I'll take my ticket from db_next_id and wait in the queue. .darrel.