[12.x] fix: one of many subquery constraints

[calebdw]

Hello!

This fixes the constraints on the OneOfMany subqueries.

Regression

The regression occurred because the addConstraints call was removed from CanBeOneOfMany::ofMany as it was adding duplicate join/where clauses to the HasOneThroughOfMany relations. Upon further inspection, this was because the HasOneOrMany::addConstraints was using the query from getRelationQuery() whereas HasOneOrManyThrough::addConstraints was using the base $this->query.

Additional Considerations

Edit: I decided to forgo the signature BC, perhaps that could be looked at in a future upgrade.

The CanBeOneOfMany::ofMany calls addConstraints to add the constraints to the subquery, however, this only added the constraints to the first subquery (the innermost query assigned to $this->oneOfManySubQuery. This means that queries with more than one aggregate will not have the constraints added to the other subqueries.

The fix for this is to move the addConstraints call and pass in the subquery so that the constraints are added to every subquery.

This can be seen in the following SQL (taken from the testGlobalScopeIsNotAppliedWhenRelationIsDefinedWithoutGlobalScopeWithComplexQuery test in DatabaseEloquentHasOneOfManyTest):

SELECT "prices".*
FROM "prices"
INNER JOIN
  (SELECT max("prices"."id") AS "id_aggregate",
          min("prices"."published_at") AS "published_at_aggregate",
          "prices"."user_id"
   FROM "prices"
   INNER JOIN
     (SELECT max("prices"."published_at") AS "published_at_aggregate",
             "prices"."user_id"
      FROM "prices"
-      WHERE "published_at" < ?
-        AND "prices"."user_id" = ?
-        AND "prices"."user_id" IS NOT NULL
+      WHERE "prices"."user_id" = ?
+        AND "prices"."user_id" IS NOT NULL
+        AND "published_at" < ?
      GROUP BY "prices"."user_id") AS "price_without_global_scope" ON "price_without_global_scope"."published_at_aggregate" = "prices"."published_at"
   AND "price_without_global_scope"."user_id" = "prices"."user_id"
-   WHERE "published_at" < ?
+   WHERE "prices"."user_id" = ?
+     AND "prices"."user_id" IS NOT NULL
+     AND "published_at" < ?
   GROUP BY "prices"."user_id") AS "price_without_global_scope" ON "price_without_global_scope"."id_aggregate" = "prices"."id"
AND "price_without_global_scope"."published_at_aggregate" = "prices"."published_at"
AND "price_without_global_scope"."user_id" = "prices"."user_id"
WHERE "prices"."user_id" = ?
  AND "prices"."user_id" IS NOT NULL

[!WARNING]
This does change the Relation::addConstraints signature to accept a nullable query parameter. This will have no affect at call sites, but could be considered a BC if others have overridden this method.
If this is not acceptable then let me know and I can explore an alternate solution.

[!NOTE]
While the additional constraints on the middle query might not strictly be necessary (given that the constraints are on the innermost query and we're using an INNER JOIN), it does simplify the HasOneThrough relation which needs the intermediate table joined on every subquery.

Without these changes, there would need to be conditional checks to conditionally join based on the query level which could be considered messier.

Closes #55155

CC: @BertvanHoekelen

Thanks!

[github-actions]

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

[BertvanHoekelen]

Thanks! I just tested your branch on our project and query looks correct and I'm not running into db performance issues.

[calebdw]

This is admittedly not very pretty, but is necessary unless the Relation::addConstraints signature is changed to allow passing the query instance in (see Additional Considerations section in PR description, and the first commit diff 818ebee for an example).

The root problem is that all subqueries need the join applied, but the innermost query is already being joined in the CanBeOneOfMany::ofMany method (the addConstraints call).