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.4] Sort foreign keys in with() method | eager loading #17730

Merged
merged 1 commit into from
Feb 3, 2017

Conversation

tarkis
Copy link
Contributor

@tarkis tarkis commented Feb 2, 2017

This sorts relation keys.

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.

At BelongsTo.php its possible to do sorting after all unique values are selected. This would sort smaller array.

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

sort($keys);

return $keys;

@taylorotwell
Copy link
Member

I don't understand why it matters?

@tarkis
Copy link
Contributor Author

tarkis commented Feb 2, 2017

Hi @taylorotwell,

as described here at laravel/ideas#378 I use cache that generates keys based on SQL queries. I noticed that I have same cache value store multiple times under different key. This change would help to save cache space and would increase select speed because It would need to access SQL database only once with one query in ('1', '2', '3'). Right now it stores 6 times under 1,2,3, 1,3,2, 2,1,3 and so on

@kestutisj
Copy link

Hi,

I have the same problem as @tarkis. I noticed that several different keys having same value in redis, but this is because I'm extending Illuminate\Database\Query\Builderclass and generates cache keys that looks like this:

$key = hash('sha256', $this->connection->getName() . $this->toSql() . serialize($this->getBindings()));

And yes, I'm agree with @tarkis, it will be nice to have sorted foreign keys, because sometimes it let's avoid this problem :)

@taylorotwell taylorotwell merged commit 0ba0887 into laravel:5.4 Feb 3, 2017
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.

3 participants