Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-2240

Inconsistent querying for parameter type (from ClassMetadata) between using Find/FindBy and DoctrineQL

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Duplicate
    • Affects Version/s: 2.3.1
    • Fix Version/s: None
    • Component/s: DQL
    • Security Level: All
    • Labels:

      Description

      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

      1. UTCDateTimeType.php
        2 kB
        Slavik Derevyanko
      1. DoctrineQL methods trace.png
        159 kB
      2. findBy methods trace.png
        141 kB

        Issue Links

          Activity

          Slavik Derevyanko created issue -
          Hide
          Slavik Derevyanko added a comment -

          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.

          Show
          Slavik Derevyanko added a comment - 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.
          Hide
          Benjamin Eberlei added a comment -

          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.

          Guilherme Blanco any idea what to do?

          Show
          Benjamin Eberlei added a comment - 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. Guilherme Blanco any idea what to do?
          Hide
          Guilherme Blanco added a comment -

          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. =)

          Show
          Guilherme Blanco added a comment - 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. =)
          Hide
          Benjamin Eberlei added a comment -

          Same as DDC-2224

          Show
          Benjamin Eberlei added a comment - Same as DDC-2224
          Benjamin Eberlei made changes -
          Field Original Value New Value
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Duplicate [ 3 ]
          Benjamin Eberlei made changes -
          Link This issue is duplicated by DDC-2224 [ DDC-2224 ]
          Hide
          Benjamin Eberlei added a comment -

          A related Github Pull-Request [GH-574] was opened
          https://github.com/doctrine/doctrine2/pull/574

          Show
          Benjamin Eberlei added a comment - A related Github Pull-Request [GH-574] was opened https://github.com/doctrine/doctrine2/pull/574
          Hide
          Slavik Derevyanko added a comment -

          Great, thanks!

          Show
          Slavik Derevyanko added a comment - Great, thanks!

          This list may be incomplete, as errors occurred whilst retrieving source from linked applications:

          • Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={query=DDC-2240, expand=changesets[0:20].revisions[0:29],reviews}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

            People

            • Assignee:
              Benjamin Eberlei
              Reporter:
              Slavik Derevyanko
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: