staudenmeir / eloquent-has-many-deep

Laravel Eloquent HasManyThrough relationships with unlimited levels
MIT License
2.67k stars 157 forks source link

Using whereHas on self-related realtionships #170

Closed hafezdivandari closed 2 years ago

hafezdivandari commented 2 years ago

I have User and UserTeam models and this self-related relation on User model:

class User extends Model
{
    use \Staudenmeir\EloquentHasManyDeep\HasRelationships;

    public function teammates()
    {
        return $this->hasManyDeep(User::class, ['App\Models\TeamUser as user_team', Team::class, TeamUser::class]);
    }
}

class TeamUser extends Pivot
{
    use \Staudenmeir\EloquentHasManyDeep\HasTableAlias;
}

It works fine and I can get teammates of a user, but when using on whereHas to get users that have e.g user id = 2 as their teammates it doesn't work:

\App\Models\User::whereHas('teammates', function (Builder $query) {
    $query->where('id', 2);
})->get()

because column 'id' in where clause is ambiguous. we can't use users.id either, here is the SQL:

select * from `users`
where exists (
  select * from `users` as `laravel_reserved_0`
  inner join `team_user` on `team_user`.`user_id` = `laravel_reserved_0`.`id`
  inner join `teams` on `teams`.`id` = `team_user`.`team_id`
  inner join `team_user` as `user_team` on `user_team`.`team_id` = `teams`.`id`
  where `users`.`id` = `user_team`.`user_id`
    and (`id` = 5)
)

I think we have 2 way to solve this:

  1. Qualify the columns of the query constraints in a has / whereHas:
    select * from `users`
    where exists (
      select * from `users` as `laravel_reserved_0`
      ...
        and (`laravel_reserved_0`.`id` = 5)
    )
  2. Use the relation name as alias instead of the generated hash laravel_reserved_0:
    select * from `users`
    where exists (
      select * from `users` as `teammates`
      ...
        and (`teammates`.`id` = 5)
    )

I believe every Laravel Relation classes that use getRelationCountHash() to generate alias for self-related relation also have this issue, am I right? (related to laravel/framework#42075)

Meanwhile thank you so much for this useful and well-written package.

staudenmeir commented 2 years ago

Hi @hafezdivandari,

I believe every Laravel Relation classes that use getRelationCountHash() to generate alias for self-related relation also have this issue, am I right?

Yes, that's right. I recommend that you use qualifyColumn() to get the correct table name automatically:

\App\Models\User::whereHas('teammates', function (Builder $query) {
    $query->where($query->qualifyColumn('id'), 2);
})->get();

Does that work for you?

hafezdivandari commented 2 years ago

Hi @staudenmeir thank you, that works but why not sending a PR on Laravel? it is obvious that using whereHas on self-related relations doesn't work. We already know the condition, we can only qualify the column when it's self related relation and is not already qualified (i.e. it doesn't contain '.') right? so it won't be a breaking change. I know they apply query constraints on this line. Maybe I should right another callScope for this, I'm not sure how to do it.

I'm really sorry to bother you, I know it is not directly related to this repo but you are expert on Laravel relations, even if I figure out how to solve this, such PRs doesn't get merge easily without support. Just close this issue if you are not interested.

staudenmeir commented 2 years ago

We already know the condition, we can only qualify the column when it's self related relation and is not already qualified (i.e. it doesn't contain '.') right? so it won't be a breaking change.

That's not possible because we don't know which table an unqualified constraint is referring to. In your case, it could refer to the teams table and we would break the query by automatically qualifying the column.

hafezdivandari commented 2 years ago

@staudenmeir I don't think so, because qualifyColumn does qualify the given column if it doesn't contain ., If I want to refer to teams table I can simply do:

\App\Models\User::whereHas('teammates', function (Builder $query) {
    $query->where($query->qualifyColumn('teams.id'), 2);  //would be as same as $query->where('teams.id', 2);
})->get();
staudenmeir commented 1 year ago

if I want to refer to teams table I can simply do:

You can do that when you write new code, but changing whereHas() would break existing queries where the column isn't qualified.