Compatibility layer for PHP installations without PDO support

[adamziel]

Even though DBAL currently offers non-PDO drivers, it depends on a number of PDO constants which renders it unusable if PHP was explicitly compiled without PDO. This PR is an attempt to shim PDO constants as stated in ticket DBAL-1156 (http://www.doctrine-project.org/jira/browse/DBAL-1156)

[doctrinebot]

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DBAL-1157

We use Jira to track the state of pull requests and the versions they got
included in.

[Ocramius]

Why not simply providing a PDO class via something like following? Much simpler, much better compatibility:

{
    "autoload": {
        "classmap": [
            "path/to/PDO.php"
        ]
    }
}

[adamziel]

I updated this pull request to match your suggestion

[stof]

defining this class with only the constants is a very bad idea IMO. It means that any project detecting the availability of PDO through a class_exists check will now think that PDO is available while it is not.

[Ocramius]

@stof I think that's not a big problem for us.
While I agree with you on the risks, it is still for the little percentage of projects that want to use Doctrine DBAL without PDO, so the crowd being affected is actually pretty small.

[stof]

Well, it means that DBAL is now saying F*** you to any library trying to detect whether PDO is available to choose their implementation, thus forbidding users to use such library and DBAL together in a project. This seems a bad idea IMO, because the responsible for this incompatibility will be us

[adamziel]

@stof actually I modified it to this form after @Ocramius specifically asked. Initially this PR was an additional class Doctrine\DBAL\PDOCompat and rest of the non-PDO-specific DBAL code refactored to use it instead of \PDO when referencing constants - what do you think about this approach?

[Ocramius]

@stof the risk seems to be acceptable, tbh

[deeky666]

@stof @adamziel @Ocramius can't we somehow make the registration of the shim class conditional on whether PDO is installed or not? Maybe by bootstrapping composer autoloader somehow (don't have enough knowledge about this).

[Ocramius]

@deeky666 looking again at this, I think it isn't worth supporting this approach anymore: we should just make ext-pdo a hard dependency and call it "done".

While removing the dependency is a nice-to-have, PDO is probably the ugliest API we can polyfill, riddled with poor design decisions, strange method signatures and behaviors.

[lcobucci]

Creating this thing to only throw an exception is pretty much the same as the hard dependency.

[stof]

Creating such class could break any other library checking the existing of the PDO class and then trying to use something else than a constant from it when it exists. So I'm -1 on adding it.

[morozov]

I think it isn't worth supporting this approach anymore: we should just make ext-pdo a hard dependency and call it "done".

Why? Doctrine doesn't use anything from PDO besides constants and even this dependency can be eliminated by getting rid of usage of PDO:: constants in favor of Doctrine's own. If an app uses DBAL and doesn't support PDO it will have to require it even if it doesn't support it.

While removing the dependency is a nice-to-have, PDO is probably the ugliest API we can polyfill, riddled with poor design decisions, strange method signatures and behaviors.

Why is it needed to polyfill anything else then constants from PDO? This shim works perfectly, but just getting rid of their usage will be the right fix IMO.

[Ocramius]

Given the discussion above, can we add ext-pdo to composer.json in this patch instead?

…

On 2 May 2017 17:12, "Christophe Coevoet" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In lib/Doctrine/DBAL/Compat/PDO.php <#808 (comment)>: > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * This software consists of voluntary contributions made by many individuals + * and is licensed under the MIT license. For more information, see + * <http://www.doctrine-project.org>. + */ + +/** + * PDO shim to keep DBAL compatible with non-PDO PHP installations. + * + * @link http://php.net/manual/en/pdo.constants.php + * @Package Doctrine\DBAL + */ +class PDO Creating such class could break any other library checking the existing of the PDO class and then trying to use something else than a constant from it when it exists. So I'm -1 on adding it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#808 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJakJxxUgoz3RLEosq_wvdgiNZR430jks5r10fegaJpZM4DnfUd> .

[Ocramius]

Closing: moving to #2709