Skip to content
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.

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

Laravel hasManyThrough returns wrong withCount but good ->count() #30575

Closed
musapinar opened this issue Nov 12, 2019 · 4 comments
Closed

Laravel hasManyThrough returns wrong withCount but good ->count() #30575

musapinar opened this issue Nov 12, 2019 · 4 comments

Comments

@musapinar
Copy link

  • Laravel Version: 6.5
  • PHP Version: 7.3.10
  • Database Driver & Version: MySQL 5.5.60

Description:

I eager load a relationship so that it can return the count of distinct commentators of a blog post. Such as this:

->withCount('commentators as total_commentators_count')

Now in my view, the following will not return the correct count: $post->total_commentators_count

but the following will return the correct count: $post->commentators->count()

How come?

Here are some screenshots (I blurred the unnecessary code for convenience) :

https://i.imgur.com/bNcrVid.png

https://i.imgur.com/WlzVqAs.png

Here is my commentators() relation (within a Commentable.php trait that my BlogPost model uses) :

    public function commentators(): hasManyThrough
    {
        /** @noinspection PhpUndefinedMethodInspection */
        return $this->hasManyThrough(
            'App\User',
            'App\Comment',
            'commentable_id',
            'id',
            'id',
            'user_id'
        )
        ->where('commentable_type', array_search(static::class, Relation::morphMap()) ? : static::class)
        ->groupBy('users.id', 'comments.commentable_id')
        ->orderByRaw('MIN(comments.created_at) ASC, users.created_at ASC, users.id ASC');
    }

The comments table is polymorphic.

and here is what the $posts (see screenshot), which is supposed to eager load commentators and count it, actually runs in SQL :

select `posts`.*, 
(select count(*) from `users` inner join `comments` on `comments`.`user_id` = `users`.`id` where `comments`.`deleted_at` is null and `posts`.`id` = `comments`.`commentable_id` and `comments`.`deleted_at` is null and `commentable_type` = ? and `users`.`deleted_at` is null) as `total_commentators_count`, 
(select count(*) from `comments` where `posts`.`id` = `comments`.`commentable_id` and `comments`.`commentable_type` = ? and `comments`.`deleted_at` is null) as `total_comments_count`, 
(select count(*) from `comments` where `posts`.`id` = `comments`.`commentable_id` and `comments`.`commentable_type` = ? and `user_id` = ? and `comments`.`deleted_at` is null) as `user_comments_count` 
from `posts` 
where `posts`.`deleted_at` is null order by `created_at` desc"

it seems to be missing all the groupBy and orderByRaw parts?

The whole idea behind this is the following: I'm trying to show how many different people commented on a post (eventhough there are 100 comments, that could be just 2 different users replying to each other).

@driesvints
Copy link
Member

I can find: #26623 which got fixed by #26676 but maybe this doesn't works for polymorphic relationships.

I've also found #26635 (comment) which maybe helps?

@staudenmeir do you know if you kept polymorphic relationships into account with your PR from above?

@musapinar
Copy link
Author

@driesvints I do not make use of select()

	public function index()
	{
		$posts = Post::query()
            //->withCount('commentators as total_commentators_count')
            ->latest()
            ->with([
                'user',
                'tags' => function($query) {
                    /** @var \Illuminate\Database\Eloquent\Builder $query */
                    $query->orderBy('name', 'ASC');
                },
                'commentators',
            ])
            ->withCount('commentableComments as total_comments_count')
            ->when(auth()->id(), function ($query) {
                /** @var \Illuminate\Database\Eloquent\Builder $query */
                return $query->withCount(['commentableComments as user_comments_count' => static function ($innerQuery) {
                    /** @var \Illuminate\Database\Eloquent\Builder $innerQuery */
                    $innerQuery->where('user_id', auth()->id());
                }]);
            })
            ->filter([
                'year' => request('year'),
                'month' => request('month'),
            ])
            ->paginate(12);

        return view('posts.index', compact('posts'));
	}

Freek dot dev linked an interesting article very recently that I will try soon, considering how HasManyThrough() is "very confusing" indeed : stitcher dot io/blog/laravel-custom-relation-classes

@staudenmeir
Copy link
Contributor

This is not a Laravel issue.

Aggregate queries with GROUP BY clauses don't actually return the result you expect them to (laravel/ideas#1693). You can also see that when running $post->commentators()->count().

Use a DISTINCT clause instead:

Post::withCount([
    'commentators as total_commentators_count' => function (Builder $query) {
        $query->select(DB::raw('count(distinct users.id)'));
    },
])->get();

it seems to be missing all the groupBy and orderByRaw parts?

Relationships should never have GROUP BY clauses.
There are no ORDER BY clauses because they don't affect the result of withCount().

@driesvints
Copy link
Member

@musapinar please see the answer above.

@staudenmeir thanks for checking in 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants