polyfill icon indicating copy to clipboard operation
polyfill copied to clipboard

Add polyfill for PDO driver specific subclasses

Open jnoordsij opened this issue 3 months ago • 8 comments

As a follow-up to #547, this PR aims to implement a full-fledged set of polyfills for the driver specific PDO subclasses introduced in PHP 8.4; see also https://wiki.php.net/rfc/pdo_driver_specific_subclasses.

As of PHP 8.5, the driver specific constants and methods available on the base PDO class will be deprecated, to be removed in PHP 9.0. See also https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_driver_specific_pdo_constants_and_methods.

This PR adds a polyfill to use the driver-specific child classes for PHP versions prior to PHP 8.4. This in turn allows one to already refer to the 'new' constant, to prevent deprecation errors on PHP 8.5, while retaining backwards compatibility. This can be particularly useful for code requiring support for multiple versions; see e.g. https://github.com/laravel/framework/issues/57141.

The classes should be suitable for constructing on older PHP versions.

Note the PDO::connect static method that has been introduced simultaneously has not been polyfilled, as I can't think of a proper way to do such a thing currently.

jnoordsij avatar Oct 16 '25 12:10 jnoordsij

Should we add the static connect method to every child class, since we can polyfill it there? How does this method behave when given a DSN for another driver and when it's called on a child class?

We could, but that feels a bit off to me. The whole idea of the PDO::connect static method is that it is able to detect the type of driver and thus subclass to use from the provided dsn, making it generic. Calling e.g. \Pdo\Mysqlite::connect(...) instead of just new \Pdo\Mysqlite(...) makes little sense to me, and probably should not be something to actively encourage in any userland code?

jnoordsij avatar Oct 16 '25 13:10 jnoordsij

About the connect method on child classes, I'd still add it, for completness of the polyfill

nicolas-grekas avatar Oct 16 '25 13:10 nicolas-grekas

I probably will look into the last bits next week; if anyone is eager to finalize on the connect method or think of additional tests to throw in here, feel free to add them.

jnoordsij avatar Oct 16 '25 13:10 jnoordsij

Calling e.g. \Pdo\Mysqlite::connect(...) instead of just new \Pdo\Mysqlite(...) makes little sense to me, and probably should not be something to actively encourage in any userland code?

But if you are on PHP 8.4, would you really call the constructor of those subclasses instead of PDO::connect()? So either way, we're encouraging userland code that is different with the polyfill than with the real thing, mainly because we cannot polyfill PDO::connect(). 😕

derrabus avatar Oct 16 '25 14:10 derrabus

But if you are on PHP 8.4, would you really call the constructor of those subclasses instead of PDO::connect()? So either way, we're encouraging userland code that is different with the polyfill than with the real thing, mainly because we cannot polyfill PDO::connect(). 😕

To be fair, if one is currently only using something like new PDO("sqlite:foo");, then migrating towards new Pdo\Sqlite("sqlite:foo"); does not seem like a weird strategy to me. In particular for projects were the driver class is already predetermined and you want to have the added benefit of stricter types for static analysis, one could prefer this over PDO::connect which will from a static point of view only ever return PDO. However I do agree that the main usecase would still be to used PDO::connect, which unfortunately is not something we can provide.

jnoordsij avatar Oct 20 '25 09:10 jnoordsij

I think this should be complete now as is. Additional feature-like tests could be nice, but on the other hand are probably very hard to implement in practice without any real database and I think the current set should suffice to prove this should work in practice. In particular the migration towards 'polyfilled' constants should be very safe to do with this.

jnoordsij avatar Oct 20 '25 09:10 jnoordsij

@nicolas-grekas with PHP 8.5 stable coming up next week, is there any chance of aiming to review and release this somewhere within that timeframe?

jnoordsij avatar Nov 12 '25 08:11 jnoordsij

Can you please proceed with these CS changes on all classes?

Done!

jnoordsij avatar Nov 12 '25 10:11 jnoordsij