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.
Problem
- Try to install with either of postgresql, or SQLite.
- Install freezes and ultimatlely fails.
Background
- Filter_plugins are tried to store in cache with 'expires' column set to an Array, which borks.
- FilterPluginManager sets the cache with only three arguments, though CacheDecorator constructor takes 4 arguments. Cache tags array is erroneously being passed into expires column.
- In the contrary, MySQL database driver silently ignores this.
Proposed resolution
- Call CacheDecorator with the correct number of arguments (expiration: CACHE_PERMANENT?), then cache tags.
Remaining tasks
- timplunkett acknowledges bug, but also points out that may be fixed by changes in #1903346: Establish a new DefaultPluginManager to encapsulate best practices. Whichever goes in first..? Although that issue does not address FilterPluginManager yet.
Related Issues
Comment | File | Size | Author |
---|---|---|---|
#2 | filter-2008644-cachedecorator-arguments-2.patch | 1.43 KB | Pancho |
#1 | filter-2008644-cachedecorator-arguments-1.patch | 1.3 KB | mradcliffe |
Comments
Comment #1
mradcliffeBlock module is the only other module to call CacheDecorator with all of the arguments.
Patch for review: should it use CACHE_PERMANENT?
Comment #2
PanchoExpanded the issue summary.
This bug keeps D8 from being installed on postgreSQL and SQLite and therefore is critical.
While there are additional bugs to be fixed (see #2001350: [meta] Drupal cannot be installed on PostgreSQL resp. #1998366: [meta] SQLite is broken), this is a confirmed and obvious bug and the fix allows the installer to get a bit further.
CACHE_PERMANENT is the default value for expiration, so unless otherwise set it should be just fine.
Just putting the USE statements into alphabetic order and breaking out two of the arguments to intermediate variables to avoid the horribly long line.
So please do another quick review, otherwise this patch should be RTBC, IMHO. Would be nice to go forward then.
Comment #3
tim.plunkettWow, nice catch!
I'm not 100% sure we can/should write tests for this.
I'll tentatively set RTBC, but if someone wants tests they should be prepared to suggest how to best do that :)
Comment #3.0
tim.plunkettExpanded and partially reworded
Comment #4
catchI assume if we had postgres test bots they'd have caught this via existing test coverage (or just not installing at all). Can't think of a test otherwise.
Comment #5
Dries CreditAttribution: Dries commentedI agree this isn't easily testable. Committed to 8.x. Thanks.
Comment #6
PanchoThanks for committing, and yes, postgres test bots would have definitely caught this.
Checking the type of parameters on all API functions might help, too, but would add an incredible overhead given that PHP is a weakly typed language. Otherwise we unfortunatly can't completely avoid occasional wrong function calls when code is refactored.
Comment #7.0
(not verified) CreditAttribution: commentedAdd related issue #1903346: Establish a new DefaultPluginManager to encapsulate best practices, and update summary.