Add failing tests and TODOs for doctrine types

[Seldaek]

Refs #278 and #339

See also doctrine param binding docs: https://www.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/data-retrieval-and-manipulation.html#binding-types

[Seldaek]

I don't think doctrine will fail though, the distinction between datetime/datetime_immutable is only made there when deserializing data from DB back into PHP AFAIK, so maybe we can let it go through..

[staabm]

thank you!

don't let you mislead by the current build errors.. they seem to be happening also on master - as I realized yesterday.

I need to investigate what needs to be done about them.

[Seldaek]

See e8b4e64 for a demo of what could be done to solve what I said in #339

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 arg

Without 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.

I don't know if this is the best/correct way to do this though, and I'm completely lost as to how to resolve the bound types further, but hopefully it is clear enough that you may be able to go forward with this? Otherwise as I said in #339 just merging that fix (e.g. the first two commits here) would already be an improvement altho it would be very permissive and perhaps not catch mistakes.

[staabm]

thank you so far. I will have a closer look when time allows.

[staabm]

this is looking really good already.

I will merge #339 for a short-term fix to unblock you in your projects and will have a deeper look into this PR here for a future release.

the failling test-cases are really valueable, so thank you.

[staabm]

memo for myself: PDO has the same bound parameter semantics

[staabm]

does doctrine allow to pass in a DateTime object without also giving a parameter-type as a 2nd arg?

or should phpstan-dba error when arg1 is DateTime and arg2 is either missing or not equal to Types::DATETIME_MUTABLE?

edit: I see, there are a few types arround dates in doctrine: https://github.com/doctrine/dbal/blob/007223cfe9dc5cf37420a6a8d8ecd48d520b69d4/src/Types/Types.php#L18-L24

[Seldaek]

I am not sure if it lets you do it without the param type, but I'd think not.

[staabm]

should this test also use $conn->executeQuery() instead? it seems there is no prepare+execute api including bound-parameter-types...?

[Seldaek]

DBAL supports this on all connection methods it seems https://github.com/doctrine/dbal/blob/3.3.x/src/Connection.php#L529

Also on quote https://github.com/doctrine/dbal/blob/3.3.x/src/Connection.php#L811

So I guess ideally this would be supported on all the things, but executeStatement/executeQuery would be the must-haves IMO as it's the lowest common denominator and the rest is kinda decorating this.

[staabm]

just to let you know. this is not forgotten, but I will do some internal cleanup before starting again on this one

[Seldaek]

No worries and no rush really