Problem

You cannot install Drupal 8 on sqlite and enable locale module. It whitescreens. The bug was introduced with new code in #1785086: Introduce a generic API for interface translation strings almost 2 months ago. Nobody noticed because people don't run tests (or Drupal 8 sites) on sqlite. In #1854752: Re-Add PDO::FETCH_PROPS_LATE to PDO::FETCH_CLASS Damien Tournoud suggests it is a foolish thing to use fetchObject() with a class name. It is not only that the core PHP implementation is broken, the PDO implementation is broken in a different way too.

Suggested solution

Grab the result set as an array and instantiate the object with that. This was suggested over in #1854752: Re-Add PDO::FETCH_PROPS_LATE to PDO::FETCH_CLASS by Damien.

The existing tests should already cover this extensively, but they did not fail before since obviously they run on MySQL and the PDO implementation did not have the same bugs as our PHP implementation has.

Comments

gábor hojtsy’s picture

Issue tags: +sprint
damien tournoud’s picture

Consider that RTBCed from here if it returns green.

jose reyero’s picture

The patch looks good to me so I'd RTBC as soon as the test is green.

jose reyero’s picture

Um, after reading the related issue I think we should replace occurrences of setFetchMode(PDO::FETCH_CLASS, ..) too, that's in dbStringLoad()

gábor hojtsy’s picture

BTW I also checked the interface to see if it is properly implemented this way, and indeed, if there was no string, NULL should be returned, which is what the code does with the patch.

jose reyero’s picture

StatusFileSize
new2.32 KB

This one removes setFetchMode(PDO::FETCH_CLASS) too.

gábor hojtsy’s picture

Status: Needs review » Needs work

Seems to me like we should fetch/instantiate one by one instead of fetching a big array and then processing that to avoid using up twice the memory footprint? Should be a minor change.

damien tournoud’s picture

You can and should use foreach($result) directly.

jose reyero’s picture

Status: Needs work » Needs review
StatusFileSize
new2.3 KB

Yes Sir :-)

gábor hojtsy’s picture

Looks good to me, RTBC when green :)

gábor hojtsy’s picture

Status: Needs review » Reviewed & tested by the community
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks so much for all of the investigation here, folks. Sorry it caused such a frackas. :(

gábor hojtsy’s picture

Issue tags: -sprint

Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.