staudenmeir / eloquent-has-many-deep

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

Weird behaviour with `getQuery()` on `HasManyDeep` relation #200

Closed tdsat closed 1 year ago

tdsat commented 1 year ago

I have the following structure

b

I have added the following relationship on the User model

 public function projectIssues(): HasManyDeep
{
    return $this->hasManyDeep(ProjectIssue::class, ['user_projects', Project::class]);
}

Everything works fine if I do $user->projectIssues or $user->projectIssues()->first()

The results of $user->projectIssues()->toSql() and $user->projectIssues()->getQuery()->toSql() are identical.

However, if I do $user->projectIssues()->getQuery()->first(), I would expect

App\Models\ProjectIssue {
   description: 'Foo'
}

instead I get

App\Models\ProjectIssue { <- Note that it's the expected class but incorrect data.
   name: 'Bar',           <- 'name' and 'year' exist on the 'projects' table, not the 'issues' one
   year: 1998,
}

So it seems to create the correct class but hydrates it with the wrong data? I'm not sure if I'm doing something wrong of there's indeed a bug. The only reason I found this is because I'm trying to use this with the Filament package.

Let me know if there's any additional info I can provide.

Edit 1: Here is a trivial test case that fails

    public function testGetQuerySomething()
    {
        $projects = Employee::find(131)->projects()->getQuery()->get(); // this fails
        //$projects = Employee::find(131)->projects()->get(); // this works

        $this->assertEquals([101, 102], $projects->pluck('id')->all());
    }
staudenmeir commented 1 year ago

Hi @tdsat,

The results of $user->projectIssues()->toSql() and $user->projectIssues()->getQuery()->toSql() are identical.

The SQL is identical, but it's not the SQL that actually gets executed (without ->getQuery()).

The actual SQL limits the selected columns while the SQL from toSql() selects all columns from all tables involved: select "issues".*, [...] from "issues" vs. select * from "issues"

That's also what happens when you apply ->getQuery() and the reason why you get data from different tables.

This behavior is not specific to the package but also the way that native Laravel relationships behave. It's not really possible to change that.

Do you need to apply ->getQuery()?

tdsat commented 1 year ago

This behavior is not specific to the package but also the way that native Laravel relationships behave. It's not really possible to change that.

Oh, ok sorry I didn't know that. I'm not very familiar with Laravel yet :(

The SQL is identical, but it's not the SQL that actually gets executed (without ->getQuery()).

I'm not sure I understand. The generated SQL for both is correct, (select * from 'issues') but there is an intermediate step somewhere that uses a different one when ->getQuery() is used?

Do you need to apply ->getQuery()?

I need an instance of Illuminate\Database\Eloquent\Builder. Is there another way to get that?

In any case, I've found an alternative to this. I get the inverse of the relationship, like this:

Issue::whereHas('project',
    fn (Builder $query) => $query->where('user_id', auth()->id())
 )

which does solve my problem, so since there's nothing to be done, we can close this issue. In any case, thanks for your time 🙇‍♂️

staudenmeir commented 1 year ago

I'm not sure I understand. The generated SQL for both is correct, (select * from 'issues') but there is an intermediate step somewhere that uses a different one when ->getQuery() is used?

The SQL that toSql() generates is them same in both cases, but it's the "wrong" one (since it doesn't limit the selected columns). This SQL does get executed when you apply ->getQuery(), but that's the whole reason why the result you get is incorrect.

If you use the relationship "normally" and don't apply ->getQuery(), the executed SQL is actually different (i.e. it limits the selected columns), but toSql() doesn't/can't know about that.

Typically, the result of toSql() is correct, but there are exceptions like relationships with multiple tables (e.g. BelongsToMany, HasManyThrough, HasManyDeep).

In any case, I've found an alternative to this. I get the inverse of the relationship, like this:

An alternative would be that you limit the selected columns yourself:

$user->projectIssues()->getQuery()->select('issues.*')->get();
tdsat commented 1 year ago
$user->projectIssues()->getQuery()->select('issues.*');

Yeap, this works for Filamant (in case anybody gets here through a search engine).

I'm still confused as to what's going on and why it doesn't work as expected in the first place, but that's beyond the scope of this. I'm closing the issue since it's not related to the package itself.

Once again, thanks for your help and time.