-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Cache null/empty returns for findOneBy #11584
base: 3.3.x
Are you sure you want to change the base?
Cache null/empty returns for findOneBy #11584
Conversation
On 'findOneBy' calls which return no an empty result set from the persister, null values are not cached. This makes it so that repeat calls of 'findOneBy' with the same criteria, always hit the database. The changes introduced in this commit make the AbstractEntityPersister cache the empty result set in the region, so that repeat calls do not hit the database again. The existing mechanism to invalidate the query cache is sufficient; any persist/update performed to the region will invalidate it, exactly how it happens for non-empty result sets. This is also more in-line with the way 'loadByCriteria' and 'loadAll' behave.
Ill be adding/fixing tests here in the next few days |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this a BC break ?
Before, I could have
$result = $repository->findOne(['foo' => $foo]);
$result2 = $repository->findOne(['foo' => $foo]);
And getting result2 !== null
because of concurrency and another process could have inserted the result in DB.
After this PR it will still return NULL. So I cannot check again my DB.
@VincentLanglet If a value is inserted in DB, the timestamp region is updated. It is no different than caching a value. |
On 'findOneBy' calls which return no an empty result set from the persister, null values are not cached.
This makes it so that repeat calls of 'findOneBy' with the same criteria, always hit the database.
The changes introduced in this commit make the AbstractEntityPersister cache the empty result set in the region, so that repeat calls do not hit the database again.
The existing mechanism to invalidate the query cache is sufficient; any persist/update performed to the region will invalidate it, exactly how it happens for non-empty result sets.
This is also more in-line with the way 'loadByCriteria' and 'loadAll' behave.
Fixes #11563