I tried to use drupal_write_record() to write a record to the database.
I spent an hour trying to track down why it isn't working.

Here's why.

While you can catch the PDOExceptions lower down the chain, these errors just return null.
This patch isn't a total solution for this (needs to be ported to other adapters, or the validation should be part of the base class), but it is a start to show the problem. We may also not want to use PDOException, because it isn't really a PDO Exception, but a Drupal API Exception.

CommentFileSizeAuthor
mysql_insert_exception_handling.patch863 bytesJacobSingh

Comments

JacobSingh’s picture

JacobSingh commented

I'm not sure why, but putting this in causes the admin page WSOD for me.

I assume that something is not catching the exception, or rather, catching it and then dying in some unpleasant way.

dries’s picture

dries commented
Issue tags: +Favorite-of-Dries

I agree that silently ignoring what are most likely to be broken queries is not the right approach. We should definitely be throwing exceptions in those cases.

For consistency and predictability of the API, those 3 checks should probably be factored out, made part of the base class and re-used in each of the database backends.

Status: Needs review » Needs work

The last submitted patch failed testing.

Crell’s picture

Crell commented
Status: Needs work » Closed (duplicate)

That code is already being factored to the base class in #481288: Add support for INSERT INTO ... SELECT FROM .... Marking this as a dupe and leaving a note over there to convert them to exceptions while we're at it.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented 
     // Don't execute query without values.
     if (!isset($this->insertValues[0]) && count($this->insertFields) > 0 && empty($this->fromQuery)) {
-      return NULL;
+      throw new PDOException("There are no available fields");
     }

This is not an exception, see #481288-16: Add support for INSERT INTO ... SELECT FROM ... for an explanation.