Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The file module uses bit-wise operators to query the {file_managed}.status. This is not, and will not be, supported by Drizzle.
Moreover, it doesn't even seem necessary, given that the only documented usage for this column is FILE_STATUS_PERMANENT vs. 0, which means temporary.
Could we simply act that this column only purpose is to store the permanent/temporary flag, and get rid of the bit-wise operators (like the one below)?
$result = db_query('SELECT fid FROM {file_managed} WHERE status & :permanent1 <> :permanent2 AND timestamp < :timestamp', array(
':permanent1' => FILE_STATUS_PERMANENT,
':permanent2' => FILE_STATUS_PERMANENT,
':timestamp' => REQUEST_TIME - DRUPAL_MAXIMUM_TEMP_FILE_AGE
));
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 809600-file-status-permanent.patch | 9.84 KB | Damien Tournoud |
| #9 | 809600-file-status-permanent.patch | 9.83 KB | Damien Tournoud |
| #8 | 809600-file-status-permanent.patch | 9.08 KB | Damien Tournoud |
| #7 | 809600-file-status-permanent.patch | 9.08 KB | Damien Tournoud |
| #6 | 809600-file-status-permanent.patch | 9.04 KB | Damien Tournoud |
Comments
Comment #1
chx CreditAttribution: chx commentedYes. And it's definitely not ANSI. Note that this patch does not change the stored values in the database or anything. It just gets rid of needless bitwise ops.
Comment #2
moshe weitzman CreditAttribution: moshe weitzman commentedchange description for this column in system.install. Might be more docs that need changing as well.
Comment #3
quicksketchThis sounds like a great idea to me. Most people (self included) thought it was always just a binary 1 or 0 already.
Comment #4
Damien Tournoud CreditAttribution: Damien Tournoud commentedHere. Changed the description of the table, changed file_space_used(), refactor the test (it was using three different test methods - ie. three different setUp() / tearDown()) for something that is nothing more then a tiny unit test).
Save the polar bears :)
Comment #5
andypostTested, this really fixes issue.
As I understand $status should be a kind of filter so maybe better use array and SQL IN()
The possible case - count a space used by remote files. OTOH the only FILE_STATUS_PERMANENT defined in core.
Current patch kills ability to use contrib-defined statuses so this leads to API change fo function file_space_used()
EDIT: As pointed in #3 schema description should be changed too. And documented.
Comment #6
Damien Tournoud CreditAttribution: Damien Tournoud commentedChanges $status of file_space_used() to be an array, per andypost. There is a smallish API change here anyway, so we better make it clean.
Comment #7
Damien Tournoud CreditAttribution: Damien Tournoud commentedForgot to fix the docblock (per andypost, again, thanks!).
Comment #8
Damien Tournoud CreditAttribution: Damien Tournoud commentedAnd fix grammar :)
Comment #9
Damien Tournoud CreditAttribution: Damien Tournoud commentedOk, actually, let's simplify things here, and admit that there are only two statuses, 0 and FILE_STATUS_PERMANENT.
This new patch:
- simplifies the test
- simplifies file_space_used() signature
- fixes the schema to use a tinyint, as we do for all other status columns
Comment #10
andypost+1 RTBC, good optimization and simplified code.
Comment #11
andypostI mean RTBC...
Comment #12
chx CreditAttribution: chx commentedYay. I wonder why this is normal... but I let it be normal for now.
Comment #13
Damien Tournoud CreditAttribution: Damien Tournoud commentedRerolled.
Comment #14
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD in favor of having Drizzle support the next x years.
Moving this to HEAD to HEAD project.
Comment #15
chx CreditAttribution: chx commentedNo, core has a history and core work is done. HEAD2HEAD can file a separate issue.