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

[8.x] Add loadAggregate() and load[min(), max(), sum() & avg()] methods to … #35029

Merged
merged 1 commit into from
Oct 30, 2020

Conversation

khalilst
Copy link
Contributor

This PR adds the following methods to the Eloquent/{Collection, Model} which were missing in PR #34965:

    //Eloquent/Collection

    public function loadAggregate($relations, $column, $function = null) {...}
    public function loadCount($relations) {...} //Just modified.
    public function loadMax($relations, $column)  {...}
    public function loadMin($relations, $column)  {...}
    public function loadSum($relations, $column)  {...}
    public function loadAvg($relations, $column)  {...}

And

    //Eloquent/Model

    public function loadAggregate($relations, $column, $function = null) {...}
    public function loadCount($relations) {...} //Just modified.
    public function loadMax($relations, $column) {...}
    public function loadMin($relations, $column) {...}
    public function loadSum($relations, $column) {...}
    public function loadAvg($relations, $column) {...}

    public function loadMorphAggregate($relation, $relations, $column, $function = null) {...}
    public function loadMorphCount($relation, $relations) {...} //Just modified.
    public function loadMorphMax($relation, $relations, $column) {...}
    public function loadMorphMin($relation, $relations, $column) {...}
    public function loadMorphSum($relation, $relations, $column) {...}
    public function loadMorphAvg($relation, $relations, $column) {...}

@driesvints driesvints changed the title Add loadAggregate() and load[min(), max(), sum() & avg()] methods to … [8.x] Add loadAggregate() and load[min(), max(), sum() & avg()] methods to … Oct 29, 2020
…Eloquent/{Collection, Model}

Also add loadMorphAggregate() and loadMorph[min(), max(), sum() & avg()] methods to Eloquent/Model
@dbakan
Copy link
Contributor

dbakan commented Oct 29, 2020

Will it be possible to load multiple relations at once? Maybe even with constraints? Such as:

$orders->loadSum([
   'orderItems.price as original_total_cost',
   'orderItems.weight as shipping_weight',
   'orderItems.price as total_refunded' => function($query) { 
      $query->where('is_refunded', true);
   },
])

@khalilst
Copy link
Contributor Author

@dbakan Yes, it is possible. Just you forgot to specify the second argument (the $column).

@dbakan
Copy link
Contributor

dbakan commented Oct 29, 2020

@khalilst I specified different columns intentionally as part of the array keys/relation names:
orderItems.price vs orderItems.weight.

To me, this would be closer to the "Laravel Way", esp. if all aggregates are being fetched within a single statement.

@khalilst
Copy link
Contributor Author

@dbakan It needs to change the source withAggregate() method in QueriesRelationships and I think it is a good idea to apply those changes in another PR after confirmation by @taylorotwell .

@dbakan
Copy link
Contributor

dbakan commented Oct 29, 2020

@khalilst I'd be happy to help with those changes.

@dbakan
Copy link
Contributor

dbakan commented Oct 30, 2020

@khalilst This is an approach I've been working on lately: https://github.com/dbakan/aggregate

It does not work for the current 8.x, but it does work for v8.11.2 (before the new withSum() etc. was merged).

It supports the dot syntax, mixed constraints and some more. It's still work in progress and has some issues, but personally, I prefer:

Order::withSum('products.price as total');
// over
Order::withSum('products as total', 'price');

And I think this would be more flexible, e.g. mix aggregates:

Order::withAggregate([
    'products as products_count' => 'count',
    'products.deleted_at AS deleted_count' => 'count',
    'products.quantity as total_items' => 'sum', // using sum() here now!
    'products.price as special_aggregate' => function($query) {
        // write more complex aggregates:
        $query->select(\DB::raw('SUM(product_orders.quantity*product_orders.price)'));
    },
    'products.discount' => 'MAX',
    'products.quantity as quantity_list' => 'group_concat', // (I am working on JSON_ARRAY_AGGR too)
]);

What do you think @khalilst @taylorotwell ?

@khalilst
Copy link
Contributor Author

@dbakan I disagree with all respect.
IMO, it pushes towards Raw SQL, instead of PHP.

If I was there to make withCount(), I prefer to use this:

Model::withCount($relations, $alias)

instead of

Medel::withCount('relation as alias');

But if others and @taylorotwell have your idea I will make another PR for these changes with your help.

@dbakan
Copy link
Contributor

dbakan commented Oct 30, 2020

IMO, it pushes towards Raw SQL, instead of PHP.

@khalilst Yes, this may be true.

I just liked the idea of supporting multiple aggregates over different columns like:

Model::loadSum(
   'relation.price AS total_cost', 
   'relation.weight AS total_weight', 
   ...
);

And doing it in a single query just like withCount. But maybe it's just me.

@taylorotwell taylorotwell merged commit 363dc94 into laravel:8.x Oct 30, 2020
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.

None yet

3 participants