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

Incorrect number of database queries when retrieving a document with class inheritance #2723

Open
parada85 opened this issue Jan 28, 2025 · 4 comments

Comments

@parada85
Copy link
Contributor

parada85 commented Jan 28, 2025

Bug Report

Q A
BC Break no
Version 2.9.2

Summary

Problem class inheritance save in entitymap

How to reproduce

I have a simple class inheritance like this:

#[ODM\DiscriminatorMap([3 => Tag::class, 4 => Category::class])]
class Section {}


class Tag extends Section {}
class Category extends Section {}

Then I do something like this:

/** @var DocumentManager $manager */
$manager = Kernel::stGetObj()->getContainer()->get('doctrine_mongodb')->getManager();
$manager->getRepository(Section::class)->find(68);
$manager->getRepository(Section::class)->find(68);

It seems like two database queries are made, but only one should be executed since it should be cached in the entity map. The issue seems to be that when it looks up the entity map, it does so using the repository class, but then stores it with the actual class (the child class) instead.

18:33:35 DEBUG     [doctrine] MongoDB command: {"find":"section","filter":{"_id":68,"type":{"$in":[3,4]}},"limit":1,"$db":"db","lsid":{"id":{"$binary":"R5St6qBWSAqg\/2jT16lktA==","$type":"04"}}}
18:33:35 DEBUG     [doctrine] MongoDB command: {"find":"section","filter":{"_id":68,"type":{"$in":[3,4]}},"limit":1,"$db":"db","lsid":{"id":{"$binary":"R5St6qBWSAqg\/2jT16lktA==","$type":"04"}}}

However, if I do this:

/** @var DocumentManager $manager */
$manager = Kernel::stGetObj()->getContainer()->get('doctrine_mongodb')->getManager();
$manager->getRepository(Tag::class)->find(68);
$manager->getRepository(Tag::class)->find(68);

It works correctly, and only one query is made.

When fetching the same document twice, only one database query should be executed if the entity is already cached in the entity map, regardless of whether the parent or child class is used.

To resolve the issue, I have overridden the find method of the DocumentRepository and do the following:

$parentClass = $this->getReflectionParentClass(new \ReflectionClass($documentName));        
if ($map = $this->getDocumentManager()->getClassMetadata($parentClass->getName())->discriminatorMap) {
     foreach ($map as $class) {
        if ($ret = $this->getDocumentManager()->getUnitOfWork()->tryGetById($id, $this->getDocumentManager()->getClassMetadata($class))) {
            return $ret;
          }
     }
}

but method tryGetById is marked internal :(

@IonBazan
Copy link
Member

The objects retrieved using find() are stored in Unit Of Work based on their FQCN and id. This is an expected behaviour when using discriminators, as otherwise we would have to all possible class names in the inheritance tree of an entity every time we fetch it from the DB.
To avoid that, you can try overriding find() method in your repository class and cache these results internally.

@parada85
Copy link
Contributor Author

parada85 commented Jan 29, 2025

I understand what you're saying, and that's exactly what I'm doing in the code I posted earlier. That is, I loop through all the child classes to check if it exists in the entityMap, and if so, it returns it; something as simple as that would avoid many unnecessary database queries. Overriding the find method is an option, and I'm already doing that, but I think this behavior should be built into the library itself.

@parada85
Copy link
Contributor Author

I’ve run the test using ORM instead of ODM, and it works fine with ORM, meaning UOW handles it well even with classes that have inheritance.

@IonBazan
Copy link
Member

You are right @parada85, looks like ORM uses rootClassName in their identityMap while ODM uses the class name instead.
https://github.com/doctrine/orm/blob/aff82af7debf9c068c2e0bb4133e24a6635c070f/src/UnitOfWork.php#L2834-L2839

public function tryGetById($id, ClassMetadata $class)
{
if (! $class->identifier) {
throw new InvalidArgumentException(sprintf('Class "%s" does not have an identifier', $class->name));
}
$serializedId = serialize($class->getDatabaseIdentifierValue($id));
return $this->identityMap[$class->name][$serializedId] ?? false;
}

Let me try to find a solution for this 🙏🏻

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

No branches or pull requests

2 participants