[5.6] [WIP] Can't lazy-load polymorphic relationships with different primary key columns

[fitztrev]

As per the contribution guide, I'm submitting a bug report in the form of a pull request containing a failing test.

Summary

You cannot lazy-load models of a polymorphic relationship that have different primary key columns.

$comments = Comment::all();
$comments->load('commentable'); // this fails if you have the below schema

posts

post_id	title	body
111	Test Post	hello world

videos

video_id	title	video
222	Test Video	my-video.mov

comments

id	commentable_type	commentable_id	body
1	App\Post	111	"comment on a post"
2	App\Video	222	"comment on a video"

// This works (eager loading):
$comments = App\Comment::with('commentable')->get();

// This fails (lazy loading):
$comments = App\Comment::all();
$comments->load('commentable');
// > Results in "no such column: videos.post_id"

To run the test(s) for yourself:

## Pull down my fork
git clone -b polymorphic-diff-keys git@github.com:fitztrev/framework.git polymorphic-diff-keys

cd polymorphic-diff-keys
composer install

## Run the tests
## These 2 work
./vendor/bin/phpunit --filter testWithNoEagerOrLazyLoad
./vendor/bin/phpunit --filter testItCanEagerLoad

## This 1 fails
./vendor/bin/phpunit --filter testItCanLazyLoad

As a footnote, I understand that having primary keys like in my example doesn't make a whole lot of sense. However that is not under my control and not something I can change.

[tillkruss]

Can you include a fix as well?

[fitztrev]

Can you include a fix as well?

I've tried but can't figure one out. The closest I've gotten is I think it's attempting to use the ownerKey from the first relationship on any that come after it:

framework/src/Illuminate/Database/Eloquent/Relations/MorphTo.php

Lines 119 to 123 in ab02a65

	protected function getResultsByType($type)
	{
	$instance = $this->createModelByType($type);
	$ownerKey = $this->ownerKey ?? $instance->getKeyName();

Hopefully someone who better understands this part of the code can provide some insight.

[staudenmeir]

I would say that the problem originates in Collection::load() where the first model is used to create the eager loading queries:

framework/src/Illuminate/Database/Eloquent/Collection.php

Line 57 in ab02a65

$query = $this->first()->newQueryWithoutRelationships()->with($relations);

In my test I could solve it by manually forcing the lazy loading into morphEagerTo():

framework/src/Illuminate/Database/Eloquent/Concerns/HasRelationships.php

Lines 193 to 195 in ab02a65

	return empty($class = $this->{$type})
	? $this->morphEagerTo($name, $type, $id, $ownerKey)
	: $this->morphInstanceTo($class, $name, $type, $id, $ownerKey);

But how can we detect lazy loading? The only way I can think of is analyzing the backtrace, which is rather terrible...

[fitztrev]

It appears #21310 is when this stopped working.

[taylorotwell]

I can't really take any action on this PR since it's just a broken test. If someone can contribute a fix that is better.

[fitztrev]

@victordzmr @fntneves Could you provide any insight on this? When laravel/ideas#587 was implemented, this functionality broke.

[v0ctor]

In principle, when $ownerKey is null, behaviour should be exactly the same as before the pull request #21310 (see changes). I need more time to investigate it.

I'm sorry for the issue.

[staudenmeir]

Since #21310 $this->ownerKey is never null in getResultsByType() when lazy loading. It's either the relationship's "real" $ownerKey (then lazy loading works) or the $primaryKey of the collection's first related model (in the example: a App\Post instance).

As a result, $instance->getKeyName() is not used when lazy loading.