SQLite follow up to #1953800 'Make the database connection serializable' [#2010322]

Problem/Motivation

After #1953800: Make the database connection serializable Drupal 8-x.dev fails to install if SQLite chosen as database system. The install fails with "The server reports the following message: The service definition "request" does not exist" after SQLite chosen as database for install in database selection form. Fixing 2 code errors in core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php allows the install to progress further. Cannot move forward on #1998366: [meta] SQLite is broken until this is in.

#1953800: Make the database connection serializable
#1998366: [meta] SQLite is broken

Comment	File	Size	Author
#6	2010322.patch	5.6 KB	Damien Tournoud
#6
#6	2010322-interdiff.txt	856 bytes	Damien Tournoud
#5	2010322.patch	5.02 KB	Damien Tournoud
#5
#3	2010322-3.patch	1.07 KB	amateescu
#3
#3	interdiff.txt	755 bytes	amateescu
#1	2010322_1_SQLite_coding_errors.patch	1011 bytes	dcrocks
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

dcrocks CreditAttribution: dcrocks commented 2 June 2013 at 20:04

File	Size
2010322_1_SQLite_coding_errors.patch	1011 bytes

There were 2 errors. Function sqlFunctionRand needed the 'static' keyword. The 'compatibility' error just needed the 'array' type hint added to Function prepare.

Comment #2

dcrocks CreditAttribution: dcrocks commented 2 June 2013 at 20:05

Status:

Active

» Needs review

needs review.

Comment #3

amateescu CreditAttribution: amateescu commented 2 June 2013 at 20:17

Status:

Needs review

» Reviewed & tested by the community

File	Size
interdiff.txt	755 bytes
2010322-3.patch	1.07 KB

Sorry for breaking this in the first place :/ Let's also fix the arguments for prepare() to match the parent method.

Comment #4

dcrocks CreditAttribution: dcrocks commented 2 June 2013 at 20:28

It shouldn't be necessary but I'm OK with it. #1 matches the call in Function PDOprepare, which seemed more appropriate to me.

Comment #5

Damien Tournoud CreditAttribution: Damien Tournoud commented 2 June 2013 at 21:01

Status:

Reviewed & tested by the community

» Needs review

File	Size
2010322.patch	5.02 KB

Actually, there is a bit more to it. We need the original PDO connection object in StatementPrefetch (because we call PDO-specific methods on it, like PDO::errorInfo()), but we also need the Drupal Connection object there (because we call Connection::getLogger()).

So the easier is just to refactor this and inject the PDO connection directly inside StatementPrefetch... and as a consequence clean-up a few now unnecessary methods.

Untested patch enclosed.

Comment #6

Damien Tournoud CreditAttribution: Damien Tournoud commented 2 June 2013 at 21:06

File	Size
2010322-interdiff.txt	856 bytes
2010322.patch	5.6 KB

And update the property declarations.

Comment #7

dcrocks CreditAttribution: dcrocks commented 3 June 2013 at 00:41

Title:

SQLite code errors introduced by 'Make the database connection serializable'

» SQLite follow up to #1953800 'Make the database connection serializable'

Since this has expanded a better title would be appropriate.

Comment #8

amateescu CreditAttribution: amateescu commented 4 June 2013 at 09:55

Status:

Needs review

» Reviewed & tested by the community

That refactoring looks good to me, thanks Damien :)

Comment #9

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott commented 5 June 2013 at 08:21

Status:

Reviewed & tested by the community

» Needs review

Hmmm... this fixes the issue with serialisation. But I actually can not install Drupal 8 using sqlite... it just pauses during the installer and tables for modules that have been enabled do not appear to have been created.