This may be a bug with cck

1) enable devel query logging
2) create a content type with a single value filefield
3) create a node of that type without a file in that fieldfield

Result: seeing query UPDATE file set status = 1 where fid = 0

Files: 
CommentFileSizeAuthor
filefield_field_update_no_file.patch599 byteshefox

Comments

Status:Needs review» Needs work

Thanks. This patch needs some work though because it looks like it would prevent files from being deleted if they were removed from a node.

Files that are being deleted have an FID so would pass that check, to my memory

Files that have been deleted are no longer in the $items list at all. Even in the patch you can see the line that checks for an empty FID:

     // Remove items from the array if they have been deleted.
     if (empty($items[$delta]) || empty($items[$delta]['fid'])) {

So we can't skip things that don't have FID, since that indicates that an item has been deleted.

A file needs to have an FID in order to be deleted, what happens is happening there is file_field_save is given a fully loaded file object with an FID and delete marker, deletes the file, and returns an empty array for filefield_field_update to remove from array. So while there is that line checking for no fid, a file being deleted will have an FID before the call to file_field_save. Files are given fids when they're uploaded, so there's not the situation where deleting a file without an fid.

I recall testing deleting and adding files manually with this patch; don't recall if went live with it but believe it also patched internal QA also.

Hm, okay @hefox. This probably needs some real testing on my part, it just set off my normal review alarms that something might be amiss. If there is always an FID, that code isn't actually executing, which would be strange but not impossible.