Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

[Proposal] sort foreign keys in with() method | eager loading #378

Closed
tarkis opened this issue Jan 24, 2017 · 2 comments
Closed

[Proposal] sort foreign keys in with() method | eager loading #378

tarkis opened this issue Jan 24, 2017 · 2 comments

Comments

@tarkis
Copy link

tarkis commented Jan 24, 2017

Laravel Version: 5.4
PHP Version: 7.0.14
Database Driver & Version: MySQL 5.5.53

Problem
I have one problem with eager loading to be exact with ordering in with() method. I use cache that generates key from SQL query.

I noticed that I have same cache value store multiple times under different key. After some testing I found a problem what with() method does not reorder foreign keys prior database query call.

Example: I have few models: town and street and they are connected via relationship. If on a first page I would call: street::with('town')->paginate(10) the SQL query for with() would look like: select * from `towns` where `towns`.`id` in ('1, '2', '3') but on a second page it looks like: select * from `towns` where `towns`.`id` in ('2, '1', '3') and here is the problem that it returns same SQL result but from to different yet same queries.

Solution
To add sort() or just helper function array_sort_recursive() at functions that returns keys. Same would be for 5.3 version with minor changes at https://github.com/laravel/framework/blob/5.3/src/Illuminate/Database/Eloquent/Relations/Relation.php#L201.

Change:
https://github.com/laravel/framework/blob/5.4/src/Illuminate/Database/Eloquent/Relations/Relation.php#L201
To:

$keys = collect($models)->map(function ($value) use ($key) {
    return $key ? $value->getAttribute($key) : $value->getKey();
})->values()->unique()->all();
sort($keys);

return $keys;

and Change:
https://github.com/laravel/framework/blob/5.4/src/Illuminate/Database/Eloquent/Relations/BelongsTo.php#L133
To:

$keys = array_values(array_unique($keys));
sort($keys);

return $keys;
@Garbee
Copy link

Garbee commented Jan 24, 2017

Just make a pull request for proposals where you already know what to change.

@tarkis
Copy link
Author

tarkis commented Feb 2, 2017

Created a pull request at laravel/framework#17730

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants