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

[11.x] Support for multiple partition by columns in groupLimit() #52072

Closed
wants to merge 4 commits into from

Conversation

kitbs
Copy link
Contributor

@kitbs kitbs commented Jul 9, 2024

This PR builds on the "eager loading with limit" functionality introduced in #49695.

It allows groupLimit() to accept an array of columns, which will add multiple columns to the partition by part of the query.

This is immediately achieved just by changing from wrap() to columnize() in Grammar::compileRowNumber(). The rest of the changes in this PR are to support the MySQL legacy workaround and of course testing.

Use Case

The use case for this is to be able to use groupLimit() on its own to get a limit of n records from each group, grouped by multiple columns (not just the foreign key in a relationship).

For example, say I want to get the latest address of each type for each user:

$addresses = Address::groupLimit(1, ['user_id', 'address_type'])
    ->latest()
    ->get();

This generates the following SQL:

select * from (
    select *, row_number() over (
        partition by `user_id`, `address_type`
        order by `created_at` desc
    ) as `laravel_row`
    from `addresses`
) as `laravel_table`
where `laravel_row` <= 1 order by `laravel_row`

Relationships

You can also use groupLimit() in a relationship definition:

class User extends Model
{
    public function latestAddresses(): HasMany
    {
        return $this->hasMany(Address::class)
            ->groupLimit(1, ['user_id', 'address_type'])
            ->latest();
    }
}

Or indeed in the original eager-loading scenario:

$users = User::with([
    'addresses' => fn (Builder $query) => $query->groupLimit(1, ['user_id', 'address_type'])->latest(),
])->get();

Both of these are similar to ofMany() to convert a HasMany relationship into a HasOne. But instead, it will still return multiple records for each parent, but only one from each group (in this example, one of each address type).

Future State

I had a bit of a think about whether, on the *Many relationship classes, Laravel magic could take care of the foreign key and only the additional column(s) would need to be manually specified, i.e. the way limit() currently sends getExistenceCompareKey()/getQualifiedFirstKeyName() to groupLimit().

This could be done either by adding a new optional $columns parameter to limit() or a new method altogether, but I thought I'd start with the basic change first and work from there.

Let me know if there is any interest in this and any preferences for how best to implement it.

@kitbs kitbs changed the title Support for multiple partition by columns in groupLimit() [11.x] Support for multiple partition by columns in groupLimit() Jul 9, 2024
@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!

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

Successfully merging this pull request may close these issues.

2 participants