[5.5] Add Eloquent Features: withSum,withMax,withMin,withAvg etc.

[xuqihua]

#18303

Add feature to 5.5.

[xuqihua]

` Example:

$row = Post::withSum('comments|like')
    ->withMax('comments|like')
    ->withMin('comments|like')
    ->withAvg('comments|like')
    ->first();

$row->toArray()

like data: 1,2,3,4,5
expect:
[
    ...
    'comments_sum' => 15,
    'comments_max' => 5,
    'comments_min' => 1,
    'comments_avg' => 3,
];`

[JosephSilber]

A pipe is an interesting character to be using for such a thing 😑

[tillkruss]

I was thinking the same. A comma might make more sense in this case.

[decadence]

Why isn't dot applicable here?

[mateusjatenee]

@decadence the dot is used for nested relationships

[decadence]

Which are not supported by withCount and these methods.

[jstoone]

Really like the feature! 👌

Just a few comments, but nicely done. 🎉

[jstoone]

The first comment in this review
I would remove the following comment, since the code speaks for it self. 👍

[jstoone]

Double negatives
You can avoid the double negative of not, not having the pipe by using:

if (Str::contains($name, '|')) { /* ... */ }

Pipe vs. comma
And also I would personally choose a comma as a separator, since I find the pipe communicates AND whereas the comma communicates more separation than the pipe. Hope that makes sense.

[marcvdm]

Maybe use the colon name:column

This is also used in the rules for example unique:posts - rule:column

[xuqihua]

@decadence
dot(.) is used in nested relation
any better suggestion?

[decadence]

I don't understand how it conficts here if these methods don't support nested relations.
Anyway I'm fine with any separator.

[taylorotwell]

I don't think I'm going to add this for now. It may be macroable?

[alexweissman]

Not sure why this issue didn't reference #16815. That seems to solve the problem with a more sensible , to separate the arguments, rather than combining them on a pipe.

Unfortunately all three PRs (#16815, #18303, and #19363) seem to lack decent unit tests (all I see are assertions on the final generated queries).

[alexweissman]

Actually, if separating out the relation and property names into two separate arguments is a problem (for example, when you want to apply constraints), what about the ~ character as a separator? There is precedent to using it for parent-child relationships, for example in CSS.

I do believe that : would be the best, but unfortunately that already appears to have special meaning, at least in Laravel 5.4: https://github.com/illuminate/database/blob/5.4/Eloquent/Builder.php#L1067

[alexweissman]

Just one more comment: it might also be worth implementing a withNumericAggregate method to parallel the numericAggregate method in Query\Builder: https://github.com/illuminate/database/blob/5.4/Query/Builder.php#L2032-L2059

This converts null sums/averages/etc to 0 and casts all aggregates to an appropriate int or float.