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

Retrieving duplicate models in collection #2

Closed
joedawson opened this issue Jul 30, 2018 · 21 comments
Closed

Retrieving duplicate models in collection #2

joedawson opened this issue Jul 30, 2018 · 21 comments

Comments

@joedawson
Copy link

Hello,

I currently have the following relations;

Collective → hasMany → Artist → belongsToMany → Video

I currently have the following hasManyDeep relation setup for a collective to fetch their videos.

public function videos()
{
    return $this->hasManyDeep(Video::class, [Artist::class, 'artist_video']);
}

However, when I fetch the videos for a collective using $collective->videos I receive duplicate videos since an the same artists in a collective, can be assigned to the same video.

How can I ensure that only unique videos are fetched? Is there something wrong with my relation?

@staudenmeir
Copy link
Owner

When multiple artists in a collective are assigned to the same video, a JOIN query naturally returns those videos multiple times.

Group the query to get unique results:

public function videos()
{
    return $this->hasManyDeep(Video::class, [Artist::class, 'artist_video'])
        ->groupBy('videos.id');
}

@joedawson
Copy link
Author

joedawson commented Jul 30, 2018

Hi @staudenmeir,

Thank you for clarifying the how the query works, I appreciate your help.

I just updated my relationship with the code you provided where the videos are then grouped by id to give me unique results, however I am receiving the following error;

SQLSTATE[42000]: Syntax error or access violation: 1055 Expression #17 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'my_db_name.artists.collective_id' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by (SQL: select videos.*, artists.collective_id from videos inner join artist_video on artist_video.video_id = videos.id inner join artists on artists.id = artist_video.artist_id where artists.collective_id = 1 and published = 1 group by videos.id)

To confirm, this is how I'm currently fetching the videos to be displayed on a collectives page.

$videos = $collective->videos()->latest('published_at')->simplePaginate(18);

@staudenmeir
Copy link
Owner

Sorry about that. I explicitly tested the query for this problem, but it worked. Now I get the same error.

The easiest solution is probably to filter the query results:

$videos = $collective->videos->unique();

I'll think about an in-query solution.

@joedawson
Copy link
Author

Hi @staudenmeir,

Thank you - I'll try this out shortly. However from experience I'm not sure I can paginate unique results. But I know that's not an issue directly related to this issue so will look around for a solution.

Thanks for your all your help, this package is excellent.

@staudenmeir
Copy link
Owner

You're right, this isn't a solution for pagination. You can make the raw query work like this:

select videos.*, min(artists.collective_id) ...

But I don't see a way to override this part of the SELECT clause when using the relationship.

I was thinking about something like $this->hasManyDeepUnique(), but the min(...) solution only works for $collective->videos and not for eager loading.

The really annoying part is that selecting artists.collective_id wouldn't even be necessary for your query. It's only necessary for eager loading, but gets added to all queries automatically.

@staudenmeir
Copy link
Owner

I found a solution, check out v1.1. You can now use pagination with ->groupBy('videos.id') to get unique results.

@joedawson
Copy link
Author

Thank you so much @staudenmeir, this works perfectly.

@Skintillion
Copy link

Is GroupBy still working? I get an error with it still.
Just doing a simple get, no pagination

Illuminate/Database/QueryException with message 'SQLSTATE[42000]: Syntax error or access violation: 1055 Expression #14 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'b.a_id' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by (SQL: 
select `e`.*, `b`.`a_id` as `laravel_through_key` 
from 
`e` 
inner join `d` on `d`.`e_id` = `e`.`id` 
inner join `c` on `c`.`d_id` = `d`.`id` 
inner join `b` on `b`.`c_id` = `c`.`id` 
where 
`d`.`deleted_at` is null and 
`c`.`deleted_at` is null and 
`b`.`deleted_at` is null and 
`b`.`a_id` = 1 
and 
`e`.`deleted_at` is null 
group by `e`.`id`
)'

Relationship:

public function e()
    {
        return $this->hasManyDeep('App\Models\E', 
            ['App\Models\B', 'App\Models\C','App\Models\D'],
            [
                'a_id',
                'id',
                'id',
                'id'
            ],
            [
                'id',
                'c_id',
                'd_id',
                'e_id'
            ]
        )->groupBy('e.id');
}

@staudenmeir
Copy link
Owner

@Skintillion You need to adjust the query:

$a->e()->getQuery()->select('e.*')->get();

@Skintillion
Copy link

Perfect thank you.

Love this package. Thanks!

@GDanielRG
Copy link

GDanielRG commented Mar 4, 2020

the groupBy is not working for me on the v.1.11 any thoughts? it gives me the error:

