Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.This is also a problem in D6, but with db_affected_rows() it is possible to get around the issue.
Basically the issue is that when you call drupal_write_record() the only time that it is will return FALSE is when there is an error in the SQL. but without db_affected_rows() there is no way to determine if changes have been made.
In Drupal 6 you would do something like
$return = drupal_write_record('table', $edit, 'id');
if (!db_affected_rows()) {
drupal_write_record('table', $edit);
}
As long as the SQL executes we have no idea if the if the rows have been updated unless we check affected rows.
Since there is now no equiv. db_affected_rows() we can't do this.
since returning SAVED_NEW/SAVED_UPDATED is pretty much useless since you know this from how you called drupal_write_record() I have changed it to return the number of affected rows.
So now you can do something like
if (!drupal_write_record('table', $edit, 'id')) {
drupal_write_record('table', $edit);
}
which is actually much nicer than the existing implementation.
| Comment | File | Size | Author |
|---|---|---|---|
| 0001-Change-return-of-drupal_write_record-to-return-the-n.patch | 8.15 KB | gordon | |
Comments
Comment #2
Crell CreditAttribution: Crell commentedTo be honest, I consider drupal_write_record() to be vestigial at this point. Just use db_insert(), db_update(), and db_merge() directly. The latter even gives you a constant back to determine whether it was an insert or update (as of about 3 days ago :-) ). And db_insert() will tell you what the just-created serial is, if any.
Comment #3
gordon CreditAttribution: gordon commentedThe new db_insert() is good, but it is not as good as drupal_write_record() for dealing with random data.
Basically I have tables which I allow people to use hook_schema_alter() to add additional fields to the table, and then they can use hook_form_alter() to add the fields to the object, and I just have to pass the entire $form_state['values'] to drupal write record and all fields including ones I know nothing about will get saved as well.
An example is if I was add field foo to the node table. I alter the node edit form and add my foo field, and since node_save() passes the entire $node to drupal_write_record() the foo field I added is also updated. No need for an additional update to write to the foo field.
I am currently working on the new version of the patch, it will not be a big patch but it is a lot more fiddly than I first thought.
Comment #4
gordon CreditAttribution: gordon commentedI am still working on this, but I am having problem getting the information out of some of the objects, like the number of affected rows.
Comment #5
gordon CreditAttribution: gordon commentedAfter some more work on one of my modules, I think what is needed is to have a merge option.
My biggest problem is that when you are saving additional data from something like a hook_node_update() and you have no idea if the row already exists for a nid/vid in the table. An idea I have is to use add an additional option to specify that a merge needs to be done, and then a db_merge() is done instead of the db_update()
The main problem with this is that db_merge() doesn't support serial fields. I don't think this is a big problem since in the above example it will not be a problem, and if you are writing to a table with a serial field you will know that the serial field as a value and can call drupal_write_record() with either an insert or an update.
Comment #6
Crell CreditAttribution: Crell commentedIs this still an issue? drupal_write_record() has several RTBC patches against it that will affect this, but I don't think it's even a drupal_write_record() issue in the first place at this point:
#755856: Don't insert default values into db in drupal_write_record()
#797680: drupal_write_record() unnecessarily sets primary keys while updating
Comment #7
mr.baileysThis was fixed in D7 by #626790: drupal_write_record returns FALSE for valid update queries and needs to be backported.
Comment #8
brad.bulger CreditAttribution: brad.bulger commentedthe change in #626790 was never implemented, or if it was, it was backed out. so we are still left with the original problem: in 7.x, without db_affected_rows(), there is no way to tell if drupal_write_record() actually changed anything.
Comment #9
Dave Cohen CreditAttribution: Dave Cohen commentedTo my way of thinking, gordon's original description of the problem is spot on, and his proposed solution makes complete sense.
Obviously, since years have gone by, I accept this is one of those wins that will never make it into Drupal. Too bad.
But shouldn't we at least change the misleading documentation?