[12.x] Apply eager load constraints to one-of-many subqueries#60060
[12.x] Apply eager load constraints to one-of-many subqueries#60060metehankuscu wants to merge 1 commit into
Conversation
192992d to
a635140
Compare
|
My agent's findings:
src/Illuminate/Database/Eloquent/Builder.php:954 copies eager-load wheres only to getOneOfManySubQuery(), which is the first/inner one-of-many subquery. For relations like latestOfMany(['published_at', 'id']) or ofMany(['published_at' => 'max', 'id' => 'max']), later tie-breaker subqueries remain unconstrained. Example: constraining eager load to an older id with the same published_at can still let the intermediate subquery select max(id) from the unconstrained rows, causing the final outer whereKey to discard the result and return null. The added test only covers a single-column aggregate, so this regression path is not covered.
User::with([ the patch copies where states.type = ? into the one-of-many subquery, but the states join remains only on the outer query. That makes the generated subquery reference a missing table/alias and can fail at runtime. Existing one-of-many relation definition closures already support joins because they receive the subquery itself; this eager-load path does not. |
a635140 to
d1ec31e
Compare
|
Thanks for the detailed review. I updated the implementation and tests to address both issues. The implementation now tracks all one-of-many aggregate subqueries and applies newly added eager-load joins and where constraints to each aggregate subquery. This covers multi-column ofMany relationships such as latestOfMany(['published_at', 'id']) and eager-load constraints that add joins. I added regression coverage for single-column, multi-column, and join-based eager-load constraints. I also verified that the existing DatabaseEloquentBuilderTest::testRelationshipEagerLoadProcess case still passes. |
|
Thanks for your pull request to Laravel! Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include. If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions! |
Fixes #59318.
Problem
When eager loading a
latestOfMany/ofManyrelationship with a constraint closure, the constraint is currently only applied to the outer relation query.For example:
The one-of-many aggregate subquery still determines the latest related model from the unconstrained dataset. If the absolute latest related model does not match the eager-load constraint, the outer query filters it out and the relationship resolves to
null, even though a matching related model exists.This is unintuitive because eager-load constraints normally scope the relationship results. In this case, the constraint must also affect the aggregate subquery because that subquery determines which related model is the "one" model.
This also affects multi-column one-of-many relationships such as
latestOfMany(['published_at', 'id']), where later tie-breaker subqueries can still select from the unconstrained dataset.Solution
This change tracks all one-of-many aggregate subqueries created by the relation.
When eager-load constraints are applied to a one-of-many relation, newly added
joinandwhereconstraints are also applied to each one-of-many aggregate subquery.This allows the aggregate subqueries to select the related model from the constrained dataset instead of selecting the absolute latest related model and letting the outer query discard it.
This also keeps join-based eager-load constraints valid by applying the newly added joins together with their related where constraints.
Why this should not break existing behavior
The change is scoped to eager loading one-of-many relationships.
It does not copy the full query state into the aggregate subqueries. It only forwards newly added eager-load
joinandwhereconstraints, along with their bindings, to the one-of-many aggregate subqueries.Existing one-of-many relationship definitions continue to behave the same. The outer query still receives the eager-load constraints as before; the aggregate subqueries now receive the same relevant constraints only where they are required to determine the correct aggregate row.
Benefit to end users
This prevents silently incorrect relationship results. Developers can constrain eager-loaded
latestOfMany/ofManyrelationships and receive the latest related model within the constrained dataset.Without this fix, applications may receive
nullfor a relationship even when a matching related model exists.Tests
Added integration regression tests to
DatabaseEloquentHasOneOfManyTestcovering:The tests verify that eager-load constraints are applied to the aggregate subqueries used to determine the selected one-of-many record.
Before this fix, the single-column and multi-column cases could return
nullbecause the aggregate subquery selected an unconstrained record that was later discarded by the outer query. Join-based eager-load constraints could also produce invalid SQL when a copied where clause referenced a join that was not present in the aggregate subquery.Ran: