liam-wiltshire / laravel-jit-loader

MIT License
214 stars 12 forks source link

Nested relations #18

Open mathieutu opened 5 years ago

mathieutu commented 5 years ago

Hi, thanks for this package. This is a simple but neat idea!

I've a problem with nested relations.

Example (pseudo code):

Parent::all()->each(function (Parent $parent) {
    $parent->children->each(function (Child $child) {
        $child->grandChildren;
    });
});

Saying that I have 2 parents, that have 3 children each, which have 4 grandChildren each, so a total of 2 3 4 objects.

With a proper eager loading (load('children.grandChildren'))I should have ~3 sql calls, one for each type.

Here we have one call for the parents (default one), one call for the children (auto eager loading), and 3 calls for the grandChildren (because one for each collection of children), so a total of 5 calls instead of 3. And it will be the same for each sub level.

I don't know if I made me properly understood, as I'm not a native speaker, but I think this is a real problem.

I have some clues about how to adress that, but I'm not sure about it.

As we store the parent collection, we can store the relations that were called, and so for each level, we can get the first collection, and call a loadMissing with the nested new relations on it (parents->loadMissing('children.grandChildren') in our case).

What's are your thoughts about it? Do you have any better idea?

liam-wiltshire commented 5 years ago

Hi There,

I'll have to look into this - I think yes when you have nested relations that load collections this will happen - it'll still be significantly less queries than not having any eager loading, but it'll be much worse than having proper eager loading.

Thanks!

liam-wiltshire commented 5 years ago

So I'm working on a solution to this, however the solutions I have currently require changes to the Collection itself (which isn't really a practical solution), so trying to find a way to get it to work just juggling the models - will keep you posted :-)

liam-wiltshire commented 5 years ago

Hi @mathieutu

Sorry, been a while since we last talked about this.

I'm working on a PR that I'm pretty sure does what you require, however I've only tested it on a fairly 'sterile' test dataset, so if you've got time, it'd be great to see if it works as expected on 'real-life' data?

https://github.com/liam-wiltshire/laravel-jit-loader/pull/22

Thanks,

Liam