[5.7] "Constraining Eager Loads" callout for unsupported eager load limit() #4918
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The callout second sentence I came up with is kind of clunky so maybe we don't need the "why?" It's difficult to succinctly describe the groupwise-maximum query problem in a short warning alert.
This pull request covers issues:
Since 2014, GitHub issues have frequently been submitted to the framework with a misunderstanding that
limit()
can be used by:The underlying issue is that developers don't realize these active record queries are not "regular" queries since they must be built and run once to fetch many Eloquent models. So the
limit()
call applies to fetching relations for all models and it doesn't run an SQLLIMIT
per model.(Aside from not realizing the SQL being run) I think this confusion stems from these two phrases in the docs:
"Constraints" implies that SQL keywords
limit()
andoffset()
can be used.This link to https://laravel.com/docs/5.7/queries implies all methods documented on that page can be used.
We can reduce developer confusion by:
orderBy()
still works and isn't a "condition".limit()
can't be called.Suggesting a 3rd-party package solution
To go an extra step, the docs can link to Composer package "staudenmeir/eloquent-eager-limit" that supports eager load
limit()
with more recent databases. To communicate this is not a sanctioned package in the Laravel ecosystem, the docs should be clear support for that solution is external.