Hello!
I’ve been working on request introduced here https://bugs.php.net/bug.php?id=74957 https://bugs.php.net/bug.php?id=74957 related to implementing new PDOInterface and PDOStatementInterfaces.
At this point of time classes PDO and PDOStatement do not implement any interfaces that’s why code that uses PDO and PDOStatement are coupled and referred to concrete implementation , not interface.
It will help to add some custom classes and behavior to these classes and to decouple caller from details of PDO layer implementations.
At this point of time, the only way to add some custom classes and behavior is to approach this is by subclassing PDO and PDOStatement.
You can use the PDO:: ATTR_STATEMENT_CLASS driver option to tell a PDO object which PDOStatement subclass to return from PDO::prepare().
But this is is not very straightforward and elegant (in terms of coding) way to do this.
But if PDO and PDOStatement implemented interfaces, it would be possible for the custom behaviour classes to implement those interfaces as well, making them interchangeable.
No changes needed in callers of PDO/PDOStatement.
Here is my PR introducing this interface
https://github.com/php/php-src/pull/2657 https://github.com/php/php-src/pull/2657
I would like to hear any feedback on it!
Thanks!
Hello!
the only way to add some custom classes and behavior is to approach this is by subclassing PDO and PDOStatement.
Please could you explicitly say when this is a problem, or what using
an interface allows?
I can imagine possible scenarios, but the discussion would be clearer
if you gave concrete examples.
cheers
Dan
Yes, sure.
Good example has been provided in related issue in bug tracker.
Assume we are using persistent PDO and want to handle long running processes and add some logic when executing queries / connections (for instance logging).
It would require your custom classes deriving from PDO and PDOStatement where we add this additional logic.
According to documentation (http://php.net/manual/en/pdo.setattribute.php http://php.net/manual/en/pdo.setattribute.php) when we are using persistent PDO we can’t use PDO::ATTR_STATEMENT_CLASS and return our custom PDOStatement class instance from prepare method.
But just implementing PDOInterface and PDOStatementInterface will allow us to implement this and have proper type hints in userland code.
In addition, using interfaces is better from code architecture perspective (which is just my opinion and a bit arguable of course).
Thanks!
Hello!
the only way to add some custom classes and behavior is to approach this is by subclassing PDO and PDOStatement.
Please could you explicitly say when this is a problem, or what using
an interface allows?I can imagine possible scenarios, but the discussion would be clearer
if you gave concrete examples.cheers
Dan
when we are using persistent PDO we can’t use PDO::ATTR_STATEMENT_CLASS and
return our custom PDOStatement classBut just implementing PDOInterface and PDOStatementInterface will allow us to implement
this and have proper type hints in userland code.
Are you sure having interfaces would change this?
I would assume you can't use PDO::ATTR_STATEMENT_CLASS with persistent
PDO due to a limitation of the implementation internal to PDO, rather
than anything to do with what sub-classes what.
Could you post a working example of being able to set
PDO::ATTR_STATEMENT_CLASS with persistent PDO?
cheers
Dan
That’s actually the thing that you can’t use PDO::ATTR_STATEMENT_CLASS with persistent PDO.
To make it possible to have persistent PDO with custom PDOStatement you should have:
- custom
CustomPDO implements PDOInterfacewhich will be somewhat proxy to PDO instance - custom
CustomPDOStatement implements PDOStatementInterfacewhich will be returned from CustomPDO::prepare and will have our additional logic + some stuff for persistence.
And in our userland code we can have type hints like someMethod(PDOInterface $pdo) or someMethod(PDOStatementInterface $stmt)
I hope it explains a bit how interfaces could help here.
when we are using persistent PDO we can’t use PDO::ATTR_STATEMENT_CLASS and
return our custom PDOStatement classBut just implementing PDOInterface and PDOStatementInterface will allow us to implement
this and have proper type hints in userland code.Are you sure having interfaces would change this?
I would assume you can't use PDO::ATTR_STATEMENT_CLASS with persistent
PDO due to a limitation of the implementation internal to PDO, rather
than anything to do with what sub-classes what.Could you post a working example of being able to set
PDO::ATTR_STATEMENT_CLASS with persistent PDO?cheers
Dan
To make it possible to have persistent PDO with custom PDOStatement you
should have:
- custom
CustomPDO implements PDOInterfacewhich will be somewhat proxy to PDO instance- custom
CustomPDOStatement implements PDOStatementInterface...
"I think you should be more explicit here between steps one and two.
http://www.sciencecartoonsplus.com/pages/gallery.php
I'm not saying that's not going to work......I'm saying you should try
to make a working example, to show that it works.
That PDO with persistent connections doesn't support
ATTR_STATEMENT_CLASS hints that it is doing some magic internally.
Working around that magic in userland, while preserving the persistent
connection functionality might be 'non-trivial'.
Actually, there should be tests for that functionality as part of the
PR, probably.
cheers
Dan
To make it possible to have persistent PDO with custom PDOStatement you
should have:
- custom
CustomPDO implements PDOInterfacewhich will be somewhat proxy to PDO instance- custom
CustomPDOStatement implements PDOStatementInterface..."I think you should be more explicit here between steps one and two.
http://www.sciencecartoonsplus.com/pages/gallery.phpI'm not saying that's not going to work......I'm saying you should try
to make a working example, to show that it works.That PDO with persistent connections doesn't support
ATTR_STATEMENT_CLASS hints that it is doing some magic internally.Working around that magic in userland, while preserving the persistent
connection functionality might be 'non-trivial'.Actually, there should be tests for that functionality as part of the
PR, probably.cheers
Dan
Yes, sure.
I’m just thinking that instead of creating own example, it’s better to take real one.
Here is the issue in Doctrine DBAL code similar to the one we’re discussing.
https://github.com/doctrine/dbal/issues/2315 https://github.com/doctrine/dbal/issues/2315
And here you can see couple of interfaces have been created. These interfaces are mirror of PDO and PDOStatement interface.
https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/ResultStatement.php https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/ResultStatement.php
https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/Statement.php https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/Statement.php
https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/Connection.php https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/Connection.php
Here is example implementation of this interfaces
https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/PDOStatement.php https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/PDOStatement.php
https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/PDOConnection.php https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/PDOConnection.php
And they came into issue with using ATTR_STATEMENT_CLASS (mentioned above)
I absolutely agree with you that implementing persistent stuff in user land code is non-trivial thing but it’s just one of use case where interfaces could be useful.
But I think that code above proves need of these interfaces because they created them on their own. And in general I think it makes user code a bit more readable.
I don’t think that concrete implementation of interfaces should be included in tests because it’s just one of use case of these new interfaces (not the easiest one and there could be more of them)
That’s actually the thing that you can’t use
PDO::ATTR_STATEMENT_CLASS with persistent PDO.
The actually question is: Why not? - From a quick glance on the code I
see no obvious reason. In speculation I assume the implementor thought
"Well, we can't guarantee that a class that is there in one request
will be there on the next release and it will quite certainly be at a
different memory address thus the cached class_entry pointer will be
wrong" but the user has to reset the attribute anyways ... we just have
to make sure the different dbh->def_stmt_flags are clean when a new PDO
connection object is created recovering an old connection ...
johannes
That’s actually the thing that you can’t use
PDO::ATTR_STATEMENT_CLASS with persistent PDO.The actually question is: Why not? - From a quick glance on the code I
see no obvious reason. In speculation I assume the implementor thought
"Well, we can't guarantee that a class that is there in one request
will be there on the next release and it will quite certainly be at a
different memory address thus the cached class_entry pointer will be
wrong" but the user has to reset the attribute anyways ... we just have
to make sure the different dbh->def_stmt_flags are clean when a new PDO
connection object is created recovering an old connection ...johannes
Besides code style/architecture things (which is of course questionable) the issue with ATTR_STATEMENT_CLASS is that it simply doesn’t work with persistent PDO connect and raises
"General error: PDO::ATTR_STATEMENT_CLASS cannot be used with persistent PDO instances"
when we are using persistent PDO we can’t use PDO::ATTR_STATEMENT_CLASS and
return our custom PDOStatement classBut just implementing PDOInterface and PDOStatementInterface will allow us to implement
this and have proper type hints in userland code.Are you sure having interfaces would change this?
I would assume you can't use PDO::ATTR_STATEMENT_CLASS with persistent
PDO due to a limitation of the implementation internal to PDO, rather
than anything to do with what sub-classes what.Could you post a working example of being able to set
PDO::ATTR_STATEMENT_CLASS with persistent PDO?cheers
Dan
Ouch, I’m sorry, Dan, for top posting instead of bottom one