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.9] Use eager loading optimization for all integer keys #28153

Merged
merged 1 commit into from Apr 16, 2019

Conversation

@rimace
Copy link
Contributor

commented Apr 9, 2019

With version 5.7.14 there where some performance optimizations for the whereIn method for relations (noted as "Improved eager loading performance" in the release notes).
This brought a breakage for users that didn't declare the primary key type in their models. So the optimizations where limited to primary keys which are auto-incremented and declared as integer keys.

As it is OK to break things for a new version I propose to drop the auto-increment requirement and make use of the performance optimization for all integer primary keys.

Side note: at the time the $keyTypevariable was introduces it was not neccessary to set it, as it was not really used, which was probably the reason why it was not noticed in the upgrade guide. As this is a breaking change (for those who still don't know about this variable) this change should be noted in the upgrade guide (see my comment in issue 26582 ).

Also see Idea 1561

@driesvints driesvints changed the title [master] Use eager loading optimization for all integer keys [5.9] Use eager loading optimization for all integer keys Apr 9, 2019

@taylorotwell

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

What is the breaking change? Provide a detailed upgrade guide required for this change.

@rimace

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

Users need to declare the correct primary key type in the $keyType variable in their models. This is the only thing that needs to be mentioned.
Long time Laravel users don`t know about it as it wasn't mentioned in any upgrade guide and they don*t read the whole documentation about models again (which is why it broke applications when this change was introduced in 5.7.14).

Example: If the primary key is a string but the $keyType variable was not changed (set as int as default), applications may break when a model eagerly loaded.

@taylorotwell taylorotwell merged commit 755d615 into laravel:master Apr 16, 2019

2 checks passed

continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@gallib gallib referenced this pull request Sep 2, 2019
@Korko Korko referenced this pull request Sep 3, 2019
@nasrulhazim nasrulhazim referenced this pull request Sep 4, 2019
@aaronsaray aaronsaray referenced this pull request Sep 4, 2019
@mikaeljorhult mikaeljorhult referenced this pull request Sep 6, 2019
@georgeburdell georgeburdell referenced this pull request Sep 10, 2019
@HDVinnie HDVinnie referenced this pull request Sep 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.