[development] A gotcha with auto-increment and MySQL

[Darrel O'Pry]

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.