8000 HasManyThrough getQuery error · Issue #55738 · laravel/framework · GitHub
[go: up one dir, main page]
Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

HasManyThrough getQuery error #55738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
8000 Closed
mathieutu opened this issue May 14, 2025 · 3 comments
Closed

HasManyThrough getQuery error #55738

mathieutu opened this issue May 14, 2025 · 3 comments

Comments

@mathieutu
Copy link
Contributor
mathieutu commented May 14, 2025 •
edited
Loading

Laravel Version

12.14.1

PHP Version

8.4

Database Driver & Version

Sqlite

Description

Hi,
When using HasManyThrough::getQuery (or toRawSql, or any other using to getQuery under the hood), the query used is wrong, because prepareQueryBuilder wasn't called.

The result in the case I have encounter is wrong models attributes fetch, especially with wrong ids (id attribute is coming from the table joined).

One solution could be to override getQuery in HasManyThrough class to call prepareQueryBuilder or shouldSelect (which contains the wanted logic).

framework/src/Illuminate/Database/Eloquent/Relations/HasOneOrManyThrough.php

Lines 520 to 526 in df12a08

protected function shouldSelect(array $columns = ['*'])
{
if ($columns == ['*']) {
$columns = [$this->related->qualifyColumn('*')];
}
return array_merge($columns, [$this->getQualifiedFirstKeyName().' as laravel_through_key']);

I can make the PR if you want.

WDYT?

Thanks,
Mathieu

Steps To Reproduce

With the example from the doc:

public function deployments(): HasManyThrough
{

     return $this->hasManyThrough(Deployment::class, Environment::class);

}

$this->deployments()->toRawSql() will return

select * from "deployments" inner join "environments" on "environments"."id" = "deployments"."environment_id" where "environment"."application_id" = XX

instead of

select "deployments".*, "environments"."application_id" as "laravel_through_key" from "deployments" inner join "environments" on "environments"."id" = "deployments"."environment_id" where "environment"."application_id" = XX
@mathieutu mathieutu changed the title HasManyThrough toBase error HasManyThrough getQuery/toBase error May 14, 2025
@mathieutu mathieutu changed the title HasManyThrough getQuery/toBase error HasManyThrough getQuery error May 14, 2025
@macropay-solutions
Copy link
macropay-solutions commented May 15, 2025 •
edited
Loading

@mathieutu

    /**
     * Get the underlying query for the relation.
     *
     * @return \Illuminate\Database\Eloquent\Builder
     */
    public function getQuery()
    {
        return $this->query;
    }

getQuery only retrieves the eloquent builder and could be called multiple times in a request. We would not recommend adding logic to it.

We noticed that toSql does not retrieve the right sql for relations and we had to throw exception in statement prepared event to get the actual sql that is being executed in order for us to async eager load relations using curl_multi_init (guzzle pool).

We did not bother to open an issue about it because relations are an extension of the eloquent builder so, this might be a known consequence.

@mathieutu
Copy link
Contributor Author

The "logic" that would be added is only a addSelect. I don't see the issue with that.
(On my case, I'm passing the query to a function that expect an Eloquent\Builder object, hence the getQuery.)

@macropay-solutions
Copy link

That will break some use cases like select distinct from relation and others for sure. You can create a macro to solve your case or manually add extra things to the returned Eloquent Builder (clone to be sure you are not persisting the changes in the object).

@laravel laravel locked and limited conversation to collaborators May 19, 2025
@crynobone crynobone converted this issue into discussion #55777 May 19, 2025

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mathieutu @macropay-solutions
0