Document that delayed INSERT queries do not return a usable last insert id
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | database system |
| Category: | task |
| Priority: | minor |
| Assigned: | Crell |
| Status: | closed |
According to CVS HEAD MySQL driver implementation, db_insert() is implement with INSERT DELAYED syntax, which will cause critical bug when working together with db_last_insert_id() (http://dev.mysql.com/doc/refman/5.0/en/insert-delayed.html):
Because the INSERT DELAYED statement returns immediately, before the rows are inserted, you cannot use LAST_INSERT_ID() to get the AUTO_INCREMENT value that the statement might generate.
Combine INSERT statement + db_last_insert_id() + insert id into other table is all over Drupal core. Whenever update legacy INSERT query with db_insert() (e.g. #299385, #299088 and #300203), this trap will always be triggered. For some special case, db_insert() is even function correctly when working TEXT but buggy whenever replace field as BLOB.
According to my understand, the INSERT DELAYED is design for MySQL master/slave architecture, based on extensibility concern; BTW, the usability of db_insert() will greatly decreased based on this limitation. Any input for this issue?
#1
Well, insert queries do not use delay unless you explicitly flag them to. Since nothing in core is currently delayed, that is a non-issue at the moment.
Also, db_last_insert_id() is slated for removal after the legacy code is ported. db_insert()->execute() will return the inserted ID if relevant. If you tell the query to be delayed, then you know you don't care about the insert ID.
So at best this is a minor documentation issue that we should note that fact in the InsertQuery::delay() docblock. Refiling it as such.
#2
@Crell: thanks for information and it is very useful. Already review patches #299385, #299088 and #300203, update them with correct use of db_insert() and remove use of db_last_insert_id(). Your kindly review may provide more example for db_insert() within core :-)
On the other hand, I think documentation is very useful, too. Since I also get loss when using db_insert(), and its relationship with db_last_insert_id()...
#3
This is just a trivial doc change, no code, so I'm going out on a limb and pushing it to RTBC.
#4
Committed to CVS HEAD. Thanks!
#5
Automatically closed -- issue fixed for two weeks with no activity.