The exception being thrown is:

PDOException: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'WHERE (module = 'file') AND (fid = '71') AND (type = 'node') AND (id = '81')' at line 1: UPDATE {file_usage} SET WHERE (module = :db_condition_placeholder_0) AND (fid = :db_condition_placeholder_1) AND (type = :db_condition_placeholder_2) AND (id = :db_condition_placeholder_3) ; Array ( [:db_condition_placeholder_0] => file [:db_condition_placeholder_1] => 71 [:db_condition_placeholder_2] => node [:db_condition_placeholder_3] => 81 ) in file_usage_delete() (line 677 of /mnt/www/html/tangle002/docroot/includes/file.inc).

Because this cleanup is being done during cron, this will occur on every page request, causing an error on each and every page because the cron invocation fails. The error is visible to the user, indicating "A server error occurred" or something quite close to that.

Comments

paul.lovvik’s picture

Status: Active » Needs review
StatusFileSize
new1.05 KB

The issue is a logic error in the file_usage_delete function (file.inc) in which a bad update query can be formed if the count is 0. In the current logic the expression is only included in the db_update if the count is > 0, and otherwise it results in a query that looks like:

UPDATE {file_usage} SET WHERE (module = :db_condition_placeholder_0) AND (fid = :db_condition_placeholder_1) AND (type = :db_condition_placeholder_2) AND (id = :db_condition_placeholder_3)

This patch fixes the logic error.

David_Rothstein’s picture

I think this makes sense. If count is 0, there can't be any purpose to that second query anyway.

effulgentsia’s picture

It's not clear from the docs of file_usage_delete() whether it is meant to support being called when there is no usage record, or whether it is meant to be the caller's responsibility to always know that there's a usage record before calling file_usage_delete(). I prefer this patch, because it makes file_usage_delete() more robust. If we go with it, we should have a test for it. If we don't go with it, we should add docs to file_usage_delete() to clarify that it must not be called unless there's a usage record to delete or decrement.

quicksketch’s picture

Issue tags: -Needs tests, -Needs documentation
StatusFileSize
new3.06 KB

Although I wouldn't really recommend calling file_usage_delete() if there aren't any records for that file (since you should always know if there are records left before calling it anyway, otherwise you're doing needless queries), I don't think that the function should be executing invalid SQL queries. paul.lovvik is correct in that there's a logical error in the existing code, rearranging it as he as done corrects the problem.

Here's the same patch with tests added/fixed for file_usage_delete(). I updated the existing assertion to check that the file_usage entry is actually entirely removed, rather than equal to 0. Then added another file_usage_delete() to make sure an error isn't thrown when no entry exists.

quicksketch’s picture

StatusFileSize
new2.57 KB

Sorry that patch included my hacks since I *still* can't run more than one simple test at a time locally. :P

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community

Awesome!

dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

extensive’s picture

Disable ckeditor module and install WYSING module and ckeditor libraries
after that when you configure profile got error
HERE IS THE SOLUTION
http://drupal.org/node/1161738