spotorm / spot2

Spot v2.x DataMapper built on top of Doctrine's Database Abstraction Layer
http://phpdatamapper.com
BSD 3-Clause "New" or "Revised" License
601 stars 101 forks source link

Fix Mapper::query() method to pass param types. #208

Open andrejs opened 7 years ago

andrejs commented 7 years ago

When using custom query, it's not possible to use custom types for query parameters. For example:

$mapper->query(
    'SELECT * FROM user WHERE state IN (?)',
    [
        ['pending', 'active']
    ],
    [
        \Doctrine\DBAL\Connection::PARAM_STR_ARRAY
    ]
);

This pull request fixes the problem by passing custom param types to \Doctrine\DBAL\Connection::executeQuery() via Mapper::query().

FlipEverything commented 7 years ago

Off: The custom query above is just an example, right? Because you don't need to use a custom query for that. You can execute it like this: $mapper->where(['state IN' => ['pending', 'active']]);

On: I can't say anything about the usefullness of passing through the custom param type.

andrejs commented 7 years ago

Yes, it's just an example for the sake of simplicity. Intended usage of \Spot\Mapper::query() is for custom, complex queries, where you might also have typed parameters. Currently using query() you can't specify them, however it's supported directly by DBAL (see \Doctrine\DBAL\Connection::executeQuery). The method just doesn't proxy them. IMHO this is unnecessary limitation of Spot.

FlipEverything commented 7 years ago

I agree!

tuupola commented 7 years ago

Needs tests, otherwise looks good to me. Valitron bug was already fixed with #217.