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

CommentFileSizeAuthor
#6 2010322.patch5.6 KBdamien tournoud
#6 2010322-interdiff.txt856 bytesdamien tournoud
#5 2010322.patch5.02 KBdamien tournoud
#3 2010322-3.patch1.07 KBamateescu
#3 interdiff.txt755 bytesamateescu
#1 2010322_1_SQLite_coding_errors.patch1011 bytesdcrocks

Comments

dcrocks’s picture

dcrocks commented
StatusFileSize
new 2010322_1_SQLite_coding_errors.patch1011 bytes

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

dcrocks’s picture

dcrocks commented
Status: Active » Needs review

needs review.

amateescu’s picture

amateescu commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new interdiff.txt755 bytes
new 2010322-3.patch1.07 KB

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

dcrocks’s picture

dcrocks commented

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

damien tournoud’s picture

damien tournoud commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 2010322.patch5.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.

damien tournoud’s picture

damien tournoud commented
StatusFileSize
new 2010322-interdiff.txt856 bytes
new 2010322.patch5.6 KB

And update the property declarations.

dcrocks’s picture

dcrocks commented
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.

amateescu’s picture

amateescu commented
Status: Needs review » Reviewed & tested by the community

That refactoring looks good to me, thanks Damien :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
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.

amateescu’s picture

amateescu commented
Status: Needs review » Reviewed & tested by the community

We have a different issue for that #1998366: [meta] SQLite is broken. This is just a small patch that fixes what we broke in #1953800: Make the database connection serializable :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Thanks amateescu! So this does what it says on the tin.

Committed 1769841 and pushed to 8.x. Thanks!

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented

Awesome! This is getting us closer.

Status: Fixed » Closed (fixed)

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