[5.1] Fix sum with no results

[KnightVision90]

For ticket #14793, this code was removed to use the numericAggregate function. When it was reverted on ticket #14994 it only replaced sum with returning aggregate even though prior to the initial change this code checked result and returned 0 in case the result was null.

This is just reverting back to how the code was prior to being changed to use numericAggregate so that it will return 0 if there is no results returned.

[tillkruss]

Why not this:

return $this->aggregate(__FUNCTION__, [$column]) ?: 0;

[KnightVision90]

This was just done to revert it back to its original state before the reverted code was applied. If Taylor wants, I can do it like this.

[GrahamCampbell]

👎 This breaks more people's code than it fixes.

[KnightVision90]

The merge and revert done caused the functionality of the sum method to change from how it had been for over a year and provided no better functionality and I'd argue a return of null ONLY when there is no sql results wouldn't make sense as it's clear by using the sum() method you are looking for a numeric response.

Can you provide some sources for people's code who are reporting this breaking?

[taylorotwell]

Honestly just think I will revert this. 5.1 is end of life in 2 months and I don't want to change existing functionality at this point. Even if it is unintuitive.

[GrahamCampbell]

This change was already made in a later laravel version anyway, because it was breaking.

[KnightVision90]

I'm not seeing any pull requests that mention moving from null to 0 broke their code and this code will match all future versions as well. '0', 0, and null are all false values so if ($val) { will continue to evaluate the same with this code but when you use the value in a blade template, it won't return any string when you are expecting it to return 0. I'm not seeing any references to it being broken before, just that it wasn't returning an integer type rather than a string for all results.

I see this exact request was done and merged in #15345, showing the previous requests broke it for more people than this PR would have. Isn't part of the purpose of LTS to be so that we know how long we can stay on a minor version before we should upgrade? We were waiting until 5.5 LTS to update minor versions and having to do that to fix a bug that was introduced due to a piece of code being missed from a revert seems counter-intuitive.