RFC: Deprecate/drop PDO

[sminnee]

This is essentially the opposite of #6502.

PDO doesn't provide any benefit to us. It's a leaky, lowest-common-denominator abstraction that introduces a lot of issues that are very difficult to debug, and in return saves us from having to mainly 3 pieces of code that are small and pretty easy to maintain (SQLite, MySQL, and PostgreSQL native connectors).

Here area few of the places where PDO has failed us:

• Can't seek on query results
• Doesn't support nested transactions, when doing transactions "properly". Not doing transactions properly causes other undocumented bugs.
• Segfaulting in SQLite when using runInSeparateProcess on a test
• Not retaining native_type metadata on PostgreSQL when using a cached prepared statement second time
• It has no notion of float/decimal values. It's type system just works with ints and strings.

In terms of its impact on bugfixing-effort, PDO is the IE6 of database connectors.

There's a possibility that we're just "using PDO wrong". There have certainly been some bugs introduced by our quirky implementation. But that's not really the point. It's full of undocumented, database-specific limitations, and improving the implementation just seems to introduce other bugs or limitations. Our users gain almost nothing from our PDO support - native drivers are at least as likely to be available as PDO. And our dev team doesn't either.

I recognise that successfully moving to DBAL would render this unnecessary. However, moving to DBAL seems like a lot of work, and I'm not sure that I'd want to park other improvements because this idea has been proposed.

So my recommendations would be:

• Flag PDO as deprecated in a 4.x minor release
• Remove PDO support in 5.0
• Change the installer to use MySQL native by default
• Allow for some performance or edge-case-feature limitations of PDO use, via judicious skipTest. For example, don't expect real nested transactions / savepoint rollback to work on PDO.

[sminnee]

Pinging @dhensby and @tractorcow about this in particular.

[tractorcow]

How about switching it out to a module instead? I use PDO for a few projects (e.g. where we use MS SQL Server as a secondary data source).

[tractorcow]

To be fair, my main preference for PDO is just not having to maintain a separate connector for each db type, although we end up having to do it anyway for all the other classes.

[sminnee]

To be fair, my main preference for PDO is just not having to maintain a separate connector for each db type, although we end up having to do it anyway for all the other classes.

Yeah and that seems plausible on paper, but my experience is that the maintenance effort of PDO is greater than the maintenance effort of MySQLConnector + PostgreSQLConnector + SQLiteConnector. Hence this RFC

How about switching it out to a module instead? I use PDO for a few projects (e.g. where we use MS SQL Server as a secondary data source).

Yeah, that would make sense, but as part of that I'd probably stop it from the 'supported modules list' at least in SS5.

We could potentially even do that in SS4 and add the module as a requirement of the core-recipe and/or framework, to avoid a semver breakage. That would involve having some overlapping namespaces in 2 packages, but that seems okay.

[tractorcow]

@sminnee I think you've made the right choice here. PDO does consolidate a lot of the code, but if we have to sacrifice correctness of function we are better off deprecating it.

I think where I got stuck in fixing native types was assuming we would have to fix PDO to get it done. Dropping it is the out of the box solution I didn't consider. :)

[sminnee]

Yeah I can still get that stuff working in PDO, it will just be a bit uglier. "Deprecated connectors are a bit ugly" feels like a better place to leave things.

The main thing that I sacrificed in PDO was true nested transactions, and savepoints (which amount to the same thing). In PDO, the rollback of a nested transaction forces the rollback of the parent too. This behaviour mirrors DBAL.

FYI: This change is in the 4 branch due to be released in 4.4.

[sminnee]

@silverstripe/core-team any objections about deprecating PDO from 4.5?

[chillu]

For the record, there's 35 core issues mentioning PDO.

I think nested transactions are increasingly important as both our decoupling of core modules and domain model complexity increases (e.g. through nested versioning stuff).

Assuming that we already support the other non-PDO drivers (and run tests on them), this seems fairly small amount of effort? In this case I'm a 👍

[Firesphere]

In addition, PDO does not support the MySQL implementation of indexed enums.

[sminnee]

OK I've posted a couple of PRs that formalise this where I think it makes sense:

• NEW: Deprecated PDO in favour of native drivers #9052
• NEW: Change .env.example to default to MySQL native driver silverstripe-installer#256

@ScopeyNZ you may want to look at this since you most recently queried PDO behaviour.

[ScopeyNZ]

Looked & approved.

The optimist in me is a little sad to see "drop PDO support". It would be nice to have support for any PDO supported DB out of the box - but then our adapters don't really work like that as they're specific to a database. I'm 👍 for this RFC. A future adapter could add back PDO (probably as a separate module) but have it be database agnostic.

[sminnee]

Yeah. This idea that PDO makes it easier to support a broad range of database backends is a lie. Our ORM has support for pluggable backends; the boilerplate needed to cover the domain that PDO covers is pretty small. PDO doesn't help us much, and it does cause significant pain.

[sminnee]

OK, once #9052, #9102 and silverstripe/silverstripe-installer#256 are merged we can close this, I think.

[ScopeyNZ]

We've had agreement from a few core contributors and the rfc/accepted tag is on this issue already. I'm just going to merge the outstanding PR :)