Should relationships group additional where clauses?

[Kaylakaze]

So the question may not be phrased right, so it may make more sense to explain in code.

Assuming Player and Test are models and there is a hasMany relationship where Player has many Tests.
Should $player->tests()->where('x', 1) generate select * from "tests" where "tests"."player_id" = "1" and "tests"."player_id" is not null and "x" = "1" or select * from "tests" where "tests"."player_id" = "1" and "tests"."player_id" is not null and ("x" = "1").

It may seem trivial using this example, but let's add a whereOr into the mix.
$player->tests()->where('x', '1')->orWhere('y', '2') generates select * from "tests" where "tests"."player_id" = "1" and "tests"."player_id" is not null and "x" = "1" or "y" = "2" where clearly the intent is to generate select * from "tests" where "tests"."player_id" = "1" and "tests"."player_id" is not null and ("x" = "1" or "y" = "2")

Clearly the intent of the user is to find the related tests where x =1 or y =2, however, the SQL generated will return ALL tests where y = 2. This is, of course, solved by using ->where(fn($q) => $q->where('x',1 )->orWhere('y', 2)) but that is awkward and, if you don't use whereOr very often on relationships, it may not register that you need to do it that way at first (by which I mean until your data has been destroyed).

This becomes even more problematic if you're using conditional logic to build your query.
$player->tests()->whereIn('x',[$a,$b])->when(is_null($b), fn($q) => $q->orWhereNull('x'))

When testing, you're likely using a limited data set and aren't going to be noticing that, were your data set expanded and in certain conditions, like $b = null, you'll be pulling far more records than you want. Even if you're using enough data so that that would happen, unless you're explicitly thinking about such an issue occurring, you probably won't be testing if more records than expected were returned since the one you expected to be returned is also being returned. (Oh, who am I kidding? Of course you're the hardcore testing type that writes at least 15 tests per line of code! You'd definitely catch this early.)

I can think of no situation where you'd request a relationship and then not want to have additional where clauses being added only to those related results.

This seems to be related to #55587 but there it seems that there's more discussion of "this is how it is" and less "is this how it should be".

[macropay-solutions]

The or has been discussed several times besides the link you wrote. You need to put the parenthesis manually with a callback if you need it. This is how it should be. The dev needs to learn how to use the framework. You can't prevent stupid things from happening.

Also using or will mess up the index selection and will generate slow queries, on top of the fact that eloquent generates already slow relationships queries.

Because of this we did not implement over api the or condition except for (col is null or col in (1,2.3)) which refers to the same column.

Update example #48339 (comment)

[Kaylakaze]

That's a stupid response.
It's about usability. It's not stupid to expect a framework that's meant to make things easier for the developer to do something stupid like this. Of course it makes sense to put parens manually when you're ORing with AND statements, but it doesn't really make sense on a relationship because those ANDs are being hidden by the system, so the system should be responsible for that.

And the only person I ever see responding to comments about this is you. That's hardly a "discussion." Plus, if it's been brought up several times, than it's clearly an issue.

[macropay-solutions]

@Kaylakaze the idea is NOT stupid, in fact it is a good idea but, using orWhere without parenthesis IS.

• The relation forwards the calls to eloquent builder when the method is not defined in the relation.
• There are N aliases to the where method: orWhere, ... etc.
• There are many types of relations and some have extra methods and some don't.
• Your idea would imply auto wrapping all extra conditions (including scopes) on a relation using callback like you mentioned >where(fn($q) => $q->where('x',1 )->orWhere('y', 2))
• If you manage to do that for all cases, you will still not be able to cover this case: $rel->getQuery()->orWhere(... so, you will not prevent this stupid thing from happening.

We tried to solve this issue, as we did (and succeeded) with other issues not solved by laravel, that is why you see our comments in these king of threads but, we were not able to come up with a bulletproof solution to prevent this kind of data leaks with the current structure of the relations.

This does not mean that you can't create a PR to solve it.