Better documentation in query.inc

Here's the patch that adds a (failing) test for this.

I looked through the entire code base to find how db_update()->execute() return values were being used (usually they are ignored):
1) drupal_write_record() seems to be assuming it's returning the ID of the record affected? That code sure looks weird and bad.
2) lock_acquire() -- I'm not sure, probably not a problem. I think every time it updates it should be a new value, so probably OK.
3) comment_operations() - not sure if this is an issue or not, but it's returning update query objects (unexecuted) for other functions to use, apparently?
4) node_type_update_nodes() - this one is currently OK, as it explicitly says it returns the number of nodes whose types were changed. But if we change the return value to pass my test above, then it will probably break this function's return value?
5) There are quite a few tests in database_test.test that use db_update's return value, but as they are all passing now, they are probably testing actually changing rows in the DB.
6) SystemQueue::claimItem() and releaseItem() -- not sure if these are OK or not.

I am pretty sure these are the only places in core where the return value of db_update()->execute() is used.

Well, most of the query classes and interfaces are missing doc on the methods, for instance. Where's the return value of db_update()->execute() documented?

Anyway, I this this is a doc issue, assuming that none of those places in the code in #2 is doing something wrong. Need to check that first, then move this to the doc component.

It's a doc bug then, not a database task. :)

There is a meta-issue now for documenting classes in Drupal in general:
#718576: [Meta] API documentation for classes and interfaces is incomplete

There is also an issue for discussing standards for documenting classes and methods:
#718596: Lacking standards for documenting classes, interfaces, methods

Looking through the functions that are using the return value of db_update() queries to make sure no one is making an error of logic like the one that was there in search.module for D6...

They all look fine to me, except drupal_write_record():

if( something) {
   $query = db_insert($table, $options)->fields($fields);
}
else {
   $query = db_update($table)->fields($fields);
}
// ...
  if ($last_insert_id = $query->execute()) {

That looks wrong. If the db_update succeeds in updating a row, this if() will be true and it will think it's the last inserted ID? WTF?

RE: #8: #719468: drupal_write_record() code is written in a confusing way

So can anyone interested in this issue as a doc bug please go comment on #718596: Lacking standards for documenting classes, interfaces, methods, because I would like us to have standards on what exactly needs to have docblocks in classes. Thanks...

This needs to be postponed until coding standards for classes have been defined. See #7 above.

Doc standards are now defined:
http://drupal.org/node/1354#classes

That was an excellent start. I went through the file and found some more things needing fixes. Here's a new patch.

#15: 718540-morefixes.patch queued for re-testing.

#15: 718540-morefixes.patch queued for re-testing.

Here's a rerolled patch.

Hmph. The failed test is:

The allowed text format appears as an option when adding a new node.
filter.test 482
FilterFormatAccessTestCase->testFormatPermissions()

This obviously has nothing to do with this patch. Just recording here for posterity...

#19: 718540-19.patch queued for re-testing.

Good nitpicks, thanks for the review! :)

Here's a slightly altered patch.