doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.86k stars 2.5k forks source link

Add note about `Query::HINT_INCLUDE_META_COLUMNS` for custom hydrators #11461

Open Korbeil opened 1 month ago

Korbeil commented 1 month ago

Hey !

I was trying to create a custom hydrator that handle everything the same as ObjectHydrator and I was not understanding WHY I don't have meta columns when using my hydrator while ObjectHydrator had them. After some search I found: https://github.com/doctrine/orm/blob/3.1.x/src/Query/SqlWalker.php#L647-L648 that forces meta columns when using ObjectHydrator but for any other hydrator you have to pass the hint.

So I added a note about that hint in the custom hydrator documentation so people knows about this :wink:

Korbeil commented 1 month ago

Actually using this doesn't work because Query and SqlWalker are executed before the custom hydrator is called so the hint I set in my prepare method is not used at all:

1^ "sqlWalker"
2^ []
^ array:1 [
  0 => "c0_.id AS id_0, c0_.name AS name_1"
]
1^ "customHydrator.prepare"
2^ array:2 [
  "deferEagerLoad" => true
  "doctrine.includeMetaColumns" => true
]
Korbeil commented 1 month ago

After a lot of thinking, I found a (not good looking, but working) method to make this works, I still think it should be documentated somewhere that this hint exists and what it does. Also, I do think we should allow hydrators to tells when they need meta columns but as the current Doctrine architecture. How and when hydrator are called make this almost impossible.

For my use-case I made a static method:

public static function getResult(Query $query): mixed
{
    $query->setHint(Query::HINT_INCLUDE_META_COLUMNS, true);
    return $query->getResult('automapper');
}

And then, when you want the result of a query:

$qb = $userRepository->createQueryBuilder('u');
$resultAutoMapper = AutoMapperHydrator::getResult($qb->getQuery());

It's not that bad and doesn't require too much for the end-user but I still think it would be better if we could set this elsewhere.