-
Notifications
You must be signed in to change notification settings - Fork 821
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
FIX Empty relations don't have extra DB calls with eager-loading #10886
FIX Empty relations don't have extra DB calls with eager-loading #10886
Conversation
cf93d02
to
8e2a0e6
Compare
8e2a0e6
to
68c1e35
Compare
@@ -1101,7 +1107,7 @@ private function fetchEagerLoadRelations(Query $query): void | |||
$relationDataClass, | |||
implode('.', $chainToDate), | |||
$relationName, | |||
$parentRelationName |
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.
The $parentRelationName
argument was never used - in fact there wasn't even a parameter for it
case 'has_one': | ||
$dummyData = Injector::inst()->create($relationDataClass); | ||
break; |
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.
has_one
isn't used currently (see the comment about why it's not necessary in fetchEagerLoadHasOne()
), but it's here for the sake of completion. It is technically valid to pass in here, and shouldn't throw an exception if we do for some reason pass it in.
2c1f09c
to
fe1cf44
Compare
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.
Just need to update variable names so things are consistent and easier to understand
fe1cf44
to
c19ef77
Compare
c19ef77
to
aff4ee7
Compare
Issue