-
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
Consider usage of setFetchMode when checking for simultaneous usage of fetch-mode EAGER and WITH condition #11174
Consider usage of setFetchMode when checking for simultaneous usage of fetch-mode EAGER and WITH condition #11174
Conversation
…f fetch-mode EAGER and WITH condition. This fixes a bug that arises when an entity relation is mapped with fetch-mode EAGER but setFetchMode LAZY (or anything that is not EAGER) has been used on the query. If the query use WITH condition, an exception is incorrectly raised (Associations with fetch-mode=EAGER may not be using WITH conditions). Fixes doctrine#11128
Is this to be considered? It may solve a BC introduced in #8391 |
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.
Thanks for this PR, this is indeed an oversight and the fix looks good.
I have two improvements for the PR.
use Doctrine\ORM\Query; | ||
use Doctrine\Tests\OrmFunctionalTestCase; | ||
|
||
class GH11128Test extends OrmFunctionalTestCase |
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.
Instead of adding a new test-case and model, you can just add another test to the existing by adapting for example EagerFetchCollectionTest::testSubselectFetchJoinWithNotAllowed
@@ -1047,7 +1047,7 @@ public function walkJoinAssociationDeclaration($joinAssociationDeclaration, $joi | |||
} | |||
} | |||
|
|||
if ($relation['fetch'] === ClassMetadata::FETCH_EAGER && $condExpr !== null) { | |||
if (($this->query->getHint('fetchMode')[$assoc['sourceEntity']][$assoc['fieldName']] ?? $relation['fetch']) === ClassMetadata::FETCH_EAGER && $condExpr !== null) { |
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.
if (($this->query->getHint('fetchMode')[$assoc['sourceEntity']][$assoc['fieldName']] ?? $relation['fetch']) === ClassMetadata::FETCH_EAGER && $condExpr !== null) { | |
$fetchMode = $this->query->getHint('fetchMode')[$assoc['sourceEntity']][$assoc['fieldName']] ?? $relation['fetch']; | |
if ($fetchMode === ClassMetadata::FETCH_EAGER && $condExpr !== null) { |
@Abbraxar would appreciate if you could finish this PR. If you don't have time I could do it! |
Hi @aprat84 ! |
Can't edit this PR, so cloned it: #11445. |
Closing this PR, go see #11445 |
Concern issue #11128