Problem/Motivation

The getDriverClass central method is still trying to use called with the following

Proposed resolution

Make the fallback work again. Remove the unnecessary classes.

Remaining tasks

None.

User interface changes

None.

API changes

None. These classes were never meant to be used directly.

CommentFileSizeAuthor
#6 2461239_6.patch7.41 KBchx
#4 2461239_4.patch6.83 KBchx
#2 driver_fallback.patch6.7 KBchx
driver_fallback.patch6.47 KBchx

Comments

Status: Needs review » Needs work

The last submitted patch, driver_fallback.patch, failed testing.

chx’s picture

chx commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new driver_fallback.patch6.7 KB

We pass in Install\Tasks but also Select and Select is in Query by core but not in Query per driver so the fallback logic is not flat.

Status: Needs review » Needs work

The last submitted patch, 2: driver_fallback.patch, failed testing.

chx’s picture

chx commented
Status: Needs work » Needs review
StatusFileSize
new 2461239_4.patch6.83 KB

OK... So ->extend('Drupal\Core\Database\Query\PagerSelectExtender') (and similar) was breaking it. After this patch we can change all callers to use ->extend('PagerSelectExtender') if we so want.

Also, here is a fun one, I never realized the driver can override the extender by creating a class called Drupal\drivernamespace\Drupal\Core\Database\Query\PagerSelectExtender. Probably that wasn't the intention but it is certainly so in HEAD and with this patch too.

Status: Needs review » Needs work

The last submitted patch, 4: 2461239_4.patch, failed testing.

chx’s picture

chx commented
Status: Needs work » Needs review
StatusFileSize
new 2461239_6.patch7.41 KB

That's just a unit test trying to pick up the now nonexistent class. Better luck with pgsql.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented 
+++ b/core/lib/Drupal/Core/Database/Connection.php
@@ -659,15 +659,22 @@ protected function expandArguments(&$query, &$args) {
+      do {
...
+      } while (!class_exists($this->driverClasses[$class]) && $namespaces);

hmmm interesting.

Crell’s picture

Crell commented
Status: Needs review » Needs work

The issue summary is incomplete and breaks off mid-sentence. I think I follow what is being done here from the patch. However, this is a regression. Early in the D8 cycle we switched the child classes to always be required in order to simplify the code. I think I made that change as part of the namespace-ification.

I don't see a reason to go back here. The empty classes hurt no one, and the patch here introduces a while loop into the driver handling that seems completely unnecessary.

I'm inclined to won't-fix.

PS: Yes, driver-specific extender classes are a deliberate design decision. I don't know that they're often used, but I am pretty sure that was deliberate.

chx’s picture

chx commented
Status: Needs work » Closed (works as designed)

Broke out one tiny, tiny bit to #2462653: Connection::getDriverClass doesn't support non-core drivers .