Illuminate/Database/QueryException with message 'SQLSTATE[42000]: Syntax error or access violation: 1066 Not unique table/alias: 'product_items' (SQL: select product_items.*, product_options.base_product_id as laravel_through_key from product_items inner join product_items on product_items.id = product_items.product_item_id inner join product_item_product_variant on product_item_product_variant.product_item_id = product_items.id inner join product_variants on product_variants.id = product_item_product_variant.product_variant_id inner join product_options on product_options.id = product_variants.product_option_id where product_options.base_product_id = 2 group by product_items.id)'

I have this in my relationship:

public function productItems()
{
    return $this->hasManyDeep(ProductItem::class, [ProductOption::class, ProductVariant::class, 'product_item_product_variant'])->groupBy('product_items.id');
}

If I remove the groupBy it works perfectly, just with the duplicate values

Im loving this package so far just need to figure this last thing out.

Thanks for any help :)

@staudenmeir
Copy link
Owner

@GDanielRG How are you querying the relationship?

@GDanielRG
Copy link

GDanielRG commented Mar 5, 2020

@staudenmeir
This is my setup:

BaseProduct -> has Many -> ProductOption -> has Many -> ProductVariant -> belongsToMany -> ProductItem

and in my Base product I have the relationship:

public function productItems()
{
    return $this->hasManyDeep(ProductItem::class, [ProductOption::class, ProductVariant::class, 'product_item_product_variant']);
}

And I just do

BaseProduct::first()->productItems

which works fine but of course because of the many to many between ProductVariant and ProductItem I get duplicate results.

and when adding the ->groupBy('product_items.id') it gives me the error above

@staudenmeir
Copy link
Owner

Relationship queries with groupBy() require additional adjustments: #2 (comment)

The (easier) alternative is removing duplicates from the query results: #2 (comment)

@GDanielRG
Copy link

GDanielRG commented Mar 5, 2020

Awesome It worked like this:

added ->groupBy('product_items.id') to the relationship and then queried:

BaseProduct::first()->productItems()->getQuery()->select('product_items.*')->get();

Thank you very much, also if you have the time, any direction on how to achieve the opposite? ProductItem::first()->baseProduct seems like belongsTo are a little trickier and cant find the correct way to build the relationship :S

@staudenmeir
Copy link
Owner

The opposite relationship looks like this:

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

    public function baseProducts()
    {
        return $this->hasManyDeep(
            BaseProduct::class,
            ['product_item_product_variant', ProductVariant::class, ProductOption::class],
            [null, null, 'id', 'id'],
            [null, null, 'product_option_id', 'base_product_id']
        );
    }
}

@GDanielRG
Copy link

Thanks for the help!

changed it to:

public function baseProduct()
{
    return $this->hasOneDeep(
        BaseProduct::class,
        ['product_item_product_variant', ProductVariant::class, ProductOption::class],
        [null, null, 'id', 'id'],
        [null, null, 'product_option_id', 'base_product_id']
    );
}

Since all the variants will always come from the same BaseProduct and works like wonders!!!

Thanks!

@messiahUA
Copy link

I suppose this approach won't work with eager loading? It adds "laravel_through_key" which seems to be impossible to remove from select.

@staudenmeir
Copy link
Owner

@messiahUA The laravel_through_key column is required for eager loading to work.

You can only remove duplicates from the results:

$users = User::with('permissions')->get()
    ->each(function (User $user) {
        $user->setRelation('permissions', $user->permissions->unique());
    });

@peterthomson
Copy link

peterthomson commented Feb 20, 2021

For anyone finding this in 2021. HasManyDeep relationship declarations accept ->distinct() as an added chain at the end of the relationship declaration. No guarantees that it'll always work, but in our use case it worked perfectly to return only unique results of the distant relation.

@MannikJ
Copy link

MannikJ commented Aug 18, 2021

For anyone finding this in 2021. HasManyDeep relationship declarations accept ->distinct() as an added chain at the end of the relationship declaration. No guarantees that it'll always work, but in our use case it worked perfectly to return only unique results of the distant relation.

Is also use this, but at least for MySQL8, PHP8 and Laravel 8.x I can say that it does't not work for all cases. For example count is applied before the distinct filter, so the count result might be bigger than the distinct results when using this. This can - depending on your app - break pagination etc.

When using distinct and there are multiple relations in between that point to the same target model there would be a different result between:
$model->deepRelation->count() // only distinct results are counted
and
$model->deepRelation()->count() // also between (join) records are counted

So if you have an influence on how the pagination works, you would need to change the query to explicitly count distinct columns. It could look something like this:
$model->deepRelation()->count('target.id')

However, I noticed this behavior when using Nova which creates the pagination automatically. And I have not yet figured out how to change that query.

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

No branches or pull requests

7 participants