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 includes/update.inc makes a guess on the fact that the drupal driver and the PDO driver has the same name for the same database. This is not true, for example for oracle... The call their drivers "oci" not "oracle".
I think this check should be removed:
elseif (isset($databases['default']['default']['driver']) && !in_array($databases['default']['default']['driver'], PDO::getAvailableDrivers())) {
$message = '<h2>A PDO database driver is required!</h2><p>You need to enable the PDO_' . strtoupper($databases['default']['default']['driver']) . ' database driver for PHP ' . DRUPAL_MINIMUM_PHP . ' or higher so that Drupal 7 can access the database.</p>';
}
Comment | File | Size | Author |
---|---|---|---|
#9 | 1029080-update-database-pdo-rev3.patch | 2.16 KB | radalin |
#7 | 1029080-update-database-pdo-rev2.patch | 2.11 KB | brianV |
#2 | 1029080-update-database-pdo.patch | 1.82 KB | Damien Tournoud |
Comments
Comment #1
sunComment #2
Damien Tournoud CreditAttribution: Damien Tournoud commentedComment #4
xjmupdate_prepare_d8_bootstrap()
does not include this check, so this is a D7 issue.Also, since no one has reported issues because of this since D7 release, I don't think this is major.
Comment #5
brianV CreditAttribution: brianV commented#2: 1029080-update-database-pdo.patch queued for re-testing.
Patch applied cleanly locally here against latest HEAD, so I'm thinking it got caught up in the testbot being funky at some point.
Comment #7
brianV CreditAttribution: brianV commentedpatch updated.
Two things needed to be addressed:
1. database.inc needed to be included before we call db_installer_object() as it calls a static function in the Database class.
2. DatabaseTasks::hasPDODriver() class needed to be made public since we are calling it from outside the class.
Comment #8
chx CreditAttribution: chx commentedThis really falls into the try category of the "do it or do not, there is no try". Just
try { connect to the database } catch { run screaming }
.Comment #9
radalin CreditAttribution: radalin commentedThis patch looks promosing, however I did not really like the idea of making a "require" call there. Instead we might move the drupal_bootsrap_database there. This way we make sure that everything related to the database is bootstrapped.
Why hasPdoDriver is protected anyway? I have checked the class and found a "installable" method. It basically calls the hasPdoDriver but it's public and seems better. I have uploaded the patch, what do you think about it?
Comment #10
brianV CreditAttribution: brianV commentedI like radalin's approach in #9 - I never noticed DatabaseTasks::installable() was available, so that saves a lot of work as well.
Also, I tested this patch against an Oracle installation (which fails with core code due to the pdo_oci php driver name vs. 'oracle' drupal name), and this handles it gracefully.
Comment #11
radalin CreditAttribution: radalin commentedI have also checked what would happen if no database conf is defined on the settings file. Update page seems to redirect to the installation page without any errors etc.
Comment #12
PanchoOh this has been reported, see #1504352: Cannot run Drupal update.php with Oracle Driver.:
radalin: "I'm trying to run drupal/update.php with the Oracle driver installed. However there is a problem with the update scripts which shows an error like:"
Renee S: "Confirmed"
brianV: "In practice, we've been commenting out that check prior to running update.php."
It's just that Oracle dbas are experts enough to handle it and the check is easy enough to comment out... but still it's just another hurdle to implement Drupal on Oracle, so we should get rid of it.
Patch in #9 looks good and still cleanly applies.
Some nitpicks:
I'd remove the first comment line. We don't bootstrap the database only to check driver info, and the rest is quite self-explanatory.
Would be nice to get a bit more specific than "the proper PDO driver for foo_db". Therefore we'd need a public getter for $this->pdoDriver.
Also, requiring DRUPAL_MINIMUM_PHP isn't enough. The driver might require an even higher PHP version as in the case of PostgreSQL which requires PHP 5.2.11 because of incompatibilities. We'd therefore need a public getter for a variable $this->minimumPhpVersion or such.
Both additions IMHO are worth it.
And then, is it correct that there is no check at all in update_prepare_d8_bootstrap()? Otherwise we should probably implement a reasonable check in D8 and backport it to D7.
Comment #13
chx CreditAttribution: chx commentedComment #14
katannshaw CreditAttribution: katannshaw commentedThis same error appears for me when running updatedb in Drush. I'm using the SQL Server Driver.
Comment #15
chx CreditAttribution: chx commentedComment #16
dasginganinjaI just ran into this bug. The driver in use is the MySQL native driver (mysqlnd) since I am using it with the APDQC module.
Comment #17
dasginganinjaPer my report in #16 the error turned out to be that PDO-mysql wasn't re-enabled after a switch from php-mysql to php-mysqlnd.