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

[5.7] Support MorphTo eager loading with selected columns #25662

Merged
merged 1 commit into from
Sep 25, 2018
Merged

[5.7] Support MorphTo eager loading with selected columns #25662

merged 1 commit into from
Sep 25, 2018

Conversation

staudenmeir
Copy link
Contributor

When eager loading MorphTo relationships, you can't limit the selected columns (of course, these columns have to be available in all the related tables):

Comment::with('commentable:id,title')->get();
# expected
select "id", "title" from "posts" where "posts"."id" in (?)

# actual
select * from "posts" where "posts"."id" in (?)

As with withoutGlobalScopes() (#25331), we can use the $macroBuffer to fix this. We can also simplify the whole implementation.

Fixes #2966.

@sisve
Copy link
Contributor

sisve commented Sep 17, 2018

Removing the withoutGlobalScopes looks like a breaking change. Are you sure that methods needs to be removed? Should this target the 5.8 release instead?

@staudenmeir
Copy link
Contributor Author

It doesn't have to be removed, but the new implementation is much shorter.

What would be a case where removing the method breaks existing code? If you override it, calling parent::withoutGlobalScopes() still works.

@sisve
Copy link
Contributor

sisve commented Sep 18, 2018

You're right, I was to quick to comment about these changes. Since the same behavior is kept in __call then my initial comment about breaking change was wrong.

@taylorotwell taylorotwell merged commit 6cca1af into laravel:5.7 Sep 25, 2018
@staudenmeir staudenmeir deleted the morph-to-select branch September 25, 2018 14:32
taylorotwell pushed a commit that referenced this pull request Sep 26, 2018
* Support MorphTo eager loading with selected columns (#25662)

* version

* hyphen-case the Syslog program name

Follow existing program name conventions to have more readable logs.

* Use Str directly
@lk77
Copy link

lk77 commented Oct 26, 2018

Hello,

#25662 introduce a bug,
we have thousand of queries using the with 'relation:field' syntax with polymorphic relations,
and it's not working anymore, the relation is now empty.
i don't know why, but it dont happen in 5.7.5, only until 5.7.6

work :

->with('morphTo');

don't work :

->with('morphTo:label');

if you have an idea, thanks.

@staudenmeir
Copy link
Contributor Author

@lk77 Please open a new issue and provide more details. How is the relationship defined? What does "don't work" mean?

@lk77
Copy link

lk77 commented Oct 29, 2018

@staudenmeir the relation is empty.
the relation is a very simple polymorphic relation,
using :

->morphTo('relation')

i would open an issue if i can replicate on fresh laravel install.

thanks.

edit: issue opened #26291

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

Successfully merging this pull request may close these issues.

None yet

4 participants