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/Motivation
This issue is part of #2454513: [meta] Make Drupal 8 work with SQLite
Drupal\system\Tests\Database\LoggingTest is currently failing on SQLite because Drupal\Core\Database\StatementPrefetch uses $dbh to refer to the PDO Connection object, while Drupal\Core\Database\Statement uses it to refer to the Drupal connection object.
Proposed resolution
Change Drupal\Core\Database\StatementPrefetch::$dbh to reference a Drupal connection object, just like Drupal\Core\Database\Statement::$dbh does.
Remaining tasks
Review.
User interface changes
Nope.
API changes
Drupal\Core\Database\StatementPrefetch::$dbh now references the Drupal connection object and Drupal\Core\Database\StatementPrefetch::$connection is renamed to $pdoConnection and it references the PDO connection object.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | interdiff.txt | 1.44 KB | amateescu |
| #3 | 2486831-3.patch | 2.51 KB | amateescu |
| #1 | 2486831.patch | 2.51 KB | amateescu |
Comments
Comment #1
amateescu CreditAttribution: amateescu for Drupal Association commentedAnd a patch.
Here's the output of a test run for the
Databasegroup.Drupal\system\Tests\Database\LoggingTestis included in there and it passes now.Comment #2
dawehnerShort variables names are always a good idea ... do you mind open a novice follow to rename this to
drupalDatabaseConnection?usually we don't start with uppercase ... what about using pdoConnection? This looks a little bit nicer.
Comment #3
amateescu CreditAttribution: amateescu for Drupal Association commentedThanks for taking a look at the patch.
1. We cannot change $dbh because
Drupal\Core\Database\Statementextends PDOStatement which is the one that provides the $dbh variable, and the entire point of this issue is to bringDrupal\Core\Database\StatementPrefetchinline withDrupal\Core\Database\Statement:)2. That's a good point, renamed according to your suggestion.
Comment #4
BerdirMakes sense, patch looks good.
Comment #5
catchCommitted/pushed to 8.0.x, thanks!