Skip to content
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

DDC-2240: Inconsistent querying for parameter type (from ClassMetadata) between using Find/FindBy and DoctrineQL #2939

Closed
doctrinebot opened this issue Jan 11, 2013 · 11 comments
Assignees
Labels

Comments

@doctrinebot
Copy link

Jira issue originally created by user slavik2121:

Hi,

I have stumbled on a problem were querying the same data with different methods (findBy and DQL) retrieves different results.

I have extended the Doctrine\DBAL\Types\DateTimeType with my own type
BVD\PetroleumBundle\DoctrineExtensions\DBAL\Types\UTCDateTimeType,

which I attach.
I expect, that whenever we deal with an entity that has a property of this type, both convertToDatabaseValue() (whenever we access the DB, conversion from DateTime to a string) and convertToPHPValue() (whenever the result from DB query is returned back, conversion from string to DateTime) have to be executed.

It is important, as the single purpose of convertToDatabaseValue() is to perform conversion of the incoming DateTime to the UTC timezone prior to conversion to string,
and then whenever we convert the DB value to DateTime to set it's timezone not to default value (server local timezone), but to UTC (as the values are stored in UTC).

Example:
Query:
$entity = $em->getRepository('BVDPetroleumBundle:FuelCardTransaction')->findOneBy(array('id' => $id, 'processed_datetime' => new \DateTime('2011-03-10 23:58:37')));

as you see, DateTime object is created without DateTimeZone set, which makes it employ the server's default timezone (say EST).

Entity has this property registered as:
/****
* @Orm\Column(type="utcdatetime")
*/
public $processed_datetime;

And this datatype is registered with Doctrine through Symfony2 configuration:
doctrine:
dbal:
types:
utcdatetime: BVD\PetroleumBundle\DoctrineExtensions\DBAL\Types\UTCDateTimeType

Whenever I query the DB, prior to SQL generation, DateTime is getting converted to UTC by UTCDateTimeType#convertToDatabaseValue(), and becomes:
'2011-03-10 23:58:37' (EST) -> '2011-03-11 04:58:37' (UTC).
Then, whenever the object is retrieved back, I expect that UTCDateTimeType#convertToPHPValue() is used to set the correct timezone information
on the created DateTime object: '2011-03-11 04:58:37' (UTC).
This is the correct behaviour that is expected, and is correctly achieved by using findBy methods to retrieve data:

$entity = $em->getRepository('BVDPetroleumBundle:FuelCardTransaction')->findOneBy(array('id' => $id, 'processed_datetime' => new \DateTime('2011-03-10 23:58:37')));

But, when the DQL is used to issue the same query:

$queryBuilder = $em->createQueryBuilder()->select('a')->from('BVDPetroleumBundle:FuelCardTransaction','a')
->where('a.id = :transaction_id')
->andWhere("a.processed_datetime = :datetime")
->setParameter('transaction_id', $id)
->setParameter("datetime", new \DateTime('2011-03-10 23:58:37'));
$entity = $queryBuilder->getQuery()->getOneOrNullResult();

Doctrine\DBAL\Types\DateTimeType#convertToDatabaseValue() is getting executed for 'processed_datetime', instead of
BVD\PetroleumBundle\DoctrineExtensions\DBAL\Types\UTCDateTimeType,

and the conversion doesn't happen, so the query doesn't return the result, that really exists in DB.

I attach two methods traces, so it's easier to identify the problem: whenever the findBy is used, and whenever the DQL is used.
I have managed to trace it to the way how both methods retrieve their $types arrays.

The reason it succeeds when used with findBy methods:
Doctrine\ORM\Persisters\BasicEntityPersister#load() is used to retrieve the data.
The $types property that holds the type information ('utcdatetime') is formed by calling
BasicEntityPersister#expandParameters($criteria), and in the process of analyzing incoming parameters it queries the entity metadata (@Orm\Column(type="utcdatetime")),
stored in BasicEntityPersister#$_class property. (method BasicEntityPersister#getType())
Then it's able to match type 'utcdatetime' to class BVD\PetroleumBundle\DoctrineExtensions\DBAL\Types\UTCDateTimeType

The reason it fails with DQL:
It seems that with DQL, it doesn't query the entity metadata (@Orm\Column(type="utcdatetime")) to derive property type. This mechanism leads the type to be recognized as simply 'datetime', and the standard handler Doctrine\DBAL\Types\DateTimeType is used instead:

The $types (which has 'datetime' instead of 'utcdatetime') array is getting formed in
Doctrine\ORM\Query#_doExecute():
list($sqlParams, $types) = $this->processParameterMappings($paramMappings);

in Doctrine\ORM\Query#processParameterMappings($paramMappings)
Doctrine\ORM\Query#processParameterValue($parameter->getValue()) is called to convert parameter from Object to string.

in Doctrine\ORM\AbstractQuery#processParameterValue($value) for object of class DateTime I would expect this to be executed:
case is_object($value) && $this->_em->getMetadataFactory()->hasMetadataFor(ClassUtils::getClass($value)):
return $this->convertObjectParameterToScalarValue($value);

but it's not, and the DateTime is returned out of it, and in Doctrine\ORM\Query\processParameterMappings $type is getting set to $parameter->getType() ('datetime')

Please confirm/contradict the issue. Right now for workaround, whenever I use DQL, have to explicitly set the timezone of DateTime prior to issuing a query.

From Russia with love,
Slavik

@doctrinebot
Copy link
Author

@doctrinebot
Copy link
Author

Comment created by slavik2121:

I realized, that with DQL,
the default type 'datetime' of Doctrine\ORM\Query\Parameter for DateTime objects
is getting set
by Doctrine\ORM\Query\ParameterTypeInferer#inferType(),

and that it's possible to set the type as a third parameter:

    $queryBuilder = $em->createQueryBuilder()
        ->select('a')
        ->from('BVDPetroleumBundle:FuelCardTransaction','a')
        ->where('a.id = :transaction_id')
        ->andWhere("a.processed_datetime = :datetime")
        ->setParameter('transaction_id', $id)
        ->setParameter("datetime", new \DateTime('2011-03-10 23:58:37'), 'utcdatetime');

It seems, this is worth noting in the documentation.

@doctrinebot
Copy link
Author

Comment created by @beberlei:

Verified, but I don't know how to fix it without breaking BC.

As a workaround you can convert the value yourself in your code, not the nicest solution, but when wrapped in a function call of your own, it shouldn't be to invasive.

[~guilhermeblanco] any idea what to do?

@doctrinebot
Copy link
Author

Comment created by @guilhermeblanco:

There's a way currently to fix this issue.
Of course that we lack of some direct support, but you can take advantage of a second method to fix your problem.

Currently, setParameter only accepts key, value, type as arguments, creating its own Query\Parameter.
But if you look at setParameters receiving an ArrayCollection, it doesn't create the Parameter. This is where you can take advantage.

Ideally, any Type could convert back and forth from DB to PHP value. During a query, the algorithm should apply also. But if we do this change, we will introduce a BC break. To solve the issue, you'll have to create your own Parameter.

From the Doctrine perspective, we only need to support $key to be a class too. If it's a class, replace the value in the collection of parameters. This is the required change in our codebase.
But until this gets done, setParameters is already compatible with the solution.

All you have to do is create a class that extends Query\Parameter, then apply your required changes when doing getValue or during object construction. Then use the method I mentioned to inject an ArrayCollection of Parameters and everything will work. =)

@doctrinebot
Copy link
Author

Comment created by @beberlei:

Same as DDC-2224

@doctrinebot
Copy link
Author

Issue was closed with resolution "Duplicate"

@doctrinebot
Copy link
Author

Comment created by @beberlei:

A related Github Pull-Request [GH-574] was opened
#574

@doctrinebot
Copy link
Author

Comment created by slavik2121:

Great, thanks!

@doctrinebot
Copy link
Author

Comment created by @doctrinebot:

A related Github Pull-Request [GH-1339] was unlabeled:
#1339

@doctrinebot
Copy link
Author

Comment created by @doctrinebot:

A related Github Pull-Request [GH-1339] was assigned:
#1339

@doctrinebot
Copy link
Author

Comment created by @doctrinebot:

A related Github Pull-Request [GH-1339] was merged:
#1339

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants