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

Files: 
CommentFileSizeAuthor
#6 2010322.patch5.6 KBDamien Tournoud
PASSED: [[SimpleTest]]: [MySQL] 55,880 pass(es).
[ View ]
#6 2010322-interdiff.txt856 bytesDamien Tournoud
#5 2010322.patch5.02 KBDamien Tournoud
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#3 2010322-3.patch1.07 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#3 interdiff.txt755 bytesamateescu
#1 2010322_1_SQLite_coding_errors.patch1011 bytesdcrocks
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Comments

dcrocks’s picture

StatusFileSize
new1011 bytes
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

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

Status:Active» Needs review

needs review.

amateescu’s picture

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new755 bytes
new1.07 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

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

dcrocks’s picture

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

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new5.02 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

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

StatusFileSize
new856 bytes
new5.6 KB
PASSED: [[SimpleTest]]: [MySQL] 55,880 pass(es).
[ View ]

And update the property declarations.

dcrocks’s picture

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

Status:Needs review» Reviewed & tested by the community

That refactoring looks good to me, thanks Damien :)

alexpott’s picture

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

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

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

Awesome! This is getting us closer.

Status:Fixed» Closed (fixed)

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