-
-
Notifications
You must be signed in to change notification settings - Fork 23
Allow simulation of DateTime objects #339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Could you point me to a code example where this feature is needed? |
There is an example in #278 - I don't have anything public sorry. But essentially doctrine allows binding advanced data types, so it'd be great if we could support that here. |
@staabm any idea what I can do to help move this forward? :) |
could you provide a failling unit test? |
hmm are you sure there is more then 1 arg to |
ohh I see, you actually meant doctrine |
Yeah sorry PDO::exec was a wrong example, DBAL's connection has them and PDOStatement too has a 3rd param $type on bindParam https://www.php.net/manual/en/pdostatement.bindparam.php that takes one of the PDO::PARAM_* consts https://www.php.net/manual/en/pdo.constants.php So IMO taking this $type into consideration would be good anyway for PDO too (i.e. error if the type is provided but does not match the real type given as value), but it is even more essential for doctrine because it supports additional types like the DATETIME_* ones. |
Yes I'm running on my fork for now, which works for me, so I can wait a bit longer for sure. Up to you. |
Partially addresses #278 - but the way I see it, in QuerySimulation::simulateParamValueType you don't have access to the param types argument, only to the parameter themselves?
I mean
PDO::exec($query, $params, $paramTypes)
<- this param types argWithout access to that we can serialize the datetime to a Y-m-d H:i:s format, but it won't be always correct, as if you pass Doctrine\DBAL\Types\Types::DATE for ex then it should use Y-m-d only. And strictly speaking we should only simulate/serialize DateTime objects if the appropriate Type is passed in $paramTypes. Otherwise it's unlikely to be a bug if you have a DateTime in there as it will fail at runtime.
This patch does fix the issue I have though so it's a improvement as it allows me to run phpstan-dba on this project of mine, but it's hardly a comprehensive fix.