file_field_presave assumes that a file object has been loaded

We also should be checking if !empty($item['fid']) because file_field_load() can in fact set $item = NULL in the case that a file failed to load. This error would be exposed if you're running a $node = node_load(1); node_save($node); that has a file or image field that references a deleted file.

We also should expose a test case for this programmatic saving of an entity with a deleted file/image field.

Yes it does! I would actually attack this differently, though:

function file_field_presave($entity_type, $entity, $field, $instance, $langcode, &$items) {
  // Make sure that each file which will be saved with this object has a
  // permanent status, so that it will not be removed when temporary files are
  // cleaned up.
  foreach ($items as $item) {
   if (!empty($item['fid'])) {
      if (($file = file_load($item['fid'])) && !$file->status) {
        $file->status = FILE_STATUS_PERMANENT;
        file_save($file);
      }
    }
  }
}

Not checking the $item can cause a mismatch with file_field_load, which can set NULL, which breaks entity_load.

Marked #1510920: image_field_presave throws exceptions when image field instance is configured as not required and #1508914: File deletion can lead to inconsistent state of file fields, unique key error as duplicates of this issue.

I think we'll also need to investigate image_field_presave() as well.

It still makes the assumption that file_load() will succeed and $item['fid'] exists, which would need to be fixed.

Fixing tags per http://drupal.org/node/1517250

We actually need to unset $items[$delta] if the file is invalid otherwise we'll get fatal errors: "PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'fid' cannot be null"

Attached is the D7 patch.

this patch in #9 works for D7

Ran into the same problem, patch #9 works for me on D7.14

confirming that #9 works fo D7.

Here's another issue queue which relates to this issue and has another fix / patch, which also resolves a problem with file_field_insert as well as pre-save.

http://drupal.org/node/1163740#comment-6944850

#1163740: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '' for key 2: INSERT INTO {file_managed}

Marked #1627860: Unable to add new content with empty upgraded image/file field as duplicated of this.

When is patch #9 getting included in a Drupal update? This solves a bug several people seem to be having that is vaguely documented in at least four or five threads I found on Google.

(FYI using Drupal 7.14 and patch worked for me as well)

It is not clear to me that this is needed for D8, since the entity system is radically different.

Let's retest the patch for D7.

Thanks so much. This worked very well for me (Drupal v 7.19). I do hope it gets into an update soon. I was getting the error after using the Node Export/Import module, importing multiple nodes that included an image field which was not always populated. Now I just have to hunt down the solution to the remaining problem of actually getting the images across (Unknown error occurred attempting to import file: public://images/image_name.jpg).

patch from #17 is not resolving the PDOException Issue here. Still getting:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'fid' cannot be null: INSERT INTO {file_usage} (fid, module, type, id, count) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4); Array ( [:db_insert_placeholder_0] => [:db_insert_placeholder_1] => file [:db_insert_placeholder_2] => node [:db_insert_placeholder_3] => 16918 [:db_insert_placeholder_4] => 1 ) in file_usage_add() (line 661 of xxx/htdocs/includes/file.inc).

i'm on drupal 7.19 (upgraded from d6)

Sorry, my Error was triggered from my own code and has apparently nothing to do with this issue.

I can't figure out if a similar issue exists in Drupal 8 or not. (The specific code isn't there anymore, but it still could be a problem wherever the equivalent functionality lives.)

Shouldn't this patch come with a test? Then we can be sure that it's fixed (in both Drupal 7 and Drupal 8) and won't ever occur again.

This just bit me too. It was rare, but my problem seemed to be having a directory specified for the file upload field to where the files should go, and having a default image assigned.

Every now and then, I would get this error while drush scripting updates:

Error processing entity. SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '' for key 'uri' [2.29 sec, 92.52 MB]  

Applying this patch made the issue vanish and my photos are fine.

Oddly enough, this only happend on the production server- I could not replicate this error before the patch was applied (using Acquia DAMP stack 7.19, PHP 5.3.x).

edit: the discrepancy is I am not running memcache locally.

Another thumbs up for the patch in #9 (on drupal 7)

EDIT: I guess #17 is the better one.

Patch #17 works - just fixed two Drupal 7.x sites encountering the same error.

Patch #17 works just fine for me with Drupal Commerce Batch Product Deletion. Thanks!

YES!! #17 works perfect

Hi,
Setting this back to 7.dev so it can be committed.
The patch in https://drupal.org/node/1443158#comment-6955596 worked perfectly!
Greetings, Martijn

The situation for 8.x is different, there we have a typed class with defined property that are lazyloaded. It doesn't have to assume, it will be loaded when accessed.

Setting back to RTBC for 7.x.

Committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/b5f311a

Somewhat against my better judgement (and apparently that of many other people who commented here) to commit this without tests, but it's been here a long time and is basically just making the code more consistent and robust in the case of a missing file, so I let it slide...

Tests would still be a good idea though.

This can't happen anymore in 8.x, so nothing that could be tested, back to 7.x

Something like the test Dave Reid suggested in #2 certainly looks like missing test coverage that could be applied to Drupal 8 also, no?

This is still occurring in D7, because the patch committed in #32 only applied to file_field_presave(), and not to file_field_insert().
The relevant issue discussing this is closed (marked duplicate of this issue) https://www.drupal.org/node/1627860#comment-8703219
Also comment #14 in this issue remarks the fact that the commit is incomplete without also patching file_field_insert().

I'm still suffering from the bug over in #1627860: Unable to add new content with empty upgraded image/file field. That issue has been marked a duplicate of this , twice, in spite of the fact that working going on over here doesn't seem to solve that problem. I'm re-titling this issue to include that problem, so hopefully future work will - and will upload a working patch from that queue here as well.

Here is the working patch from the other issue.

This patch really doesn't look right. It's possible that this is no longer a problem now that it's all been brought over to FileFieldItemList.php

Well, not fixed yet in D7 (changing version).

I still get a nasty PDO error on Drupal 7.34:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'fid' cannot be null: INSERT INTO {file_usage} (fid, module, type, id, count) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4); Array ( [:db_insert_placeholder_0] => [:db_insert_placeholder_1] => file [:db_insert_placeholder_2] => node [:db_insert_placeholder_3] => 476 [:db_insert_placeholder_4] => 1 ) in file_usage_add() (line 696 of drupal-7/includes/file.inc).

Patch in #37 still applies cleanly.

Patch in #37 still applies to 7.41.

I reapplied this patch with the last core update... It is still relevant and necessary.

I read through #1627860: Unable to add new content with empty upgraded image/file field and agree with the questions there by Dave Reid and others - why would we need to do this in file_field_insert() if we're already doing it in file_field_presave()? It doesn't seem like that's been answered... if some other module is inserting bad data into the entity after presave has already run, isn't that a bug in that module instead?

I'm actually just going to close this issue and reopen #1627860: Unable to add new content with empty upgraded image/file field instead. The bug here was fixed long ago.

The followup for tests didn't exactly happen quickly either... it's been over two years :) So I'll create a separate issue for that instead.

I have faced the below error
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'public://aps/27521.xml' for key 'uri': INSERT INTO {file_managed} (uid, filename, uri, filemime, filesize, status, timestamp, type, uuid) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8); Array ( [:db_insert_placeholder_0] => 2045 [:db_insert_placeholder_1] => 27521.xml [:db_insert_placeholder_2] => public://aps/27521.xml [:db_insert_placeholder_3] => application/xml [:db_insert_placeholder_4] => 2351 [:db_insert_placeholder_5] => 1 [:db_insert_placeholder_6] => 1494852066 [:db_insert_placeholder_7] => undefined [:db_insert_placeholder_8] => 9c3277fe-8ed2-4721-9817-9ba6059fffbc ) in drupal_write_record() (line 7376 of /www/nba-drupal/core/nets/site-1770/includes/common.inc).

to fix this issue i have added FILE_EXISTS_REPLACE as third parameter as shown in below, for me the issue is fixed.
file_save_data($xml,$xml_path.'/'.$object->nid.'.xml', FILE_EXISTS_REPLACE